search

dawonn / vectornav

ROS Interface for the VectorNav IMU/GPS
147 stars 179 forks source link

Default data publication form should be ENU #112

Closed nakai-omer closed 1 year ago

nakai-omer commented 1 year ago

Hello,

According to ROS REP-103 (https://www.ros.org/reps/rep-0103.html), ENU should be used:

For short-range Cartesian representations of geographic locations, use the [east north up](http://en.wikipedia.org/wiki/Geodetic_datum#Local_east.2C_north.2C_up_.28ENU.29_coordinates) [[5]](https://www.ros.org/reps/rep-0103.html#id9) (ENU) convention

Therefore, IMHO, the default tf_ned_to_enu should be true, and not false. Especially since libs such as slam-toolbox confront to the REP-103

dawonn commented 1 year ago

PR welcome.

nakai-omer commented 1 year ago

@dawonn I have created a PR, but after rethinking about it, it might break other people current behavior if they haven't configured tf_ned_to_enu at all. https://github.com/dawonn/vectornav/pull/116

dawonn commented 1 year ago

That's correct.

I thought you were talking about the ROS 2 version... Which I did make a significant effort to comply with REPs.

The ROS 1 version has a number of anomalies that might be better left as they were so we don't break the existing ecosystem.

dawonn commented 1 year ago

I'll leave this open for others to leave feedback on, but I'm leaning towards leaving it as is. It's not overly difficult to set that feature flag for those that want to have a more correct system.

nakai-omer commented 1 year ago

@dawonn I think there is also an issue in ROS 2. At rest the z linear_acceleration is negative, REP-145 states it should be positive:

When the device is at rest, the vector will represent the specific force solely due to gravity. I.e. if the body z axis points upwards, its z axis should indicate +g. This data must be in m/s^2.

Tom Moore, owner of the ROS robot localization package stated in one of his answers:

I think that for an ENU sensor, the Z axis points up, and on a flat surface, the acceleration should read +Z. This is because when it's on the ground, the IMU is undergoing acceleration is the direction opposite the gravity vector.
nakai-omer commented 1 year ago

I see there a commit from 2 weeks ago, that might of solve the last comment, I will pull the latest code and retry.

nakai-omer commented 1 year ago

UPDATE, it is looking good since the above mentioned commit. Since ROS 1 is irrelevant now, and ROS 2 works as expected, I am closing this.