Skip to content
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

The doc of mavros_msgs about coordinate needs better clarification. #1500

Open
petertheprocess opened this issue Oct 18, 2020 · 4 comments
Open

Comments

@petertheprocess
Copy link

http://docs.ros.org/en/api/mavros_msgs/html/msg/PositionTarget.html
#1084 (comment)
Just like the @FaboNo said

the FCU is working with the NED frame and ROS is working with the ENU frame.
Mavros takes care of the transforms between the two frames.
So if you want to send a setpoint to the FCU, it must be defined in ENU frame (X forward and Z up), Mavros will do the transform in NED coordinates.
Conversely when you want to read the current pose of the quadrotor, you can use mavros/local_position/pose, mavros will do the transformation so that you will read the pose in ENU as well

While in the documentation in http://docs.ros.org/en/api/mavros_msgs/html/msg/PositionTarget.html, FRAME_LOCAL_NED and FRAME_BODY_NED are still used. It can misguide many beginners of mavros like me.
So I suggest to rename these FRAM CONSTANT like this:

old new
FRAME_LOCAL_NED FRAME_LOCAL_ENU
FRAME_LOCAL_OFFSET_NED FRAME_LOCAL_OFFSET_ENU
FRAME_BODY_NED FRAME_BODY_FLU
FRAME_BODY_OFFSET_NED FRAME_BODY_OFFSET_FLU
uint8 coordinate_frame
uint8 FRAME_LOCAL_NED = 1
uint8 FRAME_LOCAL_OFFSET_NED = 7
uint8 FRAME_BODY_NED = 8
uint8 FRAME_BODY_OFFSET_NED = 9
@amilcarlucas
Copy link
Contributor

Good point, can you please provide a github pull-request?

@shupx
Copy link

shupx commented Mar 12, 2021

i don't think it is a good idea to revise the frame constants in 'PositionTarget.msg'. Because these frame constants are also used in some mavros plugin cpp files such as 'setpoint_raw.cpp', which means you should revise all the mavros plugins that involves the 'PositionTarget.msg' in the meantime. so it is not necessary.
For beginners, this is confusing. However the README.md file in mavros has explained this very carefully.

@vooon
Copy link
Member

vooon commented Mar 12, 2021

I do not think that it's good change because it's copy of MAV_FRAME (old one...).
And also mavros is beyond 1.0.0, so i would rather keep API stable. It's ok to add new items, add comments, that won't make it incompatible with existing user's code.

@ViniciusAbrao
Copy link

It took me some hours until I find this post and to see that the README explain that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants