Add option to publish heading as IMU message#186
Conversation
|
Thanks for your code.When calculating the quaternion for the newly generated IMU orientation, why not directly copy the quaternion from the heading, but instead recalculate a new value (q_imu = quaternion_from_euler(0, 0, -math.radians(data['heading']) + math.pi/2))? Thank you. |
It seems like the author is calculating a quaternion that complies to REP 103. The heading output is reporting a human friendly clockwise rotation from north rather than a counter-clockwise rotation from east. |
evenator
left a comment
evenator
left a comment
There was a problem hiding this comment.
Thank you for this contribution. I'm sorry I took so long to review it. I have a couple of comments that I'd like you to consider.
| def __init__(self): | ||
| super().__init__('nmea_navsat_driver') | ||
|
|
||
| self.publish_IMU = self.declare_parameter('publishIMU', False).value |
There was a problem hiding this comment.
I think it would be fine to remove this parameter. If a user doesn't want it they don't have to subscribe to it.
| # also publish as IMU-msg/interface (e.g. for robot localization pkg) | ||
| q_imu = quaternion_from_euler(0, 0, -math.radians(data['heading']) + math.pi/2) | ||
| current_heading_as_imu = Imu() | ||
| current_heading_as_imu.header = current_heading.header |
There was a problem hiding this comment.
Publishing the this heading with the same frame ID as the other heading could be problematic because they're in fact different coordinate frames.
There was a problem hiding this comment.
Sorry for only responding now. This is true, but the original heading is published in the sensor frame, which as the previous comment pointed out, does not comply with REP103. So to be overly correct the frame of the original heading should be a dedicated z-down frame (frame_id_ned): see : REP145 and REP103
I could go ahead and add an explicit "_enu" suffix to the message, to avoid ambiguity without changing original behavior.
4 participants
As this came back up for me I'm opening a pull request. It is based on #132, but without obsolete parts.
This PR enhances the driver by adding the option to publish the heading as a sensor_msgs/Imu message in ROS 2.
Changes
Introduced a new parameter:
publishIMU (bool, default: false) – enables or disables publishing of IMU messages.
Added functionality to publish an Imu message on heading/imu containing heading data when publishIMU is set to true.
Maintains existing behavior when the parameter is not enabled (no breaking changes).
Motivation
Certain applications require heading information to be available in the standard sensor_msgs/Imu format (e.g. robot_localization), allowing for easier integration with downstream components that already subscribe to IMU topics. This change provides that option without affecting users who only need the original GPS data.
Testing
Notes
The IMU message currently includes heading information derived from the NMEA data. Other IMU fields (e.g., linear acceleration, angular velocity) remain unset, just like the covariance matrix.
additional testing with further devices is very welcome!