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.

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

Add JSON-RPC access to directories and server lists. #3249

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
99 changes: 94 additions & 5 deletions docs/JSON-RPC.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ Results:
| result.skillLevel | string | The musician’s skill level (beginner, intermediate, expert, or null). |
| result.countryId | number | The musician’s country ID (see QLocale::Country). |
| result.city | string | The musician’s city. |
| result.instrumentId | number | The musician’s instrument ID (see CInstPictures::GetTable). |
| result.instrument | string | The musician’s instrument. |
| result.skillLevel | string | Your skill level (beginner, intermediate, expert, or null). |


Expand Down Expand Up @@ -186,6 +186,22 @@ Results:
| result.clients | array | The client list. See jamulusclient/clientListReceived for the format. |


### jamulusclient/pollServerList

Requests the server list from a specified directory.
Copy link
Member

Choose a reason for hiding this comment

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

Sorry for my forgetfulness here - you need to update the python script I mentioned below and not edit the file directly

Copy link
Author

Choose a reason for hiding this comment

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

I have run the script and pushed the changes.


Parameters:

| Name | Type | Description |
| --- | --- | --- |
| params.directory | string | Socket address of directory (hostname:port), e.g. anygenre1.jamulus.io:22124 |

Results:

| Name | Type | Description |
| --- | --- | --- |
| result | string | "ok" or "error" if invalid socket address |

### jamulusclient/sendChatText

Sends a chat text message.
Expand All @@ -203,6 +219,40 @@ Results:
| result | string | Always "ok". |


### jamulusclient/connect

Connect client to a server.

Parameters:

| Name | Type | Description |
| --- | --- | --- |
| params.address | string | Server socket address (hostname:port) |

Results:

| Name | Type | Description |
| --- | --- | --- |
| result | string | "ok" or error if invalid address.


### jamulusclient/disconnect

Disconnect client from server.

Parameters:

| Name | Type | Description |
| --- | --- | --- |
| params | object | No parameters (empty object). |

Results:

| Name | Type | Description |
| --- | --- | --- |
| result | string | Always "ok".


### jamulusclient/setName

Sets your name.
Expand Down Expand Up @@ -258,9 +308,9 @@ Results:
| result.clients[*].name | string | The client’s name. |
| result.clients[*].jitterBufferSize | number | The client’s jitter buffer size. |
| result.clients[*].channels | number | The number of audio channels of the client. |
| result.clients[*].instrumentCode | number | The id of the instrument for this channel. |
| result.clients[*].instrument | string | The instrument name provided by the user for this channel. |
| result.clients[*].city | string | The city name provided by the user for this channel. |
| result.clients[*].countryName | number | The text name of the country specified by the user for this channel (see QLocale::Country). |
| result.clients[*].countryName | string | The country name provided by the user for this channel. |
| result.clients[*].skillLevelCode | number | The skill level id provided by the user for this channel. |


Expand Down Expand Up @@ -444,9 +494,37 @@ Parameters:
| params.clients[*].id | number | The channel ID. |
| params.clients[*].name | string | The musician’s name. |
| params.clients[*].skillLevel | string | The musician’s skill level (beginner, intermediate, expert, or null). |
| params.clients[*].countryId | number | The musician’s country ID (see QLocale::Country). |
| params.clients[*].country | string | The musician’s country. |
| params.clients[*].city | string | The musician’s city. |
| params.clients[*].instrumentId | number | The musician’s instrument ID (see CInstPictures::GetTable). |
| params.clients[*].instrument | string | The musician’s instrument. |


### jamulusclient/serverListReceived

Emitted when the server list is received.

Parameters:

| Name | Type | Description |
| --- | --- | --- |
| params.servers | array | The server list. |
| params.servers[*].address | string | The server's socket address (hostname:port). |
| params.servers[*].name | string | The server’s name. |
| params.servers[*].country | string | The servers’s country. |
| params.servers[*].city | string | The server’s city. |


### jamulusclient/serverInfoReceived

Emitted when a server info is received.

Parameters:

