-
Notifications
You must be signed in to change notification settings - Fork 17.6k
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
Add Mount option to retract on landing #26787
base: master
Are you sure you want to change the base?
Conversation
@bitmask metadata documentation is missing |
|
||
#if HAL_MOUNT_ENABLED | ||
{ | ||
const bool is_landing = flightmode->is_landing(); |
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.
This is fine. An alternative implementation (which I suspect you also thought of) was to pass the is_landing() state into the mount and let it decide if the state has changed. It's no big deal but done that way we might move a bit more code into AP_Mount if we choose to implement this for Plane as well.
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.
Yep, I certainly did consider that. But then I thought that other libraries might also want to know when we've just transitioned into landing. Kind if figured we'd see which way things went.
It also briefly crossed my mind to use Notify events but quickly discarded that concept.
e0dc8a0
to
d942648
Compare
Fixed, thanks! |
How urgently do you want this capability in? We've got a similar feature that is vehicle-agnostic that:
Haven't pushed a PR for it yet because we never flight tested it enough (but we do have SITL tests). |
Well, given this isn't merged, can't be that much. ATM we're using modifications to MAVProxy to accomplish our goals, but it would be nice to do this in AP.
Definitely interested in seeing those patches. |
This looks mergeable to me as-is. Maybe first person to raise the PR wins and we merge this? |
See also #12042 - which might be closed when we see your PR :-) |
d942648
to
11e68c7
Compare
No description provided.