Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

Add more logging to publisher update calls.#979

Merged
dirk-thomas merged 3 commits intoros:kinetic-develfrom
deng02:logging-publisher-updates
Feb 9, 2017
Merged

Add more logging to publisher update calls.#979
dirk-thomas merged 3 commits intoros:kinetic-develfrom
deng02:logging-publisher-updates

Conversation

@deng02
Copy link
Copy Markdown
Contributor

@deng02 deng02 commented Feb 8, 2017

Because the publisher update calls are critical for setting
up topic connections between subscribers and publishers,
have added more detailed logging messages. The publishers
are included so you know what the subscriber was told, as
well as when the call succeeded or failed and if so why.
The rosout topic is filtered out to reduce noise.

Because the publisher update calls are critical for setting
up topic connections between subscribers and publishers,
have added more detailed logging messages.  The publishers
are included so you know what the subscriber was told, as
well as when the call succeeded or failed and if so why.
The rosout topic is filtered out to reduce noise.
@mikepurvis
Copy link
Copy Markdown
Member

Filtering events for the rosout topics seems a bit arbitrary. I can appreciate that this could be noisy with every single node bringing up a rosout publisher, but I'm not sure it's worth it.

@mikepurvis
Copy link
Copy Markdown
Member

@ros-pull-request-builder retest this please

@deng02
Copy link
Copy Markdown
Contributor Author

deng02 commented Feb 9, 2017

The rosout topic is "special" because every node automatically publishes to it and it's brought up automatically by the system, hence the special treatment. So I'd argue less arbitrary and more intentional. However I can agree that special treatment in code is not always desirable and after testing, I don't feel this buys us much in terms of data volume savings. I will remove it.

@dirk-thomas
Copy link
Copy Markdown
Member

I added the commit 34c57ac which moves some code to keep variables and comments closer to where they are used as well as reduce duplication. Since I only wrote the patch in the GitHub web interface it would be great if you could confirm that it still achieves your goal.

@deng02
Copy link
Copy Markdown
Contributor Author

deng02 commented Feb 9, 2017

@dirk-thomas Confirmed: output with commit 34c57ac achieves my goal.

@mikepurvis
Copy link
Copy Markdown
Member

LGTM 👍

@dirk-thomas dirk-thomas merged commit 91dbc09 into ros:kinetic-devel Feb 9, 2017
@dirk-thomas
Copy link
Copy Markdown
Member

Thank you for the contribution.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants