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

throttle TF_REPEATED_DATA to 10 seconds #516

Open
wants to merge 1 commit into
base: noetic-devel
Choose a base branch
from

Conversation

c-andy-martin
Copy link

The TF_REPEATED_DATA error, while important, can be overly verbose,
sometimes flooding the terminal scrollback buffer such that no other
output can be found. Throttle the error to 10 seconds. Implement the
throttling directly since console bridge does not implement throttling.
Because ros::Time may not yet be initialized when calling insertData,
use the stamp from the message as "now".

For more information including the justification for the warning and
the justification for a 10 second throttle, read the discussion on
issue #467:

#467

The TF_REPEATED_DATA error, while important, can be overly verbose,
sometimes flooding the terminal scrollback buffer such that no other
output can be found. Throttle the error to 10 seconds. Implement the
throttling directly since console bridge does not implement throttling.
Because ros::Time may not yet be initialized when calling insertData,
use the stamp from the message as "now".

For more information including the justification for the warning and
the justification for a 10 second throttle, read the discussion on
issue ros#467:

ros#467
@icefoxen
Copy link

icefoxen commented Oct 5, 2021

Please Eris yes. It would be nice to be able to upgrade a system from Melodic to Noetic and see the actual error messages, instead of literally spamming the logfiles into lagging out because someone decided that "works mostly correctly" is equivalent to "fatal error" except without the politeness of actually stopping the erroneous process.

@klaxalk
Copy link

klaxalk commented Jul 24, 2023

Yes, please, I totally second this. The warning is overly verbose, especially, when simulating.

@@ -118,7 +118,7 @@ class TimeCache : public TimeCacheInterface
L_TransformStorage storage_;

ros::Duration max_storage_time_;

ros::Time last_repeated_warn_time_;
Copy link
Contributor

Choose a reason for hiding this comment

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

This most probably wouldn't make it into Noetic. It changes the ABI of existing code, which would require a total rebuild of the whole Noetic buildfarm.

I suggest moving this completely to the .cpp . You can make this a static variable in the .cpp file, which would preserve ABI. It could also simplify the implementation.

If you no longer feel the urge to get this fix merged, ping @klaxalk , he told he could find some time to dedicate to this fix ;)

@peci1
Copy link
Contributor

peci1 commented Sep 29, 2023

Alternatively, this problem could be overcome by adding dependency on rosconsole_bridge and putting REGISTER_ROSCONSOLE_BRIDGE in some .cpp file that's always loaded. This would redirect all buffer messages to standard rosconsole, where the users could deal with the messages as they like.

However, I don't like this alternative too much - it would change the logging behavior someone might be relying on. Also, it would not allow to selectively discard only the TF_REPEATED_DATA messages.

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

Successfully merging this pull request may close these issues.

5 participants