Refactor Connect out to CClient

This is an extract from #2550
Co-authored-by: ann0see <[email protected]>

Author: pgScorpio

src/client.cpp

@@ -27,7 +27,6 @@
 /* Implementation *************************************************************/
 CClient::CClient ( const quint16  iPortNumber,
                    const quint16  iQosNumber,
-                   const QString& strConnOnStartupAddress,
                    const QString& strMIDISetup,
                    const bool     bNoAutoJackConnect,
                    const QString& strNClientName,
@@ -122,7 +121,7 @@ CClient::CClient ( const quint16  iPortNumber,
     QObject::connect ( &Channel, &CChannel::ConClientListMesReceived, this, &CClient::OnConClientListMesReceived );
     QObject::connect ( &Channel, &CChannel::ConClientListMesReceived, this, &CClient::ConClientListMesReceived );
 
-    QObject::connect ( &Channel, &CChannel::Disconnected, this, &CClient::Disconnected );
+    QObject::connect ( &Channel, &CChannel::Disconnected, this, &CClient::Stop );
 
     QObject::connect ( &Channel, &CChannel::NewConnection, this, &CClient::OnNewConnection );
 
@@ -184,13 +183,6 @@ CClient::CClient ( const quint16  iPortNumber,
     // start the socket (it is important to start the socket after all
     // initializations and connections)
     Socket.Start();
-
-    // do an immediate start if a server address is given
-    if ( !strConnOnStartupAddress.isEmpty() )
-    {
-        SetServerAddr ( strConnOnStartupAddress );
-        Start();
-    }
 }
 
 CClient::~CClient()
@@ -486,6 +478,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
@@ -773,11 +768,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();
@@ -872,8 +864,14 @@ void CClient::Start()
 
     // start audio interface
     Sound.Start();
+
+    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
@@ -906,6 +904,42 @@ void CClient::Stop()
     // reset current signal level and LEDs
     bJitterBufferOK = true;
     SignalLevelMeter.Reset();
+
+    // emit Disconnected() to inform UI of disconnection
+    emit Disconnected();
+}
+
+/// @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()

src/client.h

@@ -111,7 +111,6 @@ class CClient : public QObject
 public:
     CClient ( const quint16  iPortNumber,
               const quint16  iQosNumber,
-              const QString& strConnOnStartupAddress,
               const QString& strMIDISetup,
               const bool     bNoAutoJackConnect,
               const QString& strNClientName,
@@ -122,6 +121,12 @@ class CClient : public QObject
 
     void Start();
     void Stop();
+    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 );
@@ -288,6 +293,10 @@ class CClient : public QObject
     int  EvaluatePingMessage ( const int iMs );
     void CreateServerJitterBufferMessage();
 
+    // information for the connected server
+
+    QString strConnectedServerName;
+
     // only one channel is needed for client application
     CChannel  Channel;
     CProtocol ConnLessProtocol;
@@ -388,7 +397,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 );
@@ -428,7 +438,11 @@ protected slots:
 
     void CLChannelLevelListReceived ( CHostAddress InetAddr, CVector<uint16_t> 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 );

src/clientdlg.cpp

@@ -27,7 +27,6 @@
 /* Implementation *************************************************************/
 CClientDlg::CClientDlg ( CClient*         pNCliP,
                          CClientSettings* pNSetP,
-                         const QString&   strConnOnStartupAddress,
                          const QString&   strMIDISetup,
                          const bool       bNewShowComplRegConnList,
                          const bool       bShowAnalyzerConsole,
@@ -272,14 +271,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 );
 
@@ -483,7 +474,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 );
 
@@ -618,11 +613,8 @@ void CClientDlg::closeEvent ( QCloseEvent* Event )
     ConnectDlg.close();
     AnalyzerConsole.close();
 
-    // if connected, terminate connection
-    if ( pClient->IsRunning() )
-    {
-        pClient->Stop();
-    }
+    // Disconnect if needed
+    pClient->Stop();
 
     // make sure all current fader settings are applied to the settings struct
     MainMixerBoard->StoreAllFaderSettings();
@@ -740,15 +732,12 @@ 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->Stop();
 
         // initiate connection
-        Connect ( strSelectedAddress, strMixerBoardLabel );
+
+        pClient->Connect ( strSelectedAddress, strMixerBoardLabel );
 
         // reset flag
         bConnectDlgWasShown = false;
@@ -760,11 +749,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();
     }
 }
@@ -869,7 +859,7 @@ void CClientDlg::OnLicenceRequired ( ELicenceType eLicenceType )
         // disconnect from that server.
         if ( !LicenceDlg.exec() )
         {
-            Disconnect();
+            pClient->Stop();
         }
 
         // unmute the client output stream if local mute button is not pressed
@@ -1174,7 +1164,7 @@ void CClientDlg::OnSoundDeviceChanged ( QString strError )
         // the sound device setup has a problem, disconnect any active connection
         if ( pClient->IsRunning() )
         {
-            Disconnect();
+            pClient->Stop();
         }
 
         // show the error message of the device setup
@@ -1203,65 +1193,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, "Close", nullptr ); }
 
+void CClientDlg::OnDisconnect()
+{
     // change connect button text to "connect"
     butConnect->setText ( tr ( "C&onnect" ) );
 
@@ -1303,6 +1266,9 @@ void CClientDlg::Disconnect()
 
     // clear mixer board (remove all faders)
     MainMixerBoard->HideAll();
+
+    // Reset the deco
+    SetMixerBoardDeco ( RS_UNDEFINED, pClient->GetGUIDesign() );
 }
 
 void CClientDlg::UpdateDisplay()