Connect disconnect#3813
Open
digable1 wants to merge 3 commits into
Open
Conversation
Adds two client JSON-RPC methods:
- jamulusclient/connect(directory, server): validates the server is (still)
listed under the given directory's last polled server list, then attempts
the connection, returning "ok" or a loose classification of why not
("Not Found", "Unauthorized", "Gone (server no longer listed)", "Upgrade
Required (obsolete protocol, upgrade Jamulus)", "Insufficient Storage
(Full)").
- jamulusclient/disconnect(directory, server): tears down the connection
established by jamulusclient/connect, validating the params identify the
current connection.
Also includes jamulusclient/getDirectories, needed by jamulusclient/connect's
callers to discover directory socket addresses in the first place.
(jamulusclient/getCurrentDirectory and jamulusclient/setCurrentDirectory are
left out of this branch for a separate, focused review.)
Both connect/disconnect share their core logic with the desktop UI's
Connect/Disconnect button instead of duplicating it:
- CClient::ConnectToServer() is a new method used by both CClientDlg::Connect
and jamulusclient/connect to attempt a connection and classify the outcome.
- CClient::Stop() now reliably emits Disconnected() regardless of caller, and
CClientDlg::OnDisconnected()/OnClientIDReceived() (rather than Connect()/
Disconnect() themselves) own the GUI sync, so the desktop UI stays correct
whether the connection was started/stopped via the UI or via the API.
- CClient::GetServerAddress() and GetMyChannelID() are new small accessors
needed to let the UI show a sensible default server name/channel ID
regardless of which path connected. GetMyChannelID() also fills in
jamulusclient/getChannelInfo's result.id, previously a TODO because
CChannelCoreInfo lacks that field.
digable1
marked this pull request as ready for review
July 20, 2026 18:13
Member
Member
|
But yes, your approach probably is also worth combining into his codebase but this definitely needs discussion |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
**Add new connect/disconnect JSON-RPC API methods and a Id attribute to getChannelInfo result collection **
CHANGELOG:
Add new JSON-RPC API methods and channel info attribute. This includes re-factoring to share the connect and disconnect processing between the Jamulus client and the JSON-RPC API calls.
Context: Fixes an issue?
This is take 2 of an original submission of a closed issue, #3783. Reasons for this re-submission:
An attempt was made to fix the original PR submission issues, and link to that work was provided there. But as commented on that PR "I am a bit concerned about the message "This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository." that is displayed in that commit. I had not seen that before, and while I know it's not nefarious, the results from a search for that string suggested it (looks like it really is missing an original source for that commit?)". So there is confusion about whether that work actually still part of the PR pipeline, especially since that issue is now closed. I also took this opportunity to resolve the submission issues that caused this extra work to be done, so this one should be clean.
Does this change need documentation? What needs to be documented and how?
The documentation JSON-RPC.md has been updated (as you know, there's a tool for automating this).
Status of this Pull Request
What is missing until this pull request can be merged?
Checklist