Skip to content

fix: GetConCliParam skips channels not yet identified#3716

Closed
mcfnord wants to merge 1 commit into
jamulussoftware:mainfrom
mcfnord:fix/getclients-identification-gate
Closed

fix: GetConCliParam skips channels not yet identified#3716
mcfnord wants to merge 1 commit into
jamulussoftware:mainfrom
mcfnord:fix/getclients-identification-gate

Conversation

@mcfnord
Copy link
Copy Markdown
Contributor

@mcfnord mcfnord commented May 26, 2026

Problem

GetConCliParam (the backend for the jamulusserver/getClients JSON-RPC method and the GUI connected-clients list) filters channels with IsConnected() only.

IsConnected() becomes true as soon as the first UDP audio packet arrives. But bIsIdentified is only set true inside SetChanInfo(), which is called after the server sends ReqChanInfo and the client responds with its CHANNEL_INFO protocol message — typically one round-trip (20–200 ms) later.

During that window, GetConCliParam returns a default-constructed CChannelCoreInfo for the joining channel: empty name, QLocale::AnyCountry, and instrument 0 (GetNotUsedInstrument()). Any consumer reading the connected-clients list in that window gets stale data.

Fix

  • Add IsIdentified() const public getter to CChannel (mirrors the existing IsConnected() getter)
  • Gate GetConCliParam on IsConnected() && IsIdentified()

Channels in the pre-identification window become invisible to GetConCliParam until their full profile is committed. This is consistent with the existing gate in PrepAndSendPacket, which already withholds audio delivery from unidentified channels.

The serverdlg.cpp GUI path also calls GetConCliParam and receives the fix automatically.

Testing

Verified that a newly connecting client does not appear in getClients output until name, country, and instrument are all populated.

A channel becomes IsConnected() as soon as the first UDP audio packet
arrives, but bIsIdentified is only set true once SetChanInfo() is called
after the client's CHANNEL_INFO protocol message is received — one
round-trip later (typically 20–200 ms).

During that window GetConCliParam (used by getClients RPC and the GUI
connected-clients list) returns a default-constructed CChannelCoreInfo
with empty name, AnyCountry, and instrument 0 for the joining channel.

Add IsIdentified() getter and gate GetConCliParam on both IsConnected()
and IsIdentified(), matching the existing gate in PrepAndSendPacket that
already withholds audio from unidentified channels.
@mcfnord mcfnord marked this pull request as draft May 27, 2026 01:12
@mcfnord
Copy link
Copy Markdown
Contributor Author

mcfnord commented May 27, 2026

Not quite sure this is the fix yet. testing...

@pljones
Copy link
Copy Markdown
Collaborator

pljones commented May 28, 2026

I think there's a reason it's the way it is -- the oldest clients didn't identify. They still need to be count as connected.

4.x does mean we could break this backwards compatibility but there would need to be a more compelling reason than "has no identifying information in the list".

@mcfnord
Copy link
Copy Markdown
Contributor Author

mcfnord commented May 29, 2026

I'm running this new code. Does that mean old clients fail to connect to my modified server? Or maybe they just don't get listed on my modified getClients API?

serverrpc.cpp and serverdlg.cpp call GetConCliParam. Populating both with what is known seems like a key objective. Wouldn't want the extremely old client to be invisible in the Connect dialog. Instead, I guess their name just appears? I think I know how tiny this user's plight is, as most users probably have newer clients, but I don't know how impactful it is. Being invisible from the main console while sonically present would of course be a hacker-fabulous calamity, but I don't think that would be the outcome. I might have to spin up an impossibly old client to figure this out!

I will go to my unfortunate grave hoping for reliability in the instrument and nation ancillary metadata, because I use the name-instrument-nation trifecta as our simulacrum of identity.

Sensing strangers when they're actually friends is a setback for custom welcome messages. See how rich they can be when they work:

Jeff → Studio D (May 27)

▎ Welcome back, Jeff — your 5th consecutive Tuesday here! You have the room to yourself for now, but Ally should be here in about an hour. Marsha K (Jazz Jam host) should be here in about 27 minutes. The Jazz Jam is Tuesdays 8pm California time, and you can listen and record at StudioD.Live.


Ally → Studio D (May 27)

▎ Welcome back to the Jazz Jam, Ally — your third Tuesday in a row! Marsha K is predicted to arrive in about 12 minutes. You can listen live or record MP3s at StudioD.Live.


moon → Sindone (May 29) [translated from German]

