Skip to content
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

Server UI changes #2199

Merged
merged 1 commit into from
Feb 23, 2022
Merged

Server UI changes #2199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
110 changes: 51 additions & 59 deletions src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,9 @@ int main ( int argc, char** argv )
}
#endif

// TODO create settings in default state, if loading from file do that next, then come back here to
// override from command line options, then create client or server, letting them do the validation

if ( bIsClient )
{
if ( ServerOnlyOptions.size() != 0 )
Expand Down Expand Up @@ -593,65 +596,32 @@ int main ( int argc, char** argv )

if ( bUseGUI )
{
if ( strDirectoryServer.isEmpty() )
{
// per definition: if we are in "GUI" server mode and no directory server
// address is given, we use the default directory server address
strDirectoryServer = DEFAULT_SERVER_ADDRESS;
qInfo() << qUtf8Printable ( QString ( "- default directory server set: %1" ).arg ( strDirectoryServer ) );
}
}
else
{
// the inifile is not supported for the headless server mode
if ( !strIniFileName.isEmpty() )
{
qWarning() << "No initialization file support in headless server mode.";
}
}

if ( strDirectoryServer.isEmpty() )
{
// per definition, we must be a headless server and ignoring inifile, so we have all the information
// by definition, when running with the GUI we always default to registering somewhere but
Copy link
Member

Choose a reason for hiding this comment

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

This comment suggests that you can't have an unregistered ("private") server when running with the GUI. That's not true, surely (maybe it used to be?). Could the comment be clarified?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, it says what it defaults to, not what's available.

Copy link
Member

Choose a reason for hiding this comment

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

I have just tried this code for the first time (in fact it was the code for #2427 I tried, which incorporates these changes), and compared with 3.8.2, using default empty ini files to ensure all defaults were used:

/usr/local/bin/Jamulus -s -i /tmp/x.ini
./Jamulus -s -p 22125 -i /tmp/y.ini

The 3.8.2 defaulted to "Not registered", with the "make my server public" checkbox unticked, which is good.

The newer version defaulted to registering with Any Genre 1, which I think is not good. The directory drop-down should default to "None", if not set in the command line or ini file, so that a new server does not register with a directory until the user asks it to.

// until the settings are loaded we do not know where, so we cannot be prescriptive here

if ( !strServerListFileName.isEmpty() )
{
qWarning() << "Server list persistence file will only take effect when running as a directory server.";
strServerListFileName = "";
qInfo() << "Note:"
<< "Server list persistence file will only take effect when running as a directory server.";
}

if ( !strServerListFilter.isEmpty() )
{
qWarning() << "Server list filter will only take effect when running as a directory server.";
strServerListFilter = "";
}

if ( !strServerPublicIP.isEmpty() )
{
qWarning() << "Server Public IP will only take effect when registering a server with a directory server.";
strServerPublicIP = "";
qInfo() << "Note:"
<< "Server list filter will only take effect when running as a directory server.";
}
}
else
{
// either we are not headless and there is an inifile, or a directory server was supplied on the command line

// if we are not headless, certain checks cannot be made, as the inifile state is not yet known
if ( !bUseGUI && strDirectoryServer.compare ( "localhost", Qt::CaseInsensitive ) != 0 && strDirectoryServer.compare ( "127.0.0.1" ) != 0 )
// the inifile is not supported for the headless server mode
if ( !strIniFileName.isEmpty() )
{
if ( !strServerListFileName.isEmpty() )
{
qWarning() << "Server list persistence file will only take effect when running as a directory server.";
strServerListFileName = "";
}

if ( !strServerListFilter.isEmpty() )
{
qWarning() << "Server list filter will only take effect when running as a directory server.";
strServerListFileName = "";
}
qWarning() << "No initialization file support in headless server mode.";
pljones marked this conversation as resolved.
Show resolved Hide resolved
strIniFileName = "";
}
else
// therefore we know everything based on command line options

if ( strDirectoryServer.compare ( "localhost", Qt::CaseInsensitive ) == 0 || strDirectoryServer.compare ( "127.0.0.1" ) == 0 )
{
if ( !strServerListFileName.isEmpty() )
{
Expand Down Expand Up @@ -709,16 +679,41 @@ int main ( int argc, char** argv )
}
}
}
else
{
if ( !strServerListFileName.isEmpty() )
{
qWarning() << "Server list persistence file will only take effect when running as a directory server.";
strServerListFileName = "";
}

if ( !strServerListFilter.isEmpty() )
{
qWarning() << "Server list filter will only take effect when running as a directory server.";
strServerListFileName = "";
}
}

if ( !strServerPublicIP.isEmpty() )
if ( strDirectoryServer.isEmpty() )
{
QHostAddress InetAddr;
if ( !InetAddr.setAddress ( strServerPublicIP ) )
if ( !strServerPublicIP.isEmpty() )
{
qWarning() << "Server Public IP is invalid. Only plain IP addresses are supported.";
qWarning() << "Server Public IP will only take effect when registering a server with a directory server.";
strServerPublicIP = "";
}
}
else
{
if ( !strServerPublicIP.isEmpty() )
{
QHostAddress InetAddr;
if ( !InetAddr.setAddress ( strServerPublicIP ) )
{
qWarning() << "Server Public IP is invalid. Only plain IP addresses are supported.";
strServerPublicIP = "";
}
}
}
}

if ( !strServerBindIP.isEmpty() )
Expand Down Expand Up @@ -881,10 +876,6 @@ int main ( int argc, char** argv )
CLocale::LoadTranslation ( Settings.strLanguage, pApp );
}

