-
Notifications
You must be signed in to change notification settings - Fork 14.6k
ArmingCheckRequest/Reply UORB messages - update docs. #24843
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
Conversation
msg/versioned/ArmingCheckReply.msg
Outdated
uint8 num_events # ? | ||
|
||
Event[5] events | ||
Event[5] events # ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think perhaps these are references to an array of Event topics, with num_events
indicating how many are published.
What are they for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the failures, i.e. if a mode cannot arm these contain the reasons sent to the GCS for display.
msg/versioned/ArmingCheckReply.msg
Outdated
uint8 num_events # ? | ||
|
||
Event[5] events | ||
Event[5] events # ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the failures, i.e. if a mode cannot arm these contain the reasons sent to the GCS for display.
Thanks very much! I'll try again next week based on this. |
b98d0d4
to
e4563b9
Compare
msg/versioned/ArmingCheckReply.msg
Outdated
bool mode_req_prevent_arming # Prevent arming (such as in Land mode). | ||
bool mode_req_manual_control # Requires a manual controller | ||
|
||
uint8 ORB_QUEUE_LENGTH = 4 # |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You said:
In uorb, the default is to have a single buffer which is overwritten when multiple samples get published too quickly. This allows access to multiple samples by the subscriber.
So is this uorb magic like the version number? - i.e. a uint8 with the name ORB_QUEUE_LENGTH
always does some magic under the hood to create a queue, such that if you subscribe to a topic you'll be able to get up to four instances of the message if you're slow at reading them, rather than the last to arrive?
If so, then this isn't documented in https://docs.px4.io/main/en/middleware/uorb.html#message-definitions - and we should cover this syntax. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes you are correct, it's generic, and should be documented there.
If set, it has to be a power of 2 (so 2, 4, ...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks very much @bkueng . I've gone through and updated this as I think is appropriate. By which I mean I think that this much information will be helpful for people that want to understand how this is used, and potentially to use it themselves or extend it.
But you might disagree, so can you please review closely.
W.r.t. registration I have mentioned this here, because I think that this makes it easier for people to work out where the ids come from
One missing field doc, which we may be able to live without
https://github.com/PX4/PX4-Autopilot/pull/24843/files#r2113320606
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks @hamishwillee
b8e79e7
to
bc050e4
Compare
bc050e4
to
a287bc1
Compare
Sorry @bkueng After that modification I need another approval. |
Self merging. The fail is unrelated, and I trust approval from Beat on this :-) |
We've been trying to standardize docs for uorb message - by way of examples see #24818, #24842, #24662 , #24789).
This is an update of ArmingCheckRequest and ArmingCheckReply.
This is very draft (now) because I don't understand how it all works, even at high level, and even after reading #19708
@bkueng This is not urgent, but would appreciate your help filling in all the missing bits. I've put some questions in line. Many of the updates are
?
placeholders.Solution
[<value>]
to mark units[@enum <value>]
to mark allowed values[@range <minValue>,<maxValue>]
to mark ranges - inclusive[@invalid <value>]
to mark the invalid/not-supplied valueChangelog Entry
For release notes:
Other comments
See inline notes