▎ Welcome, moon — first time here! MikZ is also online right now — you probably remember him from last night. In the room: Nico (Keyboard, France) and Claudio (Electric Guitar, Italy).


over_32keys → Green Mill (May 29) [translated from Japanese]

▎ Welcome, over_32keys — first time here! Ant-----Horns is live at Gaia right now.


Melange → Hot Texas! (May 28)

▎ Welcome, Melange — back for your second Tuesday! Nomaste is already here!


nono → Sindone (May 29) [translated from French]

▎ Welcome, nono — first time here! nils (Flute, Germany) is already there. Giancaudio (Keyboard, Rome) just arrived too.


labru → Hot Texas! (May 28)

▎ Welcome, labru — first time here! Nomaste and Melange are in the room. You're the only bass player right now.


Daniel → Windy (Chicago) (May 29) [translated from Portuguese]

▎ Welcome, Daniel — first time here! The room is just starting, with RL (bass, Boston), Bill Weidner and Leo (drums). You're the only guitarist. Almost a complete band — just missing a keyboard/piano!


Murphy → Freiheit (May 28) [translated from French]

▎ Welcome back, Murphy. The group has been going for about 6 minutes, with El Barto (electric guitar, Germany), HarryCan (mic, Germany) and Tobi (Germany), and they need a bass!


Susan → Hot Texas! (May 28)

▎ Welcome, Susan — first time here! You just missed Gary (guitar) — was here for a bit about half an hour ago.


[unnamed] → Hot Texas! (May 28)

▎ You've been busy tonight — 4 servers in the last 2 hours. You can use /stream to listen or record from most browsers.


▎ Welcome back, Jeff — your 5th consecutive Tuesday here!


de 2 → Studio D (May 27)

▎ Welcome back, de 2 — 13 consecutive Tuesdays, you're a regular here for the Jazz Jam with Marsha K! ... Also online right now: Doug W.


Marsha K → Studio D (May 27)

▎ Welcome home, Marsha K — 20 hours here and you're the heart of the Jazz Jam! Tuesdays at 8pm California time. In the room, de 2, Curt, and Eric are new to Jamulus — glad to have them.

THIS IS THE BUG, CUZ THOSE GUYS ARE NOT NEW TO JAMULUS, but they look new when their instrument / country is missing.


Marsha K → Studio D (May 27, later)

▎ Welcome back, Marsha K — right on time for your Jazz Jam! Jeff, Curt, Eric, de 2, and Rob2 are also here and ready to play.

@pljones
Copy link
Copy Markdown
Collaborator

pljones commented May 29, 2026

If they've got as far as having names, they have a full connection with the profile stuff too, don't they? Or maybe that's the stuff that was added later. (I know the jam recorder starts recording before even the name arrives, so if you've got a name, you're doing well.)

@mcfnord
Copy link
Copy Markdown
Contributor Author

mcfnord commented May 30, 2026

Sounds like wont-fix so let's hope it's will-warn.

Note: During the brief interval between a client's first audio packet and
the server receiving its profile (CHANNEL_INFO), a newly joined client may
appear in results with empty/default fields — empty name, no country,
instrument 0. This window typically lasts 20–200 ms. If you need complete
profile data, check that name is non-empty before trusting a result.

I don't see any evidence of any "additional information" kind of content in our published API documentation. This is the source for that:

/// @rpc_method jamulusserver/getClients
/// @brief Returns the list of connected clients along with details about them.
/// @param {object} params - No parameters (empty object).
/// @result {number} result.connections - The number of active connections.
/// @result {array} result.clients - The list of connected clients.
/// @result {number} result.clients[*].id - The client’s channel id.
/// @result {string} result.clients[*].address - The client’s address (ip:port).
/// @result {string} result.clients[*].name - The client’s name.
/// @result {number} result.clients[*].jitterBufferSize - The client’s jitter buffer size.
/// @result {number} result.clients[*].channels - The number of audio channels of the client.
/// @result {number} result.clients[*].instrumentCode - The id of the instrument for this channel.
/// @result {string} result.clients[*].city - The city name provided by the user for this channel.
/// @result {number} result.clients[*].countryName - The text name of the country specified by the user for this channel (see QLocale::Country).
/// @result {number} result.clients[*].skillLevelCode - The skill level id provided by the user for this channel.

@brief probably should be the description of the API, but I don't see anything like @remarks or @notes here:

https://github.com/jamulussoftware/jamulus/blob/main/tools/generate_json_rpc_docs.py

So how should additional operational details be added to the API documentation?

@mcfnord mcfnord closed this May 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants