From 43b5eb5258d621086c90f64bdb2a75d4bb945f7e Mon Sep 17 00:00:00 2001 From: pgScorpio Date: Fri, 13 Sep 2024 22:42:37 +0200 Subject: [PATCH 1/3] Refactor Connect out to CClient This is an extract from https://github.com/jamulussoftware/jamulus/pull/2550 Co-authored-by: ann0see <20726856+ann0see@users.noreply.github.com> --- src/client.cpp | 65 ++++++++++++++++++++++--- src/client.h | 17 ++++++- src/clientdlg.cpp | 118 ++++++++++++++++------------------------------ src/clientdlg.h | 7 ++- src/main.cpp | 19 +++++--- 5 files changed, 130 insertions(+), 96 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index fb9bab5478..65b713176d 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -145,7 +145,7 @@ CClient::CClient ( const quint16 iPortNumber, // The first ConClientListMesReceived handler performs the necessary cleanup and has to run first: QObject::connect ( &Channel, &CChannel::ConClientListMesReceived, this, &CClient::OnConClientListMesReceived ); - QObject::connect ( &Channel, &CChannel::Disconnected, this, &CClient::Disconnected ); + QObject::connect ( &Channel, &CChannel::Disconnected, this, &CClient::Stop ); QObject::connect ( &Channel, &CChannel::NewConnection, this, &CClient::OnNewConnection ); @@ -618,6 +618,9 @@ bool CClient::SetServerAddr ( QString strNAddr ) // apply address to the channel Channel.SetAddress ( HostAddress ); + // By default, set server name to HostAddress. If using the Connect() method, this may be overwritten + SetConnectedServerName ( HostAddress.toString() ); + return true; } else @@ -905,11 +908,8 @@ void CClient::OnHandledSignal ( int sigNum ) { case SIGINT: case SIGTERM: - // if connected, terminate connection (needed for headless mode) - if ( IsRunning() ) - { - Stop(); - } + // if connected, Stop client (needed for headless mode) + Stop(); // this should trigger OnAboutToQuit QCoreApplication::instance()->exit(); @@ -1045,8 +1045,14 @@ void CClient::Start() // Disable hibernation or display dimming if the app is running on Windows SetThreadExecutionState ( ES_CONTINUOUS | ES_SYSTEM_REQUIRED | ES_DISPLAY_REQUIRED ); #endif + + emit Connected ( GetConnectedServerName() ); } +/// @method +/// @brief Stops client and disconnects from server +/// @emit Disconnected +/// Use to set CClientDlg to show not being connected void CClient::Stop() { // stop audio interface @@ -1088,6 +1094,53 @@ void CClient::Stop() // Allow hibernation or display dimming if the app is running again (Windows) SetThreadExecutionState ( ES_CONTINUOUS ); #endif + + // emit Disconnected() to inform UI of disconnection + emit Disconnected(); +} + +/// @method +/// @brief Stops the client if the client is running +/// @emit Disconnected +void CClient::Disconnect() +{ + if ( IsRunning() ) + { + Stop(); + } +} + +/// @method +/// @brief Connects to strServerAddress +/// @emit Connected (strServerName) if the client wasn't running and SetServerAddr was valid. emit happens through Start(). +/// Use to set CClientDlg to show being connected +/// @emit ConnectingFailed (error) if an error occurred +/// Use to display error message in CClientDlg +/// @param strServerAddress - the server address to connect to +/// @param strServerName - the String argument to be passed to Connecting() +void CClient::Connect ( QString strServerAddress, QString strServerName ) +{ + try + { + if ( !IsRunning() ) + { + // Set server address and connect if valid address was supplied + if ( SetServerAddr ( strServerAddress ) ) + { + SetConnectedServerName ( strServerName ); + Start(); + } + else + { + throw CGenErr ( tr ( "Received invalid server address. Please check for typos in the provided server address." ) ); + } + } + } + catch ( const CGenErr& generr ) + { + Stop(); + emit ConnectingFailed ( generr.GetErrorText() ); + } } void CClient::Init() diff --git a/src/client.h b/src/client.h index fb77930723..50e51148cd 100644 --- a/src/client.h +++ b/src/client.h @@ -159,6 +159,13 @@ class CClient : public QObject void Start(); void Stop(); + void Disconnect(); + void Connect ( QString strServerAddress, QString strServerName ); + + // The ConnectedServerName is emitted by Connected() to update the UI with a human readable server name + void SetConnectedServerName ( const QString strServerName ) { strConnectedServerName = strServerName; }; + QString GetConnectedServerName() const { return strConnectedServerName; }; + bool IsRunning() { return Sound.IsRunning(); } bool IsCallbackEntered() const { return Sound.IsCallbackEntered(); } bool SetServerAddr ( QString strNAddr ); @@ -353,6 +360,9 @@ class CClient : public QObject void FreeClientChannel ( const int iServerChannelID ); int FindClientChannel ( const int iServerChannelID, const bool bCreateIfNew ); // returns a client channel ID or INVALID_INDEX bool ReorderLevelList ( CVector& vecLevelList ); // modifies vecLevelList, passed by reference + // information for the connected server + + QString strConnectedServerName; // only one channel is needed for client application CChannel Channel; @@ -464,7 +474,8 @@ protected slots: { if ( InetAddr == Channel.GetAddress() ) { - emit Disconnected(); + // Stop client in case it received a Disconnection request + Stop(); } } void OnCLPingReceived ( CHostAddress InetAddr, int iMs ); @@ -507,7 +518,11 @@ protected slots: void CLChannelLevelListReceived ( CHostAddress InetAddr, CVector vecLevelList ); + void Connected ( QString strServerName ); + void ConnectingFailed ( QString errorMessage ); + void DisconnectClient(); void Disconnected(); + void SoundDeviceChanged ( QString strError ); void ControllerInFaderLevel ( int iChannelIdx, int iValue ); void ControllerInPanValue ( int iChannelIdx, int iValue ); diff --git a/src/clientdlg.cpp b/src/clientdlg.cpp index b102c34724..0a66e56618 100644 --- a/src/clientdlg.cpp +++ b/src/clientdlg.cpp @@ -50,7 +50,6 @@ /* Implementation *************************************************************/ CClientDlg::CClientDlg ( CClient* pNCliP, CClientSettings* pNSetP, - const QString& strConnOnStartupAddress, const bool bNewShowComplRegConnList, const bool bShowAnalyzerConsole, const bool bMuteStream, @@ -292,14 +291,6 @@ CClientDlg::CClientDlg ( CClient* pNCliP, TimerCheckAudioDeviceOk.setSingleShot ( true ); // only check once after connection TimerDetectFeedback.setSingleShot ( true ); - // Connect on startup ------------------------------------------------------ - if ( !strConnOnStartupAddress.isEmpty() ) - { - // initiate connection (always show the address in the mixer board - // (no alias)) - Connect ( strConnOnStartupAddress, strConnOnStartupAddress ); - } - // File menu -------------------------------------------------------------- QMenu* pFileMenu = new QMenu ( tr ( "&File" ), this ); @@ -507,7 +498,11 @@ CClientDlg::CClientDlg ( CClient* pNCliP, // other QObject::connect ( pClient, &CClient::ConClientListMesReceived, this, &CClientDlg::OnConClientListMesReceived ); - QObject::connect ( pClient, &CClient::Disconnected, this, &CClientDlg::OnDisconnected ); + QObject::connect ( pClient, &CClient::Connected, this, &CClientDlg::OnConnect ); + + QObject::connect ( pClient, &CClient::ConnectingFailed, this, &CClientDlg::OnConnectingFailed ); + + QObject::connect ( pClient, &CClient::Disconnected, this, &CClientDlg::OnDisconnect ); QObject::connect ( pClient, &CClient::ChatTextReceived, this, &CClientDlg::OnChatTextReceived ); @@ -646,11 +641,8 @@ void CClientDlg::closeEvent ( QCloseEvent* Event ) ConnectDlg.close(); AnalyzerConsole.close(); - // if connected, terminate connection - if ( pClient->IsRunning() ) - { - pClient->Stop(); - } + // Disconnect if needed + pClient->Disconnect(); // make sure all current fader settings are applied to the settings struct MainMixerBoard->StoreAllFaderSettings(); @@ -768,15 +760,11 @@ 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() ) - { - Disconnect(); - } + // Disconnect the client. We could be currently connected. + pClient->Disconnect(); // initiate connection - Connect ( strSelectedAddress, strMixerBoardLabel ); + pClient->Connect ( strSelectedAddress, strMixerBoardLabel ); // reset flag bConnectDlgWasShown = false; @@ -788,11 +776,12 @@ void CClientDlg::OnConnectDisconBut() // the connect/disconnect button implements a toggle functionality if ( pClient->IsRunning() ) { - Disconnect(); - SetMixerBoardDeco ( RS_UNDEFINED, pClient->GetGUIDesign() ); + pClient->Stop(); } else { + // If the client isn't running, we assume that we weren't connected. Thus show the connect dialog + // TODO: Refactor to have robust error handling ShowConnectionSetupDialog(); } } @@ -904,7 +893,7 @@ void CClientDlg::OnLicenceRequired ( ELicenceType eLicenceType ) // disconnect from that server. if ( !LicenceDlg.exec() ) { - Disconnect(); + pClient->Disconnect(); } // unmute the client output stream if local mute button is not pressed @@ -1205,10 +1194,7 @@ void CClientDlg::OnSoundDeviceChanged ( QString strError ) if ( !strError.isEmpty() ) { // the sound device setup has a problem, disconnect any active connection - if ( pClient->IsRunning() ) - { - Disconnect(); - } + pClient->Disconnect(); // show the error message of the device setup QMessageBox::critical ( this, APP_NAME, strError, tr ( "Ok" ), nullptr ); @@ -1236,65 +1222,38 @@ void CClientDlg::OnCLPingTimeWithNumClientsReceived ( CHostAddress InetAddr, int ConnectDlg.SetPingTimeAndNumClientsResult ( InetAddr, iPingTime, iNumClients ); } -void CClientDlg::Connect ( const QString& strSelectedAddress, const QString& strMixerBoardLabel ) +void CClientDlg::OnConnect ( const QString& strMixerBoardLabel ) { - // set address and check if address is valid - if ( pClient->SetServerAddr ( strSelectedAddress ) ) - { - // try to start client, if error occurred, do not go in - // running state but show error message - try - { - if ( !pClient->IsRunning() ) - { - pClient->Start(); - } - } - - catch ( const CGenErr& generr ) - { - // show error message and return the function - QMessageBox::critical ( this, APP_NAME, generr.GetErrorText(), "Close", nullptr ); - return; - } - // hide label connect to server - lblConnectToServer->hide(); - lbrInputLevelL->setEnabled ( true ); - lbrInputLevelR->setEnabled ( true ); + // hide label connect to server + lblConnectToServer->hide(); + lbrInputLevelL->setEnabled ( true ); + lbrInputLevelR->setEnabled ( true ); - // change connect button text to "disconnect" - butConnect->setText ( tr ( "&Disconnect" ) ); + // change connect button text to "disconnect" + butConnect->setText ( tr ( "&Disconnect" ) ); - // set server name in audio mixer group box title - MainMixerBoard->SetServerName ( strMixerBoardLabel ); + // set server name in audio mixer group box title + MainMixerBoard->SetServerName ( strMixerBoardLabel ); - // start timer for level meter bar and ping time measurement - TimerSigMet.start ( LEVELMETER_UPDATE_TIME_MS ); - TimerBuffersLED.start ( BUFFER_LED_UPDATE_TIME_MS ); - TimerPing.start ( PING_UPDATE_TIME_MS ); - TimerCheckAudioDeviceOk.start ( CHECK_AUDIO_DEV_OK_TIME_MS ); // is single shot timer + // start timer for level meter bar and ping time measurement + TimerSigMet.start ( LEVELMETER_UPDATE_TIME_MS ); + TimerBuffersLED.start ( BUFFER_LED_UPDATE_TIME_MS ); + TimerPing.start ( PING_UPDATE_TIME_MS ); + TimerCheckAudioDeviceOk.start ( CHECK_AUDIO_DEV_OK_TIME_MS ); // is single shot timer - // audio feedback detection - if ( pSettings->bEnableFeedbackDetection ) - { - TimerDetectFeedback.start ( DETECT_FEEDBACK_TIME_MS ); // single shot timer - bDetectFeedback = true; - } + // audio feedback detection + if ( pSettings->bEnableFeedbackDetection ) + { + TimerDetectFeedback.start ( DETECT_FEEDBACK_TIME_MS ); // single shot timer + bDetectFeedback = true; } } -void CClientDlg::Disconnect() -{ - // 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() ) - { - pClient->Stop(); - } +void CClientDlg::OnConnectingFailed ( const QString& strError ) { QMessageBox::critical ( this, APP_NAME, strError, tr ( "Close" ), nullptr ); } +void CClientDlg::OnDisconnect() +{ // change connect button text to "connect" butConnect->setText ( tr ( "C&onnect" ) ); @@ -1336,6 +1295,9 @@ void CClientDlg::Disconnect() // clear mixer board (remove all faders) MainMixerBoard->HideAll(); + + // Reset the deco + SetMixerBoardDeco ( RS_UNDEFINED, pClient->GetGUIDesign() ); } void CClientDlg::UpdateDisplay() diff --git a/src/clientdlg.h b/src/clientdlg.h index 6a34cc66f0..412b9a847e 100644 --- a/src/clientdlg.h +++ b/src/clientdlg.h @@ -100,7 +100,6 @@ class CClientDlg : public CBaseDlg, private Ui_CClientDlgBase public: CClientDlg ( CClient* pNCliP, CClientSettings* pNSetP, - const QString& strConnOnStartupAddress, const bool bNewShowComplRegConnList, const bool bShowAnalyzerConsole, const bool bMuteStream, @@ -116,8 +115,6 @@ class CClientDlg : public CBaseDlg, private Ui_CClientDlgBase void ShowAnalyzerConsole(); void UpdateAudioFaderSlider(); void UpdateRevSelection(); - void Connect ( const QString& strSelectedAddress, const QString& strMixerBoardLabel ); - void Disconnect(); void ManageDragNDrop ( QDropEvent* Event, const bool bCheckAccept ); void SetPingTime ( const int iPingTime, const int iOverallDelayMs, const CMultiColorLED::ELightColor eOverallDelayLEDColor ); @@ -148,6 +145,9 @@ class CClientDlg : public CBaseDlg, private Ui_CClientDlgBase CAnalyzerConsole AnalyzerConsole; public slots: + void OnConnect ( const QString& strServerName ); + void OnConnectingFailed ( const QString& strErrorText ); + void OnDisconnect(); void OnConnectDisconBut(); void OnTimerSigMet(); void OnTimerBuffersLED(); @@ -254,7 +254,6 @@ public slots: } void OnConnectDlgAccepted(); - void OnDisconnected() { Disconnect(); } void OnGUIDesignChanged(); void OnMeterStyleChanged(); void OnRecorderStateReceived ( ERecorderState eRecorderState ); diff --git a/src/main.cpp b/src/main.cpp index a518c2e515..77dabdb902 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1005,26 +1005,31 @@ int main ( int argc, char** argv ) } // GUI object - CClientDlg - ClientDlg ( &Client, &Settings, strConnOnStartupAddress, bShowComplRegConnList, bShowAnalyzerConsole, bMuteStream, nullptr ); + CClientDlg ClientDlg ( &Client, &Settings, bShowComplRegConnList, bShowAnalyzerConsole, bMuteStream, nullptr ); // show dialog ClientDlg.show(); + // Connect on startup if requested + if ( !strConnOnStartupAddress.isEmpty() ) + { + Client.Connect ( strConnOnStartupAddress, strConnOnStartupAddress ); + } + pApp->exec(); } else # endif { + // only start application without using the GUI + qInfo() << qUtf8Printable ( GetVersionAndNameStr ( false ) ); + + // Connect on startup if requested if ( !strConnOnStartupAddress.isEmpty() ) { - Client.SetServerAddr ( strConnOnStartupAddress ); - Client.Start(); + Client.Connect ( strConnOnStartupAddress, strConnOnStartupAddress ); } - // only start application without using the GUI - qInfo() << qUtf8Printable ( GetVersionAndNameStr ( false ) ); - pApp->exec(); } } From 1b49a78f5c156cbf744aed7866795f22f814c47d Mon Sep 17 00:00:00 2001 From: jrd Date: Sun, 19 Jul 2026 01:15:49 +0000 Subject: [PATCH 2/3] Add explicit client connection state machine Introduce EConnectionState (disconnected / connecting / connected) owned by CClient as the single source of truth. A connection is 'requested' when the audio stream starts (CS_CONNECTING) and 'established' once the server assigns our channel ID (CS_CONNECTED). Every transition emits ConnectionStateChanged. Rename the Connected(name) signal emitted from Start() to Connecting(name), since at that point the connection is only requested, not established. CClient::Connect() now terminates any current connection first, so connecting while connected behaves as a reconnect. Co-Authored-By: Claude Fable 5 --- src/client.cpp | 52 +++++++++++++++++++++++++++++++++-------------- src/client.h | 15 ++++++++++---- src/clientdlg.cpp | 11 ++++------ src/clientdlg.h | 2 +- src/util.h | 11 ++++++++++ 5 files changed, 64 insertions(+), 27 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index 65b713176d..ab3c5821a6 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -59,6 +59,7 @@ CClient::CClient ( const quint16 iPortNumber, strClientName ( strNClientName ), pSignalHandler ( CSignalHandler::getSingletonP() ), pSettings ( nullptr ), + eConnectionState ( CS_DISCONNECTED ), Channel ( false ), /* we need a client channel -> "false" */ CurOpusEncoder ( nullptr ), CurOpusDecoder ( nullptr ), @@ -1003,6 +1004,9 @@ void CClient::OnClientIDReceived ( int iServerChanID ) SetRemoteChanGain ( iChanID, 0, false ); } + // the server has assigned us a channel ID, so the connection is established + SetConnectionState ( CS_CONNECTED ); + emit ClientIDReceived ( iChanID ); } @@ -1046,7 +1050,12 @@ void CClient::Start() SetThreadExecutionState ( ES_CONTINUOUS | ES_SYSTEM_REQUIRED | ES_DISPLAY_REQUIRED ); #endif - emit Connected ( GetConnectedServerName() ); + // the connection is requested now but not yet established: the transition + // to CS_CONNECTED happens when the server assigns our channel ID + // (see OnClientIDReceived) + SetConnectionState ( CS_CONNECTING ); + + emit Connecting ( GetConnectedServerName() ); } /// @method @@ -1095,6 +1104,8 @@ void CClient::Stop() SetThreadExecutionState ( ES_CONTINUOUS ); #endif + SetConnectionState ( CS_DISCONNECTED ); + // emit Disconnected() to inform UI of disconnection emit Disconnected(); } @@ -1111,29 +1122,31 @@ void CClient::Disconnect() } /// @method -/// @brief Connects to strServerAddress -/// @emit Connected (strServerName) if the client wasn't running and SetServerAddr was valid. emit happens through Start(). +/// @brief Connects to strServerAddress. If a connection is currently requested +/// or established, that connection is terminated first. +/// @emit Connecting (strServerName) if SetServerAddr was valid. emit happens through Start(). /// Use to set CClientDlg to show being connected /// @emit ConnectingFailed (error) if an error occurred /// Use to display error message in CClientDlg /// @param strServerAddress - the server address to connect to -/// @param strServerName - the String argument to be passed to Connecting() +/// @param strServerName - the human readable server name passed to Connecting() void CClient::Connect ( QString strServerAddress, QString strServerName ) { try { - if ( !IsRunning() ) + // disconnect from any current server first so that connecting to a + // different server while connected behaves as a reconnect + Disconnect(); + + // Set server address and connect if valid address was supplied + if ( SetServerAddr ( strServerAddress ) ) { - // Set server address and connect if valid address was supplied - if ( SetServerAddr ( strServerAddress ) ) - { - SetConnectedServerName ( strServerName ); - Start(); - } - else - { - throw CGenErr ( tr ( "Received invalid server address. Please check for typos in the provided server address." ) ); - } + SetConnectedServerName ( strServerName ); + Start(); + } + else + { + throw CGenErr ( tr ( "Received invalid server address. Please check for typos in the provided server address." ) ); } } catch ( const CGenErr& generr ) @@ -1143,6 +1156,15 @@ void CClient::Connect ( QString strServerAddress, QString strServerName ) } } +void CClient::SetConnectionState ( const EConnectionState eNewConnectionState ) +{ + if ( eConnectionState != eNewConnectionState ) + { + eConnectionState = eNewConnectionState; + emit ConnectionStateChanged ( eConnectionState ); + } +} + void CClient::Init() { // check if possible frame size factors are supported diff --git a/src/client.h b/src/client.h index 50e51148cd..c275c82b00 100644 --- a/src/client.h +++ b/src/client.h @@ -162,10 +162,12 @@ class CClient : public QObject void Disconnect(); void Connect ( QString strServerAddress, QString strServerName ); - // The ConnectedServerName is emitted by Connected() to update the UI with a human readable server name + // The ConnectedServerName is emitted by Connecting() to update the UI with a human readable server name void SetConnectedServerName ( const QString strServerName ) { strConnectedServerName = strServerName; }; QString GetConnectedServerName() const { return strConnectedServerName; }; + EConnectionState GetConnectionState() const { return eConnectionState; } + bool IsRunning() { return Sound.IsRunning(); } bool IsCallbackEntered() const { return Sound.IsCallbackEntered(); } bool SetServerAddr ( QString strNAddr ); @@ -361,9 +363,14 @@ class CClient : public QObject int FindClientChannel ( const int iServerChannelID, const bool bCreateIfNew ); // returns a client channel ID or INVALID_INDEX bool ReorderLevelList ( CVector& vecLevelList ); // modifies vecLevelList, passed by reference // information for the connected server - QString strConnectedServerName; + // single source of truth for the connection state, only to be modified + // via SetConnectionState() so that every change emits ConnectionStateChanged() + EConnectionState eConnectionState; + + void SetConnectionState ( const EConnectionState eNewConnectionState ); + // only one channel is needed for client application CChannel Channel; CProtocol ConnLessProtocol; @@ -518,9 +525,9 @@ protected slots: void CLChannelLevelListReceived ( CHostAddress InetAddr, CVector vecLevelList ); - void Connected ( QString strServerName ); + void ConnectionStateChanged ( EConnectionState eConnectionState ); + void Connecting ( QString strServerName ); void ConnectingFailed ( QString errorMessage ); - void DisconnectClient(); void Disconnected(); void SoundDeviceChanged ( QString strError ); diff --git a/src/clientdlg.cpp b/src/clientdlg.cpp index 0a66e56618..fe29c47847 100644 --- a/src/clientdlg.cpp +++ b/src/clientdlg.cpp @@ -498,7 +498,7 @@ CClientDlg::CClientDlg ( CClient* pNCliP, // other QObject::connect ( pClient, &CClient::ConClientListMesReceived, this, &CClientDlg::OnConClientListMesReceived ); - QObject::connect ( pClient, &CClient::Connected, this, &CClientDlg::OnConnect ); + QObject::connect ( pClient, &CClient::Connecting, this, &CClientDlg::OnConnecting ); QObject::connect ( pClient, &CClient::ConnectingFailed, this, &CClientDlg::OnConnectingFailed ); @@ -760,10 +760,7 @@ void CClientDlg::OnConnectDlgAccepted() } } - // Disconnect the client. We could be currently connected. - pClient->Disconnect(); - - // initiate connection + // initiate connection (terminates any current connection first) pClient->Connect ( strSelectedAddress, strMixerBoardLabel ); // reset flag @@ -776,7 +773,7 @@ void CClientDlg::OnConnectDisconBut() // the connect/disconnect button implements a toggle functionality if ( pClient->IsRunning() ) { - pClient->Stop(); + pClient->Disconnect(); } else { @@ -1222,7 +1219,7 @@ void CClientDlg::OnCLPingTimeWithNumClientsReceived ( CHostAddress InetAddr, int ConnectDlg.SetPingTimeAndNumClientsResult ( InetAddr, iPingTime, iNumClients ); } -void CClientDlg::OnConnect ( const QString& strMixerBoardLabel ) +void CClientDlg::OnConnecting ( const QString& strMixerBoardLabel ) { // hide label connect to server diff --git a/src/clientdlg.h b/src/clientdlg.h index 412b9a847e..b127231b27 100644 --- a/src/clientdlg.h +++ b/src/clientdlg.h @@ -145,7 +145,7 @@ class CClientDlg : public CBaseDlg, private Ui_CClientDlgBase CAnalyzerConsole AnalyzerConsole; public slots: - void OnConnect ( const QString& strServerName ); + void OnConnecting ( const QString& strServerName ); void OnConnectingFailed ( const QString& strErrorText ); void OnDisconnect(); void OnConnectDisconBut(); diff --git a/src/util.h b/src/util.h index 1185c56936..dc1afb0bae 100644 --- a/src/util.h +++ b/src/util.h @@ -590,6 +590,17 @@ enum ERecorderState RS_RECORDING = 3 }; +// Client connection state ----------------------------------------------------- +enum EConnectionState +{ + // a connection is "requested" as soon as the client starts the audio + // stream towards the configured server and "established" once the server + // has assigned a channel ID to the client + CS_DISCONNECTED = 0, // no connection requested or established + CS_CONNECTING = 1, // connection requested but not yet established + CS_CONNECTED = 2 // connection established and current +}; + // Channel sort type ----------------------------------------------------------- enum EChSortType { From efee722b99a488d86dec1160732a25d18f4885c5 Mon Sep 17 00:00:00 2001 From: jrd Date: Tue, 21 Jul 2026 05:55:18 +0000 Subject: [PATCH 3/3] client: address review feedback on the connection-state refactor - SIGTERM/SIGINT: route through Disconnect() instead of a raw Stop(), and guard Disconnect() on the connection state rather than IsRunning() (which tracks the audio device). IsRunning() is false while connecting and in headless mode, so the old guard could skip notifying the server on shutdown; the raw Stop() worked around that but also fired a spurious disconnect when idle. Now the server is notified iff a connection is pending or established, via the single Disconnect() path. - Move EConnectionState from util.h to client.h (it is client specific). - Add a doc-comment to SetConnectionState; reword two member comments. Addresses review feedback on #3805. Co-Authored-By: Claude Opus 4.8 --- src/client.cpp | 18 +++++++++++++----- src/client.h | 16 +++++++++++++--- src/util.h | 11 ----------- 3 files changed, 26 insertions(+), 19 deletions(-) diff --git a/src/client.cpp b/src/client.cpp index ab3c5821a6..94e26a25e1 100644 --- a/src/client.cpp +++ b/src/client.cpp @@ -909,10 +909,10 @@ void CClient::OnHandledSignal ( int sigNum ) { case SIGINT: case SIGTERM: - // if connected, Stop client (needed for headless mode) - Stop(); + // tear down any pending or established connection first, so the server + // is notified we are leaving (Disconnect() is a no-op if not connected) + Disconnect(); - // this should trigger OnAboutToQuit QCoreApplication::instance()->exit(); break; @@ -1111,11 +1111,15 @@ void CClient::Stop() } /// @method -/// @brief Stops the client if the client is running +/// @brief Disconnects from the server if a connection is requested or established. +/// Idempotent: a no-op when already disconnected. /// @emit Disconnected void CClient::Disconnect() { - if ( IsRunning() ) + // Key off the connection state, not IsRunning() (which tracks the audio + // device): the two diverge while connecting and in headless mode, and on + // SIGTERM we must still send the disconnect message to the server. + if ( GetConnectionState() != CS_DISCONNECTED ) { Stop(); } @@ -1156,6 +1160,10 @@ void CClient::Connect ( QString strServerAddress, QString strServerName ) } } +/// @method +/// @brief Updates the connection state and, if it changed, notifies observers. +/// @emit ConnectionStateChanged (state) when the state actually changes +/// @param eNewConnectionState - the state to transition to void CClient::SetConnectionState ( const EConnectionState eNewConnectionState ) { if ( eConnectionState != eNewConnectionState ) diff --git a/src/client.h b/src/client.h index c275c82b00..33cb4373d2 100644 --- a/src/client.h +++ b/src/client.h @@ -88,6 +88,17 @@ #endif /* Definitions ****************************************************************/ +// Client connection state ----------------------------------------------------- +enum EConnectionState +{ + // a connection is "requested" as soon as the client starts the audio + // stream towards the configured server and "established" once the server + // has assigned a channel ID to the client + CS_DISCONNECTED = 0, // no connection requested or established + CS_CONNECTING = 1, // connection requested but not yet established + CS_CONNECTED = 2 // connection established and current +}; + // audio in fader range #define AUD_FADER_IN_MIN 0 #define AUD_FADER_IN_MAX 100 @@ -362,11 +373,10 @@ class CClient : public QObject void FreeClientChannel ( const int iServerChannelID ); int FindClientChannel ( const int iServerChannelID, const bool bCreateIfNew ); // returns a client channel ID or INVALID_INDEX bool ReorderLevelList ( CVector& vecLevelList ); // modifies vecLevelList, passed by reference - // information for the connected server + // human-readable name of the current server, shown in the UI QString strConnectedServerName; - // single source of truth for the connection state, only to be modified - // via SetConnectionState() so that every change emits ConnectionStateChanged() + // current connection state; set only via SetConnectionState() EConnectionState eConnectionState; void SetConnectionState ( const EConnectionState eNewConnectionState ); diff --git a/src/util.h b/src/util.h index dc1afb0bae..1185c56936 100644 --- a/src/util.h +++ b/src/util.h @@ -590,17 +590,6 @@ enum ERecorderState RS_RECORDING = 3 }; -// Client connection state ----------------------------------------------------- -enum EConnectionState -{ - // a connection is "requested" as soon as the client starts the audio - // stream towards the configured server and "established" once the server - // has assigned a channel ID to the client - CS_DISCONNECTED = 0, // no connection requested or established - CS_CONNECTING = 1, // connection requested but not yet established - CS_CONNECTED = 2 // connection established and current -}; - // Channel sort type ----------------------------------------------------------- enum EChSortType {