bitfocus / companion-module-qsys-remote-control

MIT License
5 stars 3 forks source link

JSON RPC 2.0 commands not null terminated #2

Closed jrsewing1 closed 6 months ago

jrsewing1 commented 4 years ago

I have been trying to test with this module for a few days. Live hardware is not responsive to the string being sent. in the Q-sys RPC documentation it calls for a null terminated RPC. I setup a branch to test with and fond that with \x00 sent after the command string the hardware will recognize that a JSON RPC command was sent, but gives back error code

If I use this as the string generator where i added the \x00: self.socket.send('{ "jsonrpc": "2.0", "id": 1234, "method": ' + cmd + '\r\x00'); I get this error back from my hardware: 32700 | Parse error. Invalid JSON was received by the server.

I believe this is because this puts the null character I added in before the JSON string is closed with the end '}'. Is there a flag somewhere else in the network stack after this string creation that will allow for the JSON RPC call to have a null character?

sorry this is just over my head and i do not really know how to continue to debug this/modify it to work.

Thank you for your help

McHauge commented 4 years ago

Thx for testing, the end bracket is included in the cmd variable, but for clarification, it should probably be included with the end characters instead.

but instead of: self.socket.send('{ "jsonrpc": "2.0", "id": 1234, "method": ' + cmd + '\r\x00'); or self.socket.send('{ "jsonrpc": "2.0", "id": 1234, "method": ' + cmd + '\r'); you could try without the return: self.socket.send('{ "jsonrpc": "2.0", "id": 1234, "method": ' + cmd + '\x00');

-McHauge

jrsewing1 commented 4 years ago

Without the return I still get back the -32700 error.

Here is the output of companion viewed from Wireshark: { "jsonrpc": "2.0", "id": 1234, "method": "Control.Set", "params": { "Name": "volPrivacy_mute", "Value": "1" } }

this checks out as valid JSON from https://jsonlint.com/ but the raw HEX being sent looks like this from that command:

capture 1

when using a terminal connect to port 1710 if i send the same command it looks like this:

image and i get the response: '{ "jsonrpc": "2.0", "id": 1234, "method": "Control.Set", "params": { "Name": "volPrivacy_mute", "Value": "1" } }`

Any thoughts on how to make the output be formatted like the second image?

-Ross

McHauge commented 4 years ago

hmm, might have something to do with how I phrased the message as a combination of strings/numbers, might have to convert it directly to a JSON object before sending it out. Just to verify do you have a dev environment set up locally (able to edit the module locally)? if so you could try using this code instead of the one used currently:

Old: self.socket.send('{ "jsonrpc": "2.0", "id": 1234, "method": ' + cmd + '\x00'); New: self.socket.send( JSON.parse('{ "jsonrpc": "2.0", "id": 1234, "method": ' + cmd + '\x00') );

if you don't I can build you a test build for windows from my pc.

jrsewing1 commented 4 years ago

I do have a local Environment, I will test this now

jrsewing1 commented 4 years ago

Must have needed to eat dinner, The self.socket.send('{ "jsonrpc": "2.0", "id": 1234, "method": ' + cmd + '\x00'); version is working, and the new version wrapped in JSON.parse() threw an exception when i would test the action.

I think this closes this one out. I am new to Git so i am not sure how to push this change out for you to include in the next build. I plan on testing out the other functions this evening. Thanks again for your help!

McHauge commented 4 years ago

No problem, I'll change the "\r" to "\x00" in the module and put it on the list of modules to update to the core. Thanks for testing! the original requester never reported back so it is nice to finally have it tested in the wild. If you find anything more please let me know so we can get it updated and ready for the release of companion V2.0

-McHauge

jrsewing1 commented 4 years ago

if i find more would you like a new issue for each thing, or continue in one thread?

-Ross

McHauge commented 4 years ago

You can just continue here for now, I just changed it to what I mentioned before. and compiled a build of it on the link below, but should be the same as yours you can cross-check it on GitHub if you want, or maybe simply test the build attached.

OneDrive

-McHauge

onfire4g05 commented 6 months ago

This should have been resolved, if it were still an issue, with the 2.0 release.