| Name | Type | Description |
| --- | --- | --- |
| params.servers[*].address | string | The server's socket address (hostname:port). |
| params.servers[*].pingTime | number | The round-trip ping time in ms. |
| params.servers[*].numClients | number | The quantity of clients connected to the server . |


### jamulusclient/connected
Expand All @@ -471,3 +549,14 @@ Parameters:
| params | object | No parameters (empty object). |


### jamulusclient/recorderState

Emitted when the client is connected to a server who's recorder state changes.

Parameters:

| Name | Type | Description |
| --- | --- | --- |
| params.state | number | The recorder state (see ERecorderState). |


4 changes: 4 additions & 0 deletions src/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,10 @@ class CClient : public QObject
Channel.GetBufErrorRates ( vecErrRates, dLimit, dMaxUpLimit );
}

CProtocol * getConnLessProtocol() {
return &ConnLessProtocol;
}

// settings
CChannelCoreInfo ChannelInfo;
QString strClientName;
Expand Down
136 changes: 128 additions & 8 deletions src/clientrpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ CClientRpc::CClientRpc ( CClient* pClient, CRpcServer* pRpcServer, QObject* pare
/// @param {number} params.clients[*].id - The channel ID.
/// @param {string} params.clients[*].name - The musician’s name.
/// @param {string} params.clients[*].skillLevel - The musician’s skill level (beginner, intermediate, expert, or null).
/// @param {number} params.clients[*].countryId - The musician’s country ID (see QLocale::Country).
/// @param {string} params.clients[*].country - The musician’s country.
/// @param {string} params.clients[*].city - The musician’s city.
/// @param {number} params.clients[*].instrumentId - The musician’s instrument ID (see CInstPictures::GetTable).
/// @param {string} params.clients[*].instrument - The musician’s instrument.
connect ( pClient, &CClient::ConClientListMesReceived, [=] ( CVector<CChannelInfo> vecChanInfo ) {
QJsonArray arrChanInfo;
for ( const auto& chanInfo : vecChanInfo )
Expand All @@ -64,9 +64,9 @@ CClientRpc::CClientRpc ( CClient* pClient, CRpcServer* pRpcServer, QObject* pare
{ "id", chanInfo.iChanID },
{ "name", chanInfo.strName },
{ "skillLevel", SerializeSkillLevel ( chanInfo.eSkillLevel ) },
{ "countryId", chanInfo.eCountry },
{ "country", QLocale::countryToString ( chanInfo.eCountry ) },
{ "city", chanInfo.strCity },
{ "instrumentId", chanInfo.iInstrument },
{ "instrument", CInstPictures::GetName ( chanInfo.iInstrument ) },
};
arrChanInfo.append ( objChanInfo );
}
Expand Down Expand Up @@ -94,11 +94,131 @@ CClientRpc::CClientRpc ( CClient* pClient, CRpcServer* pRpcServer, QObject* pare
} );
} );

/// @rpc_notification jamulusclient/serverListReceived
/// @brief Emitted when the server list is received.
/// @param {array} params.servers - The server list.
/// @param {string} params.servers[*].address - Socket address (ip_address:port)
/// @param {string} params.servers[*].name - Server name
/// @param {string} params.servers[*].country - Server country
/// @param {string} params.servers[*].city - Server city
connect ( pClient->getConnLessProtocol(), &CProtocol::CLServerListReceived, [=] ( CHostAddress /* unused */, CVector<CServerInfo> vecServerInfo ) {
QJsonArray arrServerInfo;
for ( const auto& serverInfo : vecServerInfo )
{
QJsonObject objServerInfo{
{ "address", serverInfo.HostAddr.toString() },
{ "name", serverInfo.strName },
{ "country", QLocale::countryToString ( serverInfo.eCountry) },
{ "city", serverInfo.strCity },
};
arrServerInfo.append ( objServerInfo );
pClient->CreateCLServerListPingMes ( serverInfo.HostAddr );
}
pRpcServer->BroadcastNotification ( "jamulusclient/serverListReceived",
QJsonObject{
{ "servers", arrServerInfo },
} );
} );

