3532: Add jamulusserver/setDirectory request

[pljones]

Short description of changes

CHANGELOG: Server RPC: Add jamulusserver/setDirectory request

Context: Fixes an issue?

Fixes: 3532

Does this change need documentation? What needs to be documented and how?

RPC documentation regenerated.

Status of this Pull Request

I've tested with nc, changing one of my servers from registered with a custom directory to unregistered and then back.

What is missing until this pull request can be merged?

Reviewing.

Ideally, testing by @anprdev to confirm it meets their requirements.

Checklist

• I've verified that this Pull Request follows the general code principles
• I tested my code and it does what I want
• My code follows the style guide make clang_format has gone wacky again
• I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
• I've filled all the content above

AUTOBUILD: Please build all targets

[ann0see]

Did you check that the server UI responds as expected?

[ann0see]

Do we have a place where we already have those pre defined directories? Isn't it somewhere in util.h?

[pljones]

There's a function to return the translated name. This is a serialisation of the enum, though, so it must not use the translations.

[pljones]

Did you check that the server UI responds as expected?

No - it would be the same as any other RPC-based change, though. For example, jamulusserver/setWelcomeMessage ends up here:

void CServer::SetWelcomeMessage ( const QString& strNWelcMess )
{
    // we need a mutex to secure access
    QMutexLocker locker ( &MutexWelcomeMessage );
    strWelcomeMessage = strNWelcMess;

    // restrict welcome message to maximum allowed length
    strWelcomeMessage = strWelcomeMessage.left ( MAX_LEN_CHAT_TEXT );
}

That won't update the UI, as there's no signal being raised.

Actually...

src/serverdlg.cpp:    QObject::connect ( pServer, &CServer::SvrRegStatusChanged, this, &CServerDlg::OnSvrRegStatusChanged );
...
src/serverdlg.h:    void OnSvrRegStatusChanged() { UpdateGUIDependencies(); }

src/server.cpp:    QObject::connect ( &ServerListManager, &CServerListManager::SvrRegStatusChanged, this, &CServer::SvrRegStatusChanged );

src/serverlist.cpp:    emit SvrRegStatusChanged();

So it's probably going to do better than the welcome message. (It won't get the text updates -- but at least the status will show up.)

[ann0see]

Thanks. And indeed, the UI is semi in line. It does not go to "None" correctly, but shows that the server is no longer registered. Changing custom/any genre 1/... works though.

[pljones]

Mmm, that means UpdateGUIDependencies is missing something... Possibly worth investigating?

void CServerDlg::UpdateGUIDependencies()
{
    const EDirectoryType directoryType = pServer->GetDirectoryType();
    cbxDirectoryType->setCurrentIndex ( static_cast<int> ( directoryType ) + 1 );
...

It should do that if changing from "registered" to "unregistered"...

Hmm, looks like it's definitely missing on the recording stuff, so hopefully it's not relying on UpdateGUIDependencies for that. Ah yes, UpdateRecorderStatus.

But it's a bit limited:

    QObject::connect ( pServer, &CServer::Started, this, &CServerDlg::OnServerStarted );
    QObject::connect ( pServer, &CServer::Stopped, this, &CServerDlg::OnServerStopped );

Those when someone connects or everyone disconnect (not under UI control).

    QObject::connect ( pServer, &CServer::SvrRegStatusChanged, this, &CServerDlg::OnSvrRegStatusChanged );

Aforementioned registration feedback, as it's asynchronous.

    QObject::connect ( pServer, &CServer::RecordingSessionStarted, this, &CServerDlg::OnRecordingSessionStarted );
    QObject::connect ( pServer, &CServer::StopRecorder, this, &CServerDlg::OnStopRecorder );

These two have unix signals connected to them, so those got wired to the UI. And also depend on whether the server has anyone connected.

    QObject::connect ( pServer, &CServer::CLVersionAndOSReceived, this, &CServerDlg::OnCLVersionAndOSReceived );
    QObject::connect ( &SystemTrayIcon, &QSystemTrayIcon::activated, this, &CServerDlg::OnSysTrayActivated );

No idea...

[ann0see]

So you'd argue that the refactoring is definitely Jamulus 4?

What's your take on this PR otherwise: Merge and release or wait?

I'd be in slight favor of merging even though this is a feature. Otherwise it could take a very long time to be published.

[pljones]

Yep, the additional handling for RPC is definitely out of scope for this feature.

I've kicked off an "all targets" build as the legacy macOS originally failed, just in case:
https://github.com/pljones/jamulus/actions/runs/17557282994

[ann0see]

@pljones I've tagged this for the upcoming release.

[anprdev]

Top Guys that you have done this.
Thanks