search

septentrio-gnss / septentrio_gnss_driver

ROS 1 & 2 driver for Septentrio GNSS & INS receivers
BSD 3-Clause "New" or "Revised" License
72 stars 37 forks source link

Bug Fix: IMU Orientation Mode (#1) #50

Closed rm-greenzie closed 2 years ago

rm-greenzie commented 2 years ago

This is a quick PR to fix a bug where ROSaic would always set the IMU orientation mode to SensorDefault

Motivation We had a lot of issues getting the SBi3 Pro+ working initially (both with and without using ROSaic). After contacting Septentrio Support, we learned that there is a a bug that requires the IMU orientation to be set to manual even if we are mounting the sensor in the default (0,0,0) orientation. That lead us to discover that ROSaic would always the orientation to SensorDefault 2022-01-28_10-55-02_screenshot

Solution The solution is fairly simple. rosaic_node.cpp already defines a variable manual_ that represents the IMU orientation mode. The issue was that the variable never got updated from its default value of false. This was resolved by adding a parameter the to yaml file and assigning it to the variable manual_

Feedback The main point I'd like feedback on is whether I should leave the default value of manual_mode to true. That is, have ROSaic set the IMU orientation to manual by default. It should be fine since the default values for each orientation angle are 0. Also, having it in true by default could prevent future users from falling into the same confusion about needing to always set the IMU orientation to manual (assuming that bug in the sensor persists)

thomasemter commented 2 years ago

Thank you for the detailed bug report and fix. We have also fixed this bug among others in the dev branch. The parameter manual_mode is removed there, i.e., orientation is always set to manual like you suggested.

The changes in the dev branch also introduce some new features and will soon be released but we still need some time for final tests and cleanup.

tibordome commented 2 years ago

We hope this issue is addressed in the new release. Please reopen PR if not.