/// @rpc_notification jamulusclient/serverInfoReceived
/// @brief Emitted when a server info is received.
/// @param {string} params.address - The server socket address
/// @param {number} params.pingtime - The round-trip ping time in ms
/// @param {number} params.numClients - The quantity of clients connected to the server
connect (pClient, &CClient::CLPingTimeWithNumClientsReceived, [=] ( CHostAddress InetAddr, int iPingTime, int iNumClients ) {
pRpcServer->BroadcastNotification ( "jamulusclient/serverInfoReceived",
QJsonObject{
{"address", InetAddr.toString()},
{"pingTime", iPingTime},
{"numClients", iNumClients}
} );
} );

/// @rpc_notification jamulusclient/disconnected
/// @brief Emitted when the client is disconnected from the server.
/// @param {object} params - No parameters (empty object).
connect ( pClient, &CClient::Disconnected, [=]() { pRpcServer->BroadcastNotification ( "jamulusclient/disconnected", QJsonObject{} ); } );

/// @rpc_notification jamulusclient/recorderState
/// @brief Emitted when the client is connected to a server who's recorder state changes.
/// @param {number} params.state - The recorder state
connect ( pClient, &CClient::RecorderStateReceived, [=] ( const ERecorderState newRecorderState ) {
pRpcServer->BroadcastNotification ( "jamulusclient/recorderState",
QJsonObject{
{"state", newRecorderState}
} );
} );

/// @rpc_method jamulus/pollServerList
/// @brief Request list of servers in a directory
/// @param {string} params.directory - socket address of directory to query, e.g. anygenre1.jamulus.io:22124.
/// @result {string} result - "ok" or "error" if bad arguments.
pRpcServer->HandleMethod ( "jamulusclient/pollServerList", [=] ( const QJsonObject& params, QJsonObject& response ) {
auto jsonDirectoryIp = params["directory"];
if ( !jsonDirectoryIp.isString() )
{
response["error"] = CRpcServer::CreateJsonRpcError ( CRpcServer::iErrInvalidParams, "Invalid params: directory is not a string" );
riban-bw marked this conversation as resolved.
Show resolved Hide resolved
return;
}

CHostAddress haDirectoryAddress;
if ( NetworkUtil().ParseNetworkAddress (
jsonDirectoryIp.toString(),
haDirectoryAddress,
false ) )
{
// send the request for the server list
pClient->CreateCLReqServerListMes( haDirectoryAddress );
response["result"] = "ok";
}
else
{
response["error"] = CRpcServer::CreateJsonRpcError ( CRpcServer::iErrInvalidParams, "Invalid params: directory is not a valid socket address" );
}

response["result"] = "ok";
} );

/// @rpc_method jamulusclient/connect
/// @brief Disconnect client from server
Copy link
Member

Choose a reason for hiding this comment

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

The documentation is wrong.

Suggested change
/// @brief Disconnect client from server
/// @brief Connect client to server

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in next commit.

/// @param {string} params.address - Server socket address (ip_addr:port).
/// @result {string} result - Always "ok".
pRpcServer->HandleMethod ( "jamulusclient/connect", [=] ( const QJsonObject& params, QJsonObject& response ) {
auto jsonAddr = params["address"];
if ( !jsonAddr.isString() )
{
response["error"] = CRpcServer::CreateJsonRpcError ( CRpcServer::iErrInvalidParams, "Invalid params: address is not a string" );
return;
}

Copy link
Member

Choose a reason for hiding this comment

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

See

void CClientDlg::Connect ( const QString& strSelectedAddress, const QString& strMixerBoardLabel )
as below.

Copy link
Collaborator

Choose a reason for hiding this comment

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

As above.

if ( pClient->SetServerAddr(jsonAddr.toString()) )
{
if ( !pClient->IsRunning() )
{
pClient->Start();
}
response["result"] = "ok";
}
else
{
response["error"] = CRpcServer::CreateJsonRpcError ( 1, "Bad server address" );
}
} );

/// @rpc_method jamulusclient/disconnect
/// @brief Disconnect client from server
/// @param {object} params - No parameters (empty object).
/// @result {string} result - Always "ok".
pRpcServer->HandleMethod ( "jamulusclient/disconnect", [=] ( const QJsonObject& params, QJsonObject& response ) {
if ( pClient->IsRunning() )
{
pClient->Stop();
Copy link
Member

Choose a reason for hiding this comment

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

I believe that this should call Disconnect() here instead

void CClientDlg::Disconnect()
if the UI is running.

Copy link
Collaborator

@pljones pljones Apr 19, 2024

Choose a reason for hiding this comment

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

Nothing should call Dialog methods, except internally. Ever. The Dialog UI might not exist if th RPC interface is in use.

Copy link
Member

Choose a reason for hiding this comment

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

We need to update the UI somehow. -> refactoring needed.

Copy link
Collaborator

@pljones pljones Apr 20, 2024

Choose a reason for hiding this comment

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

Yes. If a change to state happens to the Client (CClient), it should emit an event. The UI (CClientDlg) can respond to that. But the state should be in the Client itself, not the UI. That way, it's accessible to both UI and RPC-I.

}

response["result"] = "ok";
Copy link
Member

Choose a reason for hiding this comment

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

Why is this always ok and not an error if this is called on an already disconnected client?

Copy link
Author

Choose a reason for hiding this comment

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

I was only following similar code elsewhere in the interface. A common question is whether we want an error when we ask for an action and the action gives the desired effect, e.g. disconnect from a device to which we are not connected results in being disconnected. Is this erroneous? I would guess other places this same, alway "ok" result is used here allows such an action that will never fail to give an acknowledgement that the request was recevied and actioned.

Copy link
Member

Choose a reason for hiding this comment

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

Ok. Then we can leave it as is.

Q_UNUSED ( params );
} );

/// @rpc_method jamulus/getMode
/// @brief Returns the current mode, i.e. whether Jamulus is running as a server or client.
/// @param {object} params - No parameters (empty object).
Expand All @@ -125,17 +245,17 @@ CClientRpc::CClientRpc ( CClient* pClient, CRpcServer* pRpcServer, QObject* pare
/// @result {number} result.id - The channel ID.
/// @result {string} result.name - The musician’s name.
/// @result {string} result.skillLevel - The musician’s skill level (beginner, intermediate, expert, or null).
/// @result {number} result.countryId - The musician’s country ID (see QLocale::Country).
/// @result {string} result.country - The musician’s country.
/// @result {string} result.city - The musician’s city.
/// @result {number} result.instrumentId - The musician’s instrument ID (see CInstPictures::GetTable).
/// @result {number} result.instrument - The musician’s instrument.
/// @result {string} result.skillLevel - Your skill level (beginner, intermediate, expert, or null).
pRpcServer->HandleMethod ( "jamulusclient/getChannelInfo", [=] ( const QJsonObject& params, QJsonObject& response ) {
QJsonObject result{
// TODO: We cannot include "id" here is pClient->ChannelInfo is a CChannelCoreInfo which lacks that field.
Copy link
Member

Choose a reason for hiding this comment

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

Typo, but not your issue.

Suggested change
// TODO: We cannot include "id" here is pClient->ChannelInfo is a CChannelCoreInfo which lacks that field.
// TODO: We cannot include "id" here as pClient->ChannelInfo is a CChannelCoreInfo which lacks that field.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in next commit.

{ "name", pClient->ChannelInfo.strName },
{ "countryId", pClient->ChannelInfo.eCountry },
{ "country", QLocale::countryToString ( pClient->ChannelInfo.eCountry ) },
{ "city", pClient->ChannelInfo.strCity },
{ "instrumentId", pClient->ChannelInfo.iInstrument },
{ "instrument", CInstPictures::GetName ( pClient->ChannelInfo.iInstrument ) },
{ "skillLevel", SerializeSkillLevel ( pClient->ChannelInfo.eSkillLevel ) },
};
response["result"] = result;
Expand Down
Loading