// update server list AFTER restoring the settings from the
// settings file
Server.UpdateServerList();

// GUI object for the server
CServerDlg ServerDlg ( &Server, &Settings, bStartMinimized, nullptr );

Expand All @@ -902,11 +893,12 @@ int main ( int argc, char** argv )
// only start application without using the GUI
qInfo() << qUtf8Printable ( GetVersionAndNameStr ( false ) );

// enable server list if a directory server is defined
Server.SetServerRegistered ( !strDirectoryServer.isEmpty() );

// update serverlist
Server.UpdateServerList();
// CServerListManager defaults to AT_NONE, so need to switch if
// strDirectoryServer is wanted
if ( !strDirectoryServer.isEmpty() )
{
Server.SetDirectoryType ( AT_CUSTOM );
}

pApp->exec();
}
Expand Down
6 changes: 0 additions & 6 deletions src/server.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -684,12 +684,6 @@ void CServer::OnAboutToQuit()

Stop();

Copy link
Member

Choose a reason for hiding this comment

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

Was it deliberate to remove the if ( GetServerRegistered() ) { Unregister(); } here? I can't see it has moved to anywhere else.

Copy link
Member

Choose a reason for hiding this comment

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

9222c20 was the original change doing this and ServerListManager.SetDirectoryType() seems to handle Unregister() if necessary: https://github.com/jamulussoftware/jamulus/blob/master/src/serverlist.cpp#L269-L275

It's called in https://github.com/jamulussoftware/jamulus/pull/2199/files#diff-c044ba02ccf7862f1e2f56436d5b92baa7bb35601791846655fb07ac253a8697R951

So this looks fine to me.

// if server was registered at the directory server, unregister on shutdown
if ( GetServerRegistered() )
{
Unregister();
}

if ( bWriteStatusHTMLFile )
{
WriteHTMLServerQuit();
Expand Down
85 changes: 33 additions & 52 deletions src/server.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,70 +190,50 @@ class CServer : public QObject, public CServerSlots<MAX_NUM_CHANNELS>

void CreateCLServerListReqVerAndOSMes ( const CHostAddress& InetAddr ) { ConnLessProtocol.CreateCLReqVersionAndOSMes ( InetAddr ); }

// Jam recorder ------------------------------------------------------------
// IPv6 Enabled
bool IsIPv6Enabled() { return bEnableIPv6; }

// GUI settings ------------------------------------------------------------
int GetClientNumAudioChannels ( const int iChanNum ) { return vecChannels[iChanNum].GetNumAudioChannels(); }

void SetDirectoryType ( const EDirectoryType eNCSAT ) { ServerListManager.SetDirectoryType ( eNCSAT ); }
EDirectoryType GetDirectoryType() { return ServerListManager.GetDirectoryType(); }
bool IsDirectoryServer() { return ServerListManager.IsDirectoryServer(); }
ESvrRegStatus GetSvrRegStatus() { return ServerListManager.GetSvrRegStatus(); }

void SetServerName ( const QString& strNewName ) { ServerListManager.SetServerName ( strNewName ); }
QString GetServerName() { return ServerListManager.GetServerName(); }
void SetServerCity ( const QString& strNewCity ) { ServerListManager.SetServerCity ( strNewCity ); }
QString GetServerCity() { return ServerListManager.GetServerCity(); }
void SetServerCountry ( const QLocale::Country eNewCountry ) { ServerListManager.SetServerCountry ( eNewCountry ); }
QLocale::Country GetServerCountry() { return ServerListManager.GetServerCountry(); }

bool GetRecorderInitialised() { return JamController.GetRecorderInitialised(); }
void SetEnableRecording ( bool bNewEnableRecording );
bool GetDisableRecording() { return bDisableRecording; }
QString GetRecorderErrMsg() { return JamController.GetRecorderErrMsg(); }
bool GetRecordingEnabled() { return JamController.GetRecordingEnabled(); }
bool GetDisableRecording() { return bDisableRecording; }
void RequestNewRecording() { JamController.RequestNewRecording(); }

void SetEnableRecording ( bool bNewEnableRecording );

QString GetRecordingDir() { return JamController.GetRecordingDir(); }

void SetRecordingDir ( QString newRecordingDir )
void SetRecordingDir ( QString newRecordingDir )
{
JamController.SetRecordingDir ( newRecordingDir, iServerFrameSizeSamples, bDisableRecording );
}

void CreateAndSendRecorderStateForAllConChannels();

// delay panning
void SetEnableDelayPanning ( bool bDelayPanningOn ) { bDelayPan = bDelayPanningOn; }
bool IsDelayPanningEnabled() { return bDelayPan; }

// IPv6 Enabled
bool IsIPv6Enabled() { return bEnableIPv6; }

// Server list management --------------------------------------------------
void UpdateServerList() { ServerListManager.Update(); }

void Unregister() { ServerListManager.Unregister(); }

void SetServerRegistered ( const bool bState ) { ServerListManager.SetEnabled ( bState ); }

bool GetServerRegistered() { return ServerListManager.GetEnabled(); }

void SetDirectoryAddress ( const QString& sNDirectoryAddress ) { ServerListManager.SetDirectoryAddress ( sNDirectoryAddress ); }

QString GetDirectoryAddress() { return ServerListManager.GetDirectoryAddress(); }

void SetDirectoryType ( const EDirectoryType eNCSAT ) { ServerListManager.SetDirectoryType ( eNCSAT ); }

EDirectoryType GetDirectoryType() { return ServerListManager.GetDirectoryType(); }

void SetServerName ( const QString& strNewName ) { ServerListManager.SetServerName ( strNewName ); }

QString GetServerName() { return ServerListManager.GetServerName(); }

void SetServerCity ( const QString& strNewCity ) { ServerListManager.SetServerCity ( strNewCity ); }

QString GetServerCity() { return ServerListManager.GetServerCity(); }

void SetServerCountry ( const QLocale::Country eNewCountry ) { ServerListManager.SetServerCountry ( eNewCountry ); }

QLocale::Country GetServerCountry() { return ServerListManager.GetServerCountry(); }
QString GetRecordingDir() { return JamController.GetRecordingDir(); }

void SetWelcomeMessage ( const QString& strNWelcMess );
QString GetWelcomeMessage() { return strWelcomeMessage; }

ESvrRegStatus GetSvrRegStatus() { return ServerListManager.GetSvrRegStatus(); }
void SetDirectoryAddress ( const QString& sNDirectoryAddress ) { ServerListManager.SetDirectoryAddress ( sNDirectoryAddress ); }
QString GetDirectoryAddress() { return ServerListManager.GetDirectoryAddress(); }

QString GetServerListFileName() { return ServerListManager.GetServerListFileName(); }
bool SetServerListFileName ( QString strFilename ) { return ServerListManager.SetServerListFileName ( strFilename ); }

// GUI settings ------------------------------------------------------------
void SetAutoRunMinimized ( const bool NAuRuMin ) { bAutoRunMinimized = NAuRuMin; }
bool GetAutoRunMinimized() { return bAutoRunMinimized; }

int GetClientNumAudioChannels ( const int iChanNum ) { return vecChannels[iChanNum].GetNumAudioChannels(); }
void SetEnableDelayPanning ( bool bDelayPanningOn ) { bDelayPan = bDelayPanningOn; }
bool IsDelayPanningEnabled() { return bDelayPan; }

protected:
// access functions for actual channels
Expand Down Expand Up @@ -292,6 +272,8 @@ class CServer : public QObject, public CServerSlots<MAX_NUM_CHANNELS>

virtual void customEvent ( QEvent* pEvent );

void CreateAndSendRecorderStateForAllConChannels();

// if server mode is normal or double system frame size
bool bUseDoubleSystemFrameSize;
int iServerFrameSizeSamples;
Expand Down Expand Up @@ -433,9 +415,8 @@ public slots:

void OnCLSendEmptyMes ( CHostAddress TargetInetAddr )
{
// only send empty message if server list is enabled and this is not
// a directory server
if ( ServerListManager.GetEnabled() && !ServerListManager.IsDirectoryServer() )
// only send empty message if not a directory server
if ( !ServerListManager.IsDirectoryServer() )
{
ConnLessProtocol.CreateCLEmptyMes ( TargetInetAddr );
}
Expand Down
Loading