Skip to content

Don't imply that multiple drivers have errors #2168

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.

Sign up for GitHub

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

Closed
ann0see wants to merge 2 commits into from
Closed

Don't imply that multiple drivers have errors #2168

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d8ae5b
Don't imply that multiple drivers have errors
ann0see Feb 26, 2022
fd7bfe6
Do not show ASIO related warning message if using JACK
ann0see Feb 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions src/soundbase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,23 +167,32 @@ QString CSoundBase::SetDev ( const QString strDevName )
if ( !vsErrorList.isEmpty() )
Copy link
Collaborator

@pljones pljones Dec 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lines 154 to 165 are irrelevant if this we enter this if. They should be placed after the block.

Copy link
Member Author

@ann0see ann0see Dec 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might the problem be that this code is run once at startup and also during runtime if someone changes the sound devices?

Copy link
Collaborator

@pljones pljones Dec 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before line 197 was introduced, the variable set there wasn't referenced until after this if block. However, now line 197 is there, it makes sense.

{
// create error message with all details
QString sErrorMessage = "<b>" + QString ( tr ( "No usable %1 audio device found." ) ).arg ( strSystemDriverTechniqueName ) +
"</b><br><br>" + tr ( "These are all the available drivers with error messages:" ) + "<ul>";
QString sErrorMessage = tr ( "<b>No usable %1 audio device found.</b><br><br>" ).arg ( strSystemDriverTechniqueName );

for ( int i = 0; i < lNumDevs; i++ )
{
sErrorMessage += "<li><b>" + GetDeviceName ( i ) + "</b>: " + vsErrorList[i] + "</li>";
sErrorMessage += "<b>" + GetDeviceName ( i ) + "</b>: " + vsErrorList[i] + "<br><br>";
}
sErrorMessage += "</ul>";

#ifdef _WIN32
#if defined( _WIN32 ) && !defined( WITH_JACK )
// to be able to access the ASIO driver setup for changing, e.g., the sample rate, we
// offer the user under Windows that we open the driver setups of all registered
// ASIO drivers
sErrorMessage += "<br/>" + tr ( "Do you want to open the ASIO driver setup to try changing your configuration to a working state?" );
sErrorMessage += "<br>" + tr ( "You may be able to fix errors in the driver settings." );

if ( QMessageBox::Yes == QMessageBox::information ( nullptr, APP_NAME, sErrorMessage, QMessageBox::Yes | QMessageBox::No ) )
// for Windows use a QMessageBox to show the error
QMessageBox qmASIOWarningBox;
QPushButton* btnASIOSettings = qmASIOWarningBox.addButton ( tr ( "Open ASIO settings" ), QMessageBox::AcceptRole );

qmASIOWarningBox.addButton ( QMessageBox::Cancel );
qmASIOWarningBox.setText ( sErrorMessage );
qmASIOWarningBox.setIcon ( QMessageBox::Warning );
qmASIOWarningBox.exec();

if ( qmASIOWarningBox.clickedButton() == btnASIOSettings )
{
// TODO: This or some related code aborts the app even if the driver is valid after user changes.
// This should be handled differently. For reference, please see https://github.com/jamulussoftware/jamulus/pull/2168
Comment on lines +194 to +195
Copy link
Member Author

@ann0see ann0see Feb 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pljones I wrote a TODO comment here.

LoadAndInitializeFirstValidDriver ( true );
Copy link
Collaborator

@pljones pljones Dec 16, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vsErrorList = LoadAndInitializeFirstValidDriver ( true ); and then check the list size. If it's zero (and I've not checked the code to make sure I'm right here...), return strReturn rather than continuing.

Copy link
Member Author

@ann0see ann0see Dec 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's more work needed then just that. See the commit I mentioned: Probably it's in a deeper level.

}

Expand Down