Documentation for MAV_CMD variants #437
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Adds parsing for params into the MavProfile data structure. Add a table of paramters to all MAV_CMD variants including description, valid values and unit. Change xml parser to use expand_empty_elements=true, this removed a lot of duplicate code and prevents a some of potential bugs.
onur-ozkan
reviewed
Dec 1, 2025
Collaborator
onur-ozkan
left a comment
onur-ozkan
left a comment
There was a problem hiding this comment.
A snapshot test coverage would be nice.
Comment on lines
+784
to
+801
| match (self.min_value, self.max_value, self.increment) { | ||
| (Some(min), Some(max), Some(inc)) => { | ||
| if min + inc == max { | ||
| format!("{min}, {max}") | ||
| } else if min + 2. * inc == max { | ||
| format!("{}, {}, {}", min, min + inc, max) | ||
| } else { | ||
| format!("{}, {}, .. , {}", min, min + inc, max) | ||
| } | ||
| } | ||
| (Some(min), Some(max), None) => format!("{min} .. {max}"), | ||
| (Some(min), None, Some(inc)) => format!("{}, {}, .. ", min, min + inc), | ||
| (Some(min), None, None) => format!("≥ {min}"), | ||
| (None, Some(max), Some(inc)) => format!(".. , {}, {}", max - inc, max), | ||
| (None, Some(max), None) => format!("≤ {max}"), | ||
| (None, None, Some(inc)) => format!("Multiples of {inc}"), | ||
| (None, None, _) => String::new(), | ||
| } |
Collaborator
There was a problem hiding this comment.
This is very hard to follow and debug. Also, moving lines would change the behavior significantly (which is dangerous). Can we somehow re-write this in a better and safer way?
| } | ||
| params[param_index - 1] = MavParam { | ||
| index: param_index, | ||
| description: param_description.unwrap_or_default(), |
Collaborator
There was a problem hiding this comment.
Why not making description Option<String as well?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
MAV_CMD should get better support that the current method of manually having to fill the paramters fields. This adds the XML parsing part and used it for documentation but can later be used for codegen too.
Also simplify the XML parsing by treating self closing tags as an start and end tag, the self closing tag handling had just copied code anyways. This should also avoid some bugs like #393.
Example of new doc:
