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

Adapt pbc.c to current json rpc spec #1857

Merged
merged 2 commits into from
Dec 14, 2023

Conversation

sklemer1
Copy link

@sklemer1 sklemer1 commented Jul 3, 2022

Just added 'package' to rpc json and provided a cli option for it.

@s-martin s-martin added enhancement future3 Relates to future3 development labels Jul 3, 2022
@s-martin
Copy link
Collaborator

s-martin commented Dec 4, 2023

Could we merge this?

@pabera
Copy link
Collaborator

pabera commented Dec 4, 2023

There is no harm with this implementation if I analyse this correctly. But the JSON RPC spec does NOT have a package pattern, as far as I can see that.

@sklemer1 Maybe you can help us understand where this is coming from?

@s-martin
Copy link
Collaborator

@pabera
I get this error:

pi@phoniebox:~/RPi-Jukebox-RFID/src/cli_client $ ./pbc -o "{ "jsonrpc": "2.0", "method": "host.shutdown" }"
Received {"error": {"code": -1, "message": "Missing mandatory parameter 'package'."}, "id": 123} (87 Bytes)

so it seems package is necessary, although I didn't find it in the spec

@sklemer1
Copy link
Author

It is some time since I addes this -- the reason was that the JSON from Phonie-Box used these 'package' attributes where as the cli client did not. I haven't looked into the specification -- I saw the need as ist was used in the real world implementation :).

@s-martin
Copy link
Collaborator

Thanks for clarification.

@s-martin s-martin merged commit 9dceb84 into MiczFlor:future3/develop Dec 14, 2023
AlvinSchiller pushed a commit to AlvinSchiller/RPi-Jukebox-RFID that referenced this pull request Jan 9, 2024
* pbc.c: add 'package' to rpc commands

* pbc.c: Also recognise negative numbers as int in values
(somehow hacky solution)
@pabera pabera added this to the v3.5 milestone Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement future3 Relates to future3 development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants