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

Fix warning "Failed to meet update rate!" #602

Merged
merged 2 commits into from Oct 12, 2020

Conversation

mjs973
Copy link

@mjs973 mjs973 commented Oct 11, 2020

This patch reads the parameter server key silent_tf_failure only once at startup. It resolves #601.

Commit EDC5BEA from April 2020 causes an access to the ros parameter
server for every IMU packet that is processed. If the ros parameter
server is not local, the fetch time can lead to lost input messages,
and erratic timing of filter updates.

The fix is to fetch the parameter silent_tf_failure only once at startup.
This addresses issue cra-ros-pkg#601.

Signed-off-by: Mike Scheutzow <mjs973@hotmail.com>
@@ -476,6 +476,10 @@ template<class T> class RosFilter
//!
bool useControl_;

//! @brief Whether or not to print warning for tf lookup failure
//!
bool silent_tf_failure_;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for both catching and addressing this issue.

Given that this is against melodic-devel, and we're still using the old (and technically incorrect) style for variable names in this branch, would you mind making this consistent with the others, and changing this to silentTfFailure_? I'll make sure it matches the style when I cherry pick it into the other branches. Thanks!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My branch has been updated with this change.

Signed-off-by: Mike Scheutzow <mjs973@hotmail.com>
@ayrton04 ayrton04 merged commit 2a03b08 into cra-ros-pkg:melodic-devel Oct 12, 2020
@ayrton04
Copy link
Collaborator

Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to meet update rate!
2 participants