ElettraSciComp / witmotion_IMU_ros

ROS wrapper for the family of IMU sensor devices manufactured by Witmotion Ltd.
MIT License
27 stars 31 forks source link

Added timeout parameter #26

Closed gsokoll closed 1 year ago

gsokoll commented 1 year ago

Per ROS2 test reports (https://github.com/ElettraSciComp/witmotion_IMU_ros/issues/15), the node consistently failed with a " Sensor error: Timed out waiting for data, please check device connection and baudrate!" when the sensor polling interval matched the IMU output rate. Investigating the code, this was occuring because the read data function was not correctly checking for the absence of new data. Changes to the underlying witmotion IMU QT library have been made to address this issue (refer https://github.com/ElettraSciComp/witmotion_IMU_QT/pull/7). This PR adds a timeout parameter to complement the changes made as part of the IMU QT PR.

The ROS2 launch mods (https://github.com/ElettraSciComp/witmotion_IMU_ros/pull/25) have been included in this PR as I neede them during testing of the ROS2 code.

twdragon commented 1 year ago

@gsokoll the pull request will be ready for merging after we will agree on my comment on the underlying PR. After the merge, would you create also the PR in ROS1 branch? The contribution itself seems great, so it is important to have it both in ROS1 and ROS2 branches, I see. The code should be very similar, practically the same

gsokoll commented 1 year ago

Sure. I have zero experience with ROS1, but I am happy to try.

gsokoll commented 1 year ago

I have prepared a ROS1 version in a local clone. Once the underlying PR in IMU_QT is merged in, I will submit the PR for this repo.

twdragon commented 1 year ago

@gsokoll the PR in the underlying library is merged, so it is possible to proceed.

gsokoll commented 1 year ago

Have cleaned up a few things I broke, and also submitted a matching PR for the ROS1 main branch. Both this and the ROS1 PR should be ok for a merge.

twdragon commented 1 year ago

@gsokoll can you please update also the underlying library remote submodule commit via git? Now the repository is set to 21e72c0 which is from March 2021, so the version of the library should match the current state of the leading repository. After that I do not see the obstacles to merge both PRs

gsokoll commented 1 year ago

Ok, try now.