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

footprint_filter: print less tf warnings #13

Merged
merged 1 commit into from
Dec 24, 2013

Conversation

v4hn
Copy link
Contributor

@v4hn v4hn commented Dec 18, 2013

On startup this filter produces about two pages of console output
(ROS_ERRORs) on ExtrapolationExceptions because the listener is not yet setup.
This commit reduces this to one warning.

@v4hn
Copy link
Contributor Author

v4hn commented Dec 18, 2013

In my opinion, the proper way to fix this is to wait for the requested transformation before update is called for the first time.
However, it seems to me that neither the configure function nor the constructor is a good place to do this...

@vrabaud
Copy link
Contributor

vrabaud commented Dec 22, 2013

hi, thx for trying to fix that inconvenience. I think it is dangerous to do it the way you did: what if the tf is lost later but for different reasons ? You then do not get any information. How about the simpler:

ROS_ERROR_THROTTLE(1, "Transform unavailable %s", ex.what());

@v4hn
Copy link
Contributor Author

v4hn commented Dec 22, 2013

Hey Vincent,

my pull-request works in the situation you describe.
For every successful transform I reset the transform_available_ flag,
so the warning is printed once each time the transform is lost.

I didn't know about *_THROTTLE. I still believe it's enough to print
the warning once every time a problem is first encountered, but if you
insist, _THROTTLE would work as well. A rate of .2 should suffice
for this message though; 1 would likely still produce more than one
message on startup (over here).

More importantly though, I do not consider missing tfs an error.
It happens quite often in ROS that tfs are not available unless you wait
for them explicitly. Imho waiting does not really make sense in this case
because it's called so often that it's probably better to drop a scan
than to wait for the tf. That's why I changed the macro to ROS_WARN.

Even one error on startup is too much in my opinion. It takes much more time
to look through the startup log output to see whether everything works as expected,
because "oh a red message" is no indicator for serious problems anymore.

Thanks for your time!

@vrabaud
Copy link
Contributor

vrabaud commented Dec 22, 2013

right, your fix works sorry. Now, there are two ways to behave I believe:

  • emit a warning (and not an error you're right) once but emit another one when things are back to normal (otherwise you never know when the problem disappears)
  • emit a warning as long as there is a problem (which is happening right now but throttle would help reduce the number of outputs) which helps witness a problem as it keeps popping up

I believe 2 is usually what's done: you get a warning whenever there's a problem and nothing otherwise (it's weird to get a warning when things are back to normal).

So please either add something in the log to say you are back to normal, or go with a throttle (if you go with it, a frequency of 1Hz does not seem crazy: you will end up have less than 10 warnings overall right ? if you get a real problem, it seems to me that 0.2 would make you notice it too late or it would get lost in the logs).

Opinions ? Thx ! I'm learning things :)

On startup this filter produces about two pages of console output
(ROS_ERRORs) on ExtrapolationExceptions because the listener is
not setup yet. This commit reduces this to throttled info messages
until the transform works for the first time.
@v4hn
Copy link
Contributor Author

v4hn commented Dec 22, 2013

I don't see why you would need a warning when things work (again),
because then you have a working pipeline and no need to look into
the log files anymore. But I see your point.

Concerning the "few" warnings on startup: I aim at a clean startup
without warnings. Even one warning is annoying, although much better
than two pages of errors..

I completely reworked the pull-request, please comment.

@vrabaud
Copy link
Contributor

vrabaud commented Dec 24, 2013

ok, I think your PR works as it is, thx for ironing out those details !

vrabaud added a commit that referenced this pull request Dec 24, 2013
footprint_filter: print less tf warnings
@vrabaud vrabaud merged commit a21f63b into ros-perception:hydro-devel Dec 24, 2013
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.

2 participants