From c01b96da21dd0634280caafd5a8c285c37ef8ced Mon Sep 17 00:00:00 2001 From: pgScorpio Date: Fri, 25 Mar 2022 22:30:21 +0100 Subject: [PATCH] Connection status issue #2519 first stage In this first stage I just renamed some functions in CClient to make the problem clear. See "---> pgScorpio:" comments in the problem area's. The next stage will actually change the code to solve this issue. --- src/client.cpp | 12 +++++++----- src/client.h | 16 ++++++++++++---- src/clientdlg.cpp | 31 ++++++++++++++++++++----------- src/clientsettingsdlg.cpp | 2 +- src/settings.cpp | 20 ++++++++++---------- 5 files changed, 50 insertions(+), 31 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index a1b34a8eeb..0323685b22 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -186,7 +186,7 @@ CClient::CClient ( const quint16 iPortNumber, if ( !strConnOnStartupAddress.isEmpty() ) { SetServerAddr ( strConnOnStartupAddress ); - Start(); + StartConnection(); // ---> pgScorpio: Was Start() } } @@ -298,7 +298,8 @@ void CClient::CreateServerJitterBufferMessage() void CClient::OnCLPingReceived ( CHostAddress InetAddr, int iMs ) { // make sure we are running and the server address is correct - if ( IsRunning() && ( InetAddr == Channel.GetAddress() ) ) + if ( SoundIsStarted() && // ---> pgScorpio: Does NOT mean we are Connected ! + ( InetAddr == Channel.GetAddress() ) ) { // take care of wrap arounds (if wrapping, do not use result) const int iCurDiff = EvaluatePingMessage ( iMs ); @@ -819,7 +820,7 @@ void CClient::OnClientIDReceived ( int iChanID ) emit ClientIDReceived ( iChanID ); } -void CClient::Start() +void CClient::StartConnection() // ---> pgScorpio: Was Start() { // init object Init(); @@ -828,10 +829,11 @@ void CClient::Start() Channel.SetEnable ( true ); // start audio interface - Sound.Start(); + Sound.Start(); // ---> pgScorpio: There is NO Check if Sound.Start() actually is successfull !! + // ---> pgScorpio: If Sound.Start fails here GUI (clientdlg) and Channel are definitely out of sync ! } -void CClient::Stop() +void CClient::StopConnection() { // stop audio interface Sound.Stop(); diff --git a/src/client.h b/src/client.h index cdb21f6e6e..5a5a2bc788 100644 --- a/src/client.h +++ b/src/client.h @@ -119,10 +119,18 @@ class CClient : public QObject virtual ~CClient(); - void Start(); - void Stop(); - bool IsRunning() { return Sound.IsRunning(); } - bool IsCallbackEntered() const { return Sound.IsCallbackEntered(); } + void StartConnection(); // ---> pgScorpio: Was Start(), but Start what ? ( Should be Connect() ?) + void StopConnection(); // ---> pgScorpio: Was Stop(), but Stop what ? ( Should be Disconnect() ?) + bool SoundIsStarted() // ---> pgScorpio: Was IsRunning(), but what is running ?? + { + return Sound.IsRunning(); // ---> pgScorpio: Even this name is incorrect ! + // ---> pgScorpio: Sound.bRun is set when Sound is started, but this does not guarantee sound is actually running + } + + bool SoundIsRunning() const + { + return Sound.IsCallbackEntered(); + } // ---> pgScorpio: was IsCallbackEntered() but this is the actuall SoundIsRunning() ! bool SetServerAddr ( QString strNAddr ); double GetLevelForMeterdBLeft() { return SignalLevelMeter.GetLevelForMeterdBLeftOrMono(); } diff --git a/src/clientdlg.cpp b/src/clientdlg.cpp index c94c03b707..e8d8163e67 100644 --- a/src/clientdlg.cpp +++ b/src/clientdlg.cpp @@ -609,9 +609,9 @@ void CClientDlg::closeEvent ( QCloseEvent* Event ) AnalyzerConsole.close(); // if connected, terminate connection - if ( pClient->IsRunning() ) + if ( pClient->SoundIsStarted() ) // ---> pgScorpio: This does NOT mean the connection is started ! { - pClient->Stop(); + pClient->StopConnection(); // ---> pgScorpio: Was pClient->Stop() } // make sure all current fader settings are applied to the settings struct @@ -732,7 +732,7 @@ void CClientDlg::OnConnectDlgAccepted() // first check if we are already connected, if this is the case we have to // disconnect the old server first - if ( pClient->IsRunning() ) + if ( pClient->SoundIsStarted() ) // pgScorpio: Was pClient->IsRunning() Again this is NOT connection started ! { Disconnect(); } @@ -748,7 +748,7 @@ void CClientDlg::OnConnectDlgAccepted() void CClientDlg::OnConnectDisconBut() { // the connect/disconnect button implements a toggle functionality - if ( pClient->IsRunning() ) + if ( pClient->SoundIsStarted() ) // pgScorpio: Was pClient->IsRunning() Again this is NOT connection started ! { Disconnect(); SetMixerBoardDeco ( RS_UNDEFINED, pClient->GetGUIDesign() ); @@ -1141,7 +1141,7 @@ void CClientDlg::OnTimerCheckAudioDeviceOk() // timeout to check if a valid device is selected and if we do not have // fundamental settings errors (in which case the GUI would only show that // it is trying to connect the server which does not help to solve the problem (#129)) - if ( !pClient->IsCallbackEntered() ) + if ( !pClient->SoundIsRunning() ) // ---> pgScorpio Was pClient->IsCallbackEntered() { QMessageBox::warning ( this, APP_NAME, @@ -1154,10 +1154,11 @@ void CClientDlg::OnTimerDetectFeedback() { bDetectFeedback = false; } void CClientDlg::OnSoundDeviceChanged ( QString strError ) { - if ( !strError.isEmpty() ) + if ( !strError + .isEmpty() ) // ---> pgScorpio: This check should already be done in CClient ! but currently the Disconnect code is at the wrong place. { // the sound device setup has a problem, disconnect any active connection - if ( pClient->IsRunning() ) + if ( pClient->SoundIsStarted() ) // ---> pgScorpio: Was pClient->IsRunning(), Again this is NOT connection started !() ) { Disconnect(); } @@ -1190,6 +1191,8 @@ void CClientDlg::OnCLPingTimeWithNumClientsReceived ( CHostAddress InetAddr, int void CClientDlg::Connect ( const QString& strSelectedAddress, const QString& strMixerBoardLabel ) { + // ---> pgScorpio: This code does not belong here but in CClient !! + // set address and check if address is valid if ( pClient->SetServerAddr ( strSelectedAddress ) ) { @@ -1197,9 +1200,9 @@ void CClientDlg::Connect ( const QString& strSelectedAddress, const QString& str // running state but show error message try { - if ( !pClient->IsRunning() ) + if ( !pClient->SoundIsStarted() ) // ---> pgScorpio: Again this is NOT connection started !() ) { - pClient->Start(); + pClient->StartConnection(); } } @@ -1210,6 +1213,8 @@ void CClientDlg::Connect ( const QString& strSelectedAddress, const QString& str return; } + // ---> pgScorpio: This code should be a OnConnecting() slot ! + // hide label connect to server lblConnectToServer->hide(); lbrInputLevelL->setEnabled ( true ); @@ -1238,15 +1243,19 @@ void CClientDlg::Connect ( const QString& strSelectedAddress, const QString& str void CClientDlg::Disconnect() { + // ---> pgScorpio: This code does not belong here but in CClient !! + // only stop client if currently running, in case we received // the stopped message, the client is already stopped but the // connect/disconnect button and other GUI controls must be // updated - if ( pClient->IsRunning() ) + if ( pClient->SoundIsStarted() ) // ---> pgScorpio: Again this is NOT connection started !() ) { - pClient->Stop(); + pClient->StopConnection(); } + // ---> pgScorpio: This code should be a OnDisconncted() slot + // change connect button text to "connect" butConnect->setText ( tr ( "C&onnect" ) ); diff --git a/src/clientsettingsdlg.cpp b/src/clientsettingsdlg.cpp index 61fc525c9e..ec3dc7f5de 100644 --- a/src/clientsettingsdlg.cpp +++ b/src/clientsettingsdlg.cpp @@ -1049,7 +1049,7 @@ void CClientSettingsDlg::UpdateDisplay() UpdateJitterBufferFrame(); UpdateSoundCardFrame(); - if ( !pClient->IsRunning() ) + if ( !pClient->SoundIsStarted() ) // pgScorpio: Was !pClient->IsRunning() Again this is NOT a connection status, Should be pClient->IsConnected() { // clear text labels with client parameters lblUpstreamValue->setText ( "---" ); diff --git a/src/settings.cpp b/src/settings.cpp index f994c0910b..f374f5f2e4 100644 --- a/src/settings.cpp +++ b/src/settings.cpp @@ -856,16 +856,16 @@ if ( GetNumericIniSet ( IniXMLDocument, "server", "centservaddrtype", static_cas directoryType = static_cast ( iValue ); } else - // clang-format on - if ( GetNumericIniSet ( IniXMLDocument, - "server", - "directorytype", - static_cast ( AT_NONE ), - static_cast ( AT_CUSTOM ), - iValue ) ) - { - directoryType = static_cast ( iValue ); - } + // clang-format on + if ( GetNumericIniSet ( IniXMLDocument, + "server", + "directorytype", + static_cast ( AT_NONE ), + static_cast ( AT_CUSTOM ), + iValue ) ) + { + directoryType = static_cast ( iValue ); + } // clang-format off // TODO compatibility to old version < 3.9.0