Support specified frames of reference in landing target module #2011
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.
This change implements a check to support the set landing target frame of reference when using the "~/landing_target/raw" interface.
MAV_FRAME::LOCAL_NED
case are the same as the previous implementationMAV_FRAME::BODY_FRD
case have been derived by myself, and seem to work as expected under my testing. A cross-check is probably a good idea.One issue with this commit is in regards to handling the case where a use does not want to use the position data at all, and in doing so, is likely to not set the frame parameter. It is expected that this would result in a value of "0" being set, which aligns to MAV_FRAME::GLOBAL. This is not a valid setting for any autopilots I'm aware of, so I've used this is a selector for whether to use the position data or not.
Ideally this would be addressed by exposing "position_valid" in the
mavros_msgs/LandingTarget
, and going from there. An alternative could be to check if a valid quaternion is set.Another contentious point is the use of MAV_FRAME to interpret the frame from the message. This does not align with the enum in
mavros_msgs/LandingTarget
, however, I don't think this enum is well-aligned with either ROS options (which a user's perspective would be) or the MavLink options, which have the same names, but with different values for some.This could all probably be addressed in this pull request, I was just hesitant to break any message functionality. without some input. Ideas welcome!