jamulussoftware / jamulussoftware/jamulus
JSON-RPC - Extend jamulusserver/getClients to include additional client attributes
- Dominant language
- C
- Stars
- 1.1k
- Forks
- 248
- Avg merge
- 2d 3h
- Merged PRs (30d)
- 9
Description
**What is the current behaviour and why should it be changed?**
The current getClients RPC method does not return skill level, country, city, or instrument for connected clients. In order to close the gap between desired functionality in (now closed) PR #2006, getClients can be extended to provide these additional properties.
**Describe possible approaches**
@dtinth had as specific goal of not touching server.cpp when initially authoring JSON-RPC. This limited the information available to the RPC interface. The most direct solution I see now is to modify GetConCliParam() to add an additional vector that can be populated with Channel Information (core). **GetConCliParam() is already used by JSON-RPC within the getClients method.**
```
void CServer::GetConCliParam ( CVector& vecHostAddresses,
CVector& vecsName,
CVector& veciJitBufNumFrames,
CVector& veciNetwFrameSizeFact,
CVector& vecChanInfo )
```
Changing this method impacts server.cpp/h and serverdlg.cpp only. While serverdlg would not make use of this additional information, having connection attributes such as country, city, instrument, and skill would allow the serverdlg to make use of this information in the future within the UI's "connections" widget.
The modified GetConCliParam method proposed is:
```
void CServer::GetConCliParam ( CVector& vecHostAddresses,
CVector& vecsName,
CVector& veciJitBufNumFrames,
CVector& veciNetwFrameSizeFact,
CVector& vecChanInfo )
{
// init return values
vecHostAddresses.Init ( iMaxNumChannels );
vecsName.Init ( iMaxNumChannels );
veciJitBufNumFrames.Init ( iMaxNumChannels );
veciNetwFrameSizeFact.Init ( iMaxNumChannels );
vecChanInfo.Init ( iMaxNumChannels );
// check all possible channels
for ( int i = 0; i < iMaxNumChannels; i++ )
{
if ( vecChannels[i].IsConnected() )
{
// get requested data
vecHostAddresses[i] = vecChannels[i].GetAddress();
vecsName[i] = vecChannels[i].GetName();
veciJitBufNumFrames[i] = vecChannels[i].GetSockBufNumFrames();
veciNetwFrameSizeFact[i] = vecChannels[i].GetNetwFrameSizeFact();
vecChanInfo[i] = vecChannels[i].GetChanInfo();
}
}
}
```
The new channel attributes would then be added to the getClients JSON-RPC method:
```
CVector veciNetwFrameSizeFact;
CVector vecChanInfo;
pServer->GetConCliParam ( vecHostAddresses, vecsName, veciJitBufNumFrames, veciNetwFrameSizeFact, vecChanInfo );
// we assume that all vectors have the same length
const int iNumChannels = vecHostAddresses.Size();
// fill list with connected clients
for ( int i = 0; i < iNumChannels; i++ )
{
if ( vecHostAddresses[i].InetAddr == QHostAddress ( static_cast ( 0 ) ) )
{
continue;
}
QJsonObject client{
{ "id", i },
{ "address", vecHostAddresses[i].toString ( CHostAddress::SM_IP_PORT ) },
{ "name", vecsName[i] },
{ "jitterBufferSize", veciJitBufNumFrames[i] },
{ "channels", pServer->GetClientNumAudioChannels ( i ) },
{ "instrument", vecChanInfo[i].iInstrument },
{ "city", vecChanInfo[i].strCity },
{ "skillLevel", vecChanInfo[i].eSkillLevel },
{ "countryCode", vecChanInfo[i].eCountry },
};
clients.append ( client );
}
```
The proposal here returns the enumerated values of some attributes to be consistent with the current approach. Issue #2468 proposes to change this. The solution proposed here could be refactored if that issue moves forward.
**Has this feature been discussed and generally agreed?**
No
Contributor guide
Research direction
Start with GetConCliParam in server.cpp and server.h, then trace the existing getClients JSON-RPC entry point and inspect CChannelCoreInfo. Check the related serverdlg.cpp call sites and issue #2468 before choosing the response representation. Done means getClients returns the specified client attributes without breaking existing callers.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- api, backend
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100