Enhanced serial communication #92
Conversation
knarfS
commented
Sep 8, 2015
knarfS
commented
- Added command 'output' to get the actual output state of the device
- Changed the response of 'bl', 'cal', 'monitor', 'reset', 'on' and 'off' from 'ok' to 'bl', 'cal', ....
- Added 'on', 'off' and 'uvlo' responses when the states/settings are changed with the ReLoad Pro device.
Added new command output
- Added new command 'output' - Moved 'on' and 'off' responses to utils.c - Changed responses of 'bl', 'cal', 'monitor', 'reset', 'on' and 'off'
Added a serial 'uvlo' response when the undervoltage setting is change at the ReLoad Pro device.
Moved the 'on'/'off' responses from the comms.c to here, so the response is also send when the output state is changed with the ReLoad Pro device.
|
Thanks for the contribution! While I'm onboard with most of the changes, changing the responses for existing commands from "ok" to the command name will break any existing code that interfaces with the R:LP, which I really don't want to do. If you're happy to drop those changes, and squash them into a single commit (or one per feature, rather than one per file), I'd be happy to accept your PR. |
|
Hi Nick. Yes I'm aware of that problem, but the actual responses makes it very hard for a client application to handle the serial communication. This is due to the fact, that the communication is both blocking (wait for the response of a command, like set X) and non-blocking (wait for non requested responses like set, on, off, read, ...). So you can change 'bl', 'cal', and 'reset' back to the 'ok' response. But I would vote for the changed responses of the 'on'/'off' commands. So the responses would be the same for the unrequested responses, like for the 'set' command or the 'read' command. At the moment I'm working on a client for the RLP in Qt (Windows, Linux), with recording functionality, graphs (current, voltage, power, resistance, amp hour and watt hour over time, amp hour and watt hour), min-/max-values, etc. |
|
I agree that the current setup isn't ideal. What I'd really like to do is replace the whole communications protocol with something SCPI compatible. But in the meantime, I don't think it's safe to simply break the protocol for anyone else who's already using it. Assuming it doesn't add too much overhead to the firmware size, you could add a command to switch on the new response format for existing commands. Different responses for new commands are absolutely okay. |
|
Yes, it took a few minutes to figure out how the command parsing in commands.h works :) What's about the 'on'/'off' response? Will you re-add the 'ok' response in comms.c? That would result in 2 responses ('on'/'off' and 'ok'). I'm fine with that behavior. |
|
I think the existing behavour for existing commands - 'ok' only - needs to be preserved for compatibility. I realise this isn't ideal for an app, though it's not intractable, since 'ok' is only issued in response to interactive commands, you can listen until you receive that. If you're happy to amend the PR so it doesn't break compatibility, and squash the commits down into one, I'm happy to accept the PR. |
