mavlink / mavros

MAVLink to ROS gateway with proxy for Ground Control Station
Other
875 stars 989 forks source link

coordinate frame conversion on vision_position_estimate #225

Closed thedinuka closed 9 years ago

thedinuka commented 9 years ago

I know this has been discussed before (#49) but I think the way the orientation is converted from ENU to NED in the vision_position_estimate.c is incorrect. (However, the way the position is converted is correct). In lines 142 - 145

vision_position_estimate(stamp.toNSec() / 1000,
                                       position.y(), position.x(), -position.z(),
                                       roll, -pitch, -yaw); // ??? please check!

This is assuming that Euler angles can be swapped when the axes is swapped, but that assumption is not true. Euler angles have an order associated with them and unless you follow that order of rotation, your end result can be different from what is expected.So this prevents you from swapping roll and pitch angles just because the roll axis and pitch axis is swapped. The correct way to do this I believe is to derive the right rotation matrix for NED and then convert from rotation matrix to Euler angles as follows:

tf::Matrix3x3 rot_ned2enu = [0, 1, 0,
                                             1, 0, 0,
                                             0, 0, -1];
tf::Matrix3x3 rot_enu2body(orientation);
tf::Matrix3x3 rot_ned2body =  rot_enu2body*rot_ned2enu;
rot_ned2body.getRPY(roll, pitch, yaw);

Note: I'm not sure whether orientation is equivalent to rot_enu2body or rot_body2enu. I guess it depends on the ROS node which does the actual pose calculation.

Additionally, I do not think that calculating RPY here and sending it to the FCU is probably not a good idea due to the singularities of Euler angles. Wouldn't it be better to send the quaternion directly?

mhkabir commented 9 years ago

@thedinuka We'll be sending the quaternion actually, as soon as the new message is done. See https://github.com/mavlink/mavlink/pull/333. That said, we still need to do the ENU-> NED here in ROS to maintain standards. Also, we're not sure about the current conventions, so contributions here would be welcome : https://github.com/mavlink/mavros/issues/216

tonybaltovski commented 9 years ago

@thedinuka The rotation matrix for body frames is rot_x(pi/2) = [1,0,0; 0, -1,0; 0, 0,-1]. The body frame and the inertial frame are different. The ROS body frame is defined as X is forward, Y is left and Z is up and multiplying will get you to the correct frame which is used.

LorenzMeier commented 9 years ago

Tony, do you have a link to the docs suggesting X is forward?

tonybaltovski commented 9 years ago

@LorenzMeier it's REP 103.

vooon commented 9 years ago

See also #216, i think is more general that just vision plugin.

congleetea commented 9 years ago

@thedinuka I change to vision_position_estimate(stamp.toNSec() / 1000, position.y(), position.x(), -position.z(), roll, -pitch, -yaw + 1.57); i.e change -yaw to -yaw+1.57. it works.

TSC21 commented 9 years ago

@congleetea did you get those values from try and retry or do you have a scientific explanation to it? have you tried to apply the same conversion to https://github.com/mavlink/mavros/blob/master/mavros/src/plugins/imu_pub.cpp#L183? What's the result?

TSC21 commented 9 years ago

@congleetea are you able to test https://github.com/mavlink/mavros/pull/208 and see if you can get the same results?

congleetea commented 9 years ago

@TSC21 I just test vision_pose, when conversing body frame from ENU to NED, we use (x y z)-->(x,-y,-z) (w,x,y,z)-->(x,-y,-z,w) so we get ENU ---> NED roll ---->roll pitch ----->-pitch yaw ----->-yaw. however, we mistake that the yaw is based on the x axis in world frame(when x in body framw is along with x in world frame , yaw is 0), and have 90 degree rotation. so we should -yaw + 1.57. btw, I use optitrack and remap the pose to vision_pose.

TSC21 commented 9 years ago

@congleetea thanks for the feedback. Two things:

  1. Can you please present and explain your solution in https://github.com/mavlink/mavros/issues/216 so it can be discussed and maybe consider it's application in the rest of the plugins?
  2. I added mocap support to the firmware at https://github.com/PX4/Firmware/pull/2317. So you can now use ATT_POS_MOCAP msg to send your optitrack data to the FCU, using, for example, the mocap_pose_estimate.cpp plugin of mavros. Testing is welcomed :)
congleetea commented 9 years ago

@TSC21 ok. I send it using mocap plugin first, however , I check .csv file in QGC, VICN.x y z have nothing. wether I need set something ? besides, the inav app doesn't fuse vicon message.

TSC21 commented 9 years ago

@congleetea what branch are you using? PX4/Firmware#2317 is still not merged so you have to use https://github.com/TSC21/Firmware/tree/mocap_support. The message being fused is ATT_POS_MOCAP not VICON_POSITION_ESTIMATE. So, in sdlog2, you should also check for MOCP.x, MOCP.y and MOCP.z (https://github.com/PX4/Firmware/pull/2317/files#diff-d92320704a8bd89ce2c077e7824982faR527)

congleetea commented 9 years ago

@TSC21 thanks, I use the master branch. I will test it !

TSC21 commented 9 years ago

Ok @congleetea. Please leave your test results in https://github.com/PX4/Firmware/pull/2317 and https://github.com/mavlink/mavros/issues/307. Log graphs mainly, but a video is welcomed :D

congleetea commented 9 years ago

@TSC21 ok.

TSC21 commented 9 years ago

Some concerns: check the weights on INAV and see if everything is according to what you need (mocap weight is at maximum, you may also reduce the weight of baro so mocap has more weight on altitude estimation).

congleetea commented 9 years ago

@TSC21 HI, can you build the mocap_support branch successfully? I can not build it. whether the command "git submodule update" get a incorrect version ?

TSC21 commented 9 years ago

@congleetea check new PR: https://github.com/PX4/Firmware/pull/2361. You should be able to compile now.

vooon commented 9 years ago

Does #317 fix that issue?