Skip to content

Commit

Permalink
Connection status issue jamulussoftware#2519 first stage
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pgScorpio committed Apr 5, 2022
1 parent 3bb34f4 commit 9aeceb0
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 31 deletions.
12 changes: 7 additions & 5 deletions src/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ CClient::CClient ( const quint16 iPortNumber,
if ( !strConnOnStartupAddress.isEmpty() )
{
SetServerAddr ( strConnOnStartupAddress );
Start();
StartConnection(); // ---> pgScorpio: Was Start()
}
}

Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -819,7 +820,7 @@ void CClient::OnClientIDReceived ( int iChanID )
emit ClientIDReceived ( iChanID );
}

void CClient::Start()
void CClient::StartConnection() // ---> pgScorpio: Was Start()
{
// init object
Init();
Expand All @@ -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();
Expand Down
16 changes: 12 additions & 4 deletions src/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -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(); }
Expand Down
31 changes: 20 additions & 11 deletions src/clientdlg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
Expand All @@ -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() );
Expand Down Expand Up @@ -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,
Expand All @@ -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();
}
Expand Down Expand Up @@ -1190,16 +1191,18 @@ 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 ) )
{
// try to start client, if error occurred, do not go in
// running state but show error message
try
{
if ( !pClient->IsRunning() )
if ( !pClient->SoundIsStarted() ) // ---> pgScorpio: Again this is NOT connection started !() )
{
pClient->Start();
pClient->StartConnection();
}
}

Expand All @@ -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 );
Expand Down Expand Up @@ -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" ) );

Expand Down
2 changes: 1 addition & 1 deletion src/clientsettingsdlg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 ( "---" );
Expand Down
20 changes: 10 additions & 10 deletions src/settings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -858,16 +858,16 @@ if ( GetNumericIniSet ( IniXMLDocument, "server", "centservaddrtype", static_cas
directoryType = static_cast<EDirectoryType> ( iValue );
}
else
// clang-format on
if ( GetNumericIniSet ( IniXMLDocument,
"server",
"directorytype",
static_cast<int> ( AT_NONE ),
static_cast<int> ( AT_CUSTOM ),
iValue ) )
{
directoryType = static_cast<EDirectoryType> ( iValue );
}
// clang-format on
if ( GetNumericIniSet ( IniXMLDocument,
"server",
"directorytype",
static_cast<int> ( AT_NONE ),
static_cast<int> ( AT_CUSTOM ),
iValue ) )
{
directoryType = static_cast<EDirectoryType> ( iValue );
}

// clang-format off
// TODO compatibility to old version < 3.9.0
Expand Down

0 comments on commit 9aeceb0

Please sign in to comment.