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 diagnostics #10

Merged
merged 5 commits into from
Jul 12, 2016
Merged

Fix diagnostics #10

merged 5 commits into from
Jul 12, 2016

Conversation

efernandez
Copy link
Member

This basically remove old/deprecated variables and diagnostics from an old non-asynchronous implementation.

Note that the last commit is sort of WIP, or could be addressed later.

This addresses #5

@ipa-fxm @bmagyar

@efernandez
Copy link
Member Author

Ignore Travis CI, it's not really supported. I'm working on that too... see #9

@bmagyar
Copy link
Member

bmagyar commented Mar 14, 2016

LGTM 👍


stat.add("loop time in [sec]", status_.main_loop_time);
stat.add("data age in [sec]", status_.reading_age);
stat.add("current lock priority", static_cast<int>(status_.priority));

Choose a reason for hiding this comment

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

now only current lock priority is displayed (however, this works)
why is the Key-Value pair for current priority removed? Is this information not retrievable? (this would be the information I'm looking for)

current lock priority is still 0 in case one of the topics is unmasked

Copy link
Member Author

Choose a reason for hiding this comment

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

@ipa-fxm current lock priority replaces current priority.

The others are simply not used anywhere.

The current lock priority is related with the locks, not the topics. But I honestly didn't have time to look if I can make the diagnostics more clear.

Do you mind if I merge this and then we discuss how to improve it on an issue?
I'd like to have this merged for ROS kinetic release.

@efernandez
Copy link
Member Author

Merging...

@ipa-fxm Please create an issue if you still consider there's something missed after this.

My intention is to release a new version for jade and kinetic with this. Do you need this on indigo too?

@efernandez efernandez merged commit e7a4fb8 into indigo-devel Jul 12, 2016
@efernandez efernandez deleted the fix_diagnostics branch July 12, 2016 13:50
@efernandez efernandez mentioned this pull request Jul 12, 2016
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.

3 participants