Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify device driver error message #2496

Merged

Conversation

ann0see
Copy link
Member

@ann0see ann0see commented Mar 12, 2022

Short description of changes

Split out from #2168

Re-submitting since the other PR seemed to contained too much different code changes (added ASIO driver Setup button, which could be part of another PR.

CHANGELOG: Simplify device driver error message on Windows by removing unneeded HTML

Context: Fixes an issue?

No. But it should make the error message more readable.
Does this change need documentation? What needs to be documented and how?
I doubt it.

Status of this Pull Request

PoC, still open for improvements.

What is missing until this pull request can be merged?

Test on Windows, Review

Checklist

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • My code follows the style guide
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I've filled all the content above

@ann0see ann0see force-pushed the feature/simplifyASIOWarning branch from 1c57741 to cc03211 Compare March 13, 2022 07:41
src/soundbase.cpp Outdated Show resolved Hide resolved
src/soundbase.cpp Outdated Show resolved Hide resolved
@ann0see ann0see force-pushed the feature/simplifyASIOWarning branch 2 times, most recently from e966f06 to 7c9032e Compare March 13, 2022 11:21
src/soundbase.cpp Outdated Show resolved Hide resolved
@pgScorpio pgScorpio mentioned this pull request Mar 13, 2022
5 tasks
@ann0see
Copy link
Member Author

ann0see commented Mar 13, 2022

Ok. See the screenshot of the message:

noASIO

@ann0see
Copy link
Member Author

ann0see commented Mar 20, 2022

@gilgongo could you please Review?

@pljones
Copy link
Collaborator

pljones commented Mar 20, 2022

Ok. See the screenshot of the message:

Hm. That screenshot looks confusing. I get the same error message twice, once for each driver, and then I'm asked if I want to open "driver settings". Which driver? Do both get opened? That'll be pretty confusing...

@ann0see
Copy link
Member Author

ann0see commented Mar 20, 2022

That’s an issue with the initial (internal) error message. It would have been the same before this PR, so it’s worth changing it elsewhere (@pgScorpio maybe your change can also solve this confusing error handling)

@pgScorpio
Copy link
Contributor

pgScorpio commented Mar 20, 2022

That’s an issue with the initial (internal) error message. It would have been the same before this PR, so it’s worth changing it elsewhere (@pgScorpio maybe your change can also solve this confusing error handling)

Yes give my new sound implementation a try.

All sound related error messages, whatsthis- tooltip- and other sound related texts now come from CSoundbase and can be defined in the CSound implementation.

It also solves various other issues, like not being able to change the settings of the 'skipped' drivers.

@ann0see ann0see force-pushed the feature/simplifyASIOWarning branch from 7c9032e to aaf4db6 Compare March 22, 2022 22:02
@ann0see
Copy link
Member Author

ann0see commented Mar 27, 2022

@gilgongo @pljones could you please re-review?

src/soundbase.cpp Outdated Show resolved Hide resolved
src/soundbase.cpp Outdated Show resolved Hide resolved
src/soundbase.cpp Outdated Show resolved Hide resolved
src/soundbase.cpp Outdated Show resolved Hide resolved
@ann0see ann0see force-pushed the feature/simplifyASIOWarning branch 2 times, most recently from 6251c9c to 4cf7dec Compare April 13, 2022 20:39
@ann0see ann0see requested a review from gilgongo April 13, 2022 20:39
@gilgongo
Copy link
Member

I think the wording's OK, but I'm not sure how it's being presented - are we still showing a warning for each driver (as in #2496 (comment))?

@ann0see
Copy link
Member Author

ann0see commented May 16, 2022

Yes. There's nothing I can do about the presentation at the moment. That's what the sound redesign will hopefully handle.

@gilgongo
Copy link
Member

OK sure. Well the wording is certainly improved, so that's cool.

@ann0see ann0see force-pushed the feature/simplifyASIOWarning branch from 4cf7dec to c1be675 Compare May 16, 2022 19:19
@ann0see
Copy link
Member Author

ann0see commented May 16, 2022

@gilgongo I've now rebased this PR.

@gilgongo gilgongo merged commit 55c6ecc into jamulussoftware:master May 21, 2022
@ann0see ann0see deleted the feature/simplifyASIOWarning branch May 21, 2022 20:27
@ann0see ann0see added this to the Release 3.9.0 milestone May 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants