-
Notifications
You must be signed in to change notification settings - Fork 310
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
[Diagnostics] Add diagnostics of execution time and periodicity of the controllers and controller_manager #1871
[Diagnostics] Add diagnostics of execution time and periodicity of the controllers and controller_manager #1871
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I successfully tested the new diagnostics with the demos (synchronous controllers), but as I haven't used them much a note for myself (would have been valid with #1764 already)
- per default, /diagnostics is sent with 1Hz by the Updater, we don't set a different value in the CM's constructor
- diagnostics_updater_ lives in a non RT-thread, and so do also the callback methods.
One comment as we are adding more and more parameters: Would it make sense to use generate_parameter_library also for the CM? I think we can't use it for the per-controller parameters because maps do not work well here, and there is no way to "namespace" parameters with GPL. But for the others?
@christophfroehlich yes, I thought of using the GPL for this, but I wanted to see your opinion here before taking any decision. I'm open to having this |
…ith more information
30744f3
to
faf9c75
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1871 +/- ##
==========================================
- Coverage 87.83% 87.59% -0.25%
==========================================
Files 121 122 +1
Lines 12555 12766 +211
Branches 1131 1146 +15
==========================================
+ Hits 11028 11182 +154
- Misses 1108 1155 +47
- Partials 419 429 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
controller_interface/include/controller_interface/controller_interface_base.hpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks really good but I posted a few questions sir
Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you!
This PR aims to add diagnostics to the controller_manager and the loaded controllers regarding their periodicity and execution time.
Needs: ros-controls/realtime_tools#192
The diagnostics should kinda look like the following screenshots (The following are only to look at the summary):