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

Hide canTransform console output #472

Closed

Conversation

smnogar
Copy link

@smnogar smnogar commented Oct 24, 2021

I got sick of seeing canTransform spam the terminal with the message:

Warning: Invalid frame ID "robot/base_link" passed to canTransform argument source_frame - frame does not exist
at line 156 in /Users/steve/Documents/ros2_galactic/src/ros2/geometry2/tf2/src/buffer_core.cpp

Obviously its desired behavior that canTransform should be checking for frames that don't exist yet. I'm sure there are cleaner ways of doing this but here is my method. Relevant issue is #405.

Copy link
Member

@aprotyas aprotyas left a comment

Choose a reason for hiding this comment

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

No comments on whether this change is desirable, but leaving some inline comments to address.

tf2/src/buffer_core.cpp Outdated Show resolved Hide resolved
tf2/src/buffer_core.cpp Outdated Show resolved Hide resolved
@clalancette
Copy link
Contributor

I don't think we should be hiding this completely. It's a useful indicator of whether things eventually get published that will complete the tf tree. Further, if we remove this now after so many years of having it in, people will wonder what happened when they upgraded.

That said, I do think that this should throttled. That is, we should only print it for a particular transform once every X number of seconds (we can argue about X, I think it should be ~5 seconds).

@smnogar
Copy link
Author

smnogar commented Oct 25, 2021

@clalancette Looking over the ros1 version of geometry2, it doesn't output anything when canTransform fails to find a frame. This was introduced in ros2.

I agree that functions like lookupTransform and other related functions should still output when a requested frame doesn't exist, but in the case of canTransform, a user may be waiting for a frame to exist before performing some action. Prior to this fix, every check of canTransform that returns false generates console error warnings.

@clalancette
Copy link
Contributor

@emersonknapp Can you provide a bit more context on the intent of #187 ? Having this console spam hasn't been super popular (see this PR, #395, #405, #396, and #358, at least), but I also don't want to remove it if it has a valid use.

@emersonknapp
Copy link
Contributor

I think the main point was that the existing error messages were not informative. I agree it's too spammy - maybe a "throttle" or "once on state change" would be more useful - but I don't really have any strong feelings any particular way.

@aprotyas
Copy link
Member

"once on state change"

+1

@smnogar smnogar requested a review from aprotyas November 4, 2021 03:01
@aprotyas
Copy link
Member

aprotyas commented Nov 4, 2021

@smnogar I don't think this is the way to proceed with this issue since it would suppress output even when it is both valid and warranted.

Since everyone is on the same page about the output being spammy, the design should follow either of the general ideas provided by @emersonknapp. If you want to try implementing that, go for it - but that should probably be submitted as a new PR.

@smnogar
Copy link
Author

smnogar commented Nov 8, 2021

I disagree that the output is warranted, since this kind of output was never present in ros1. I'll close this, and if someone else wants to implement the solution @emersonknapp suggested they can make a new PR.

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.

4 participants