Parameter protocol messages - update to clarify responses#2365
Parameter protocol messages - update to clarify responses#2365hamishwillee wants to merge 20 commits intomasterfrom
Conversation
…e set Updated descriptions for PARAM_REQUEST_READ, PARAM_REQUEST_LIST, PARAM_VALUE, PARAM_SET, and PARAM_ERROR messages to clarify usage and response requirements.
|
Let's not rush this one in. We run the risk of making a whole bunch of things no longer follow the spec, which is a little rude. For example, ArduPilot won't fill in the name for out-of-range or permission-denied errors if you don't sent it in yourself. Of course, we only actually really use set-by-name as we're quite clear by-numeric-id doesn't really work, but the wording would put us in violation. Maybe we should start with |
|
@peterbarker Sure! This is why we have review. Note that no one will be filling in the name of out of range errors, because they don't have a name. If it doesn't say that, it should. It would help if you could add those variations as suggestions - I really can't guess them. Note also that this needs to match mavlink/mavlink-devguide#644 |
Co-authored-by: Peter Barker <pbarker@barker.dropbear.id.au>
Co-authored-by: Peter Barker <pbarker@barker.dropbear.id.au>
Co-authored-by: Peter Barker <pbarker@barker.dropbear.id.au>
| The receiving component must emit PARAM_VALUE if the parameter can be read, or PARAM_ERROR if there is an error (older protocol versions send no response on error). | ||
| A PARAM_VALUE response must include both the `param_id` and `param_index`. | ||
| A PARAM_ERROR response must include the `param_id` or `param_index` included in the request, and should include both if available. |
There was a problem hiding this comment.
@peterbarker FYI this is updated in response to your comment https://github.com/mavlink/mavlink/pull/2365/files#r2476014580
Co-authored-by: Peter Barker <pbarker@barker.dropbear.id.au>
|
Thanks @peterbarker I think I have addressed your comments. Time for another review. |
Updated descriptions for PARAM_REQUEST_READ, PARAM_REQUEST_LIST, PARAM_VALUE, PARAM_SET, and PARAM_ERROR messages to clarify usage and response requirements.
This makes the descriptions more coherent and consistent so that they are better cross linked.
Also clarifies that the index is not necessarily invariant.
The reason for the change comes out of #2344 (comment) - clarifying what values must be set for parameter identity in the emitted command. I chose:
So you always return the id (name) if you have it, because that's the invariant thing we really want. However sometimes you can't have it for example in param_error, if you requested an index out of range.
We don't care about the index in the setter case; as long as it is correct.
@peterbarker I think this is good and makes things more clear.