Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove node ros::Rate period auto computation and only read it from a node parameter #47

Closed
rsiryani opened this issue Aug 20, 2021 · 0 comments
Assignees
Milestone

Comments

@rsiryani
Copy link
Contributor

ROS should handle the node at least two time faster than the highest frequency output message.
This is mandatory to make sure two consecutive messages dont' get the same ROS time.

For now, it's computed based on device output messages configuration but the implementation is complex and not safe.
Moreover, the current implementation doesn't correctly handle IMU Short messages.

We should also make sure it's working correctly with High Performance INS that doesn't expose output message configuration.

As a result, I recommend setting this only from a node parameter and updating the documentation to properly explain this.

The default value should be set to 400 to support standard SBG INS that output data at 200 Hz.

@rsiryani rsiryani added this to the Rework Time milestone Aug 20, 2021
@rsiryani rsiryani added this to To do in Revamped ROS driver via automation Aug 20, 2021
@rsiryani rsiryani assigned bsaussay and unassigned mzembsbg Aug 25, 2021
@bsaussay bsaussay moved this from To do to In Progress in Revamped ROS driver Aug 25, 2021
@bsaussay bsaussay moved this from In Progress to Done in Revamped ROS driver Aug 25, 2021
bsaussay added a commit that referenced this issue Aug 25, 2021
#47 Remove the computation of the node rate frequency
bsaussay pushed a commit that referenced this issue Aug 27, 2021
@bsaussay bsaussay linked a pull request Aug 27, 2021 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

3 participants