nmea_navsat_driver icon indicating copy to clipboard operation
nmea_navsat_driver copied to clipboard

If heading available, publish IMU msg as well

Open jediofgever opened this issue 2 years ago • 4 comments

I think it’s often that people of robotics use this driver with robot_localization package, IMU messages are accepted for state estimation nodes but in the current form heading is only Published as QuaternionStamped, it’s convenient if the quaternion is just pushed into IMU no ?

jediofgever avatar Aug 12 '21 20:08 jediofgever

Publishing an IMU message from a device that isn't an IMU seems misleading, so I'm not sure about this. If any other users or developers have an opinion, I'd welcome it.

evenator avatar Aug 15 '21 16:08 evenator

That’s reasonable, like I said, this was purely aimed to make easier integration of heading(if avail) with robot_localization.

jediofgever avatar Aug 15 '21 17:08 jediofgever

Since you're you asked for comments @evenator - yes, it seems misleading at first glance, but in case the GNSS poses shall be used in sensor fusion it is crucial to get them into the fusion with as few as possible intermediate steps in order to reduce the delay. And robot_localization as the probably most commonly used generic ROS framework for these applications relies on IMU messages for orientation input. Therefore I also included the IMU messages in my fork, e.g. here: https://github.com/boschresearch/nmea_navsat_driver/blob/master/src/libnmea_navsat_driver/driver.py#L704-L713

However when including a generic IMU message for heading purposes one has to be careful with the coordinate system conventions, so angular offset and rotation direction. Some GNSS receivers provide options to adjust these, others not. Some of these issues can be handled via TF, others not. One example: for GNSS receivers heading is usually calculated clockwise (latitude -> longitude), but ground based robots usually use an ENU coordinate system with counterclockwise (east -> north) rotations around the yaw axis. Since things can get messy here I'd personally prefer rotation direction inversion and offset parameters, which can be set via launch file.

ggaessler avatar Aug 16 '21 06:08 ggaessler

Based on @ggaessler's comment, I think providing an IMU message is reasonable. If you can clean up this PR as requested (remove extraneous changes and ensure all files are PEP8 formatted), I can merge it.

evenator avatar Oct 26 '21 15:10 evenator

Is there still an interest in this? If so, i created a version from a fresh fork https://github.com/ros-drivers/nmea_navsat_driver/compare/ros2...tschuette22:nmea_navsat_driver:ros2 that uses an additional parameter to toggle the IMU messages. I could add the additional parameters for arbitrary offset and rotation direction as suggested by @ggaessler. Right now, it's assuming clockwise rotation of the heading reported by the receiver and zero at north, which is transformed to counter-clockwise, zero at east.

tschuette22 avatar Feb 11 '23 19:02 tschuette22

I'm closing this PR as abandoned. @tschuette22 if you'd like to open a PR from your fork, we can consider that.

evenator avatar Dec 26 '23 16:12 evenator