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.

Sign up for GitHub

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

Jump to bottom

Add diagnostics #820

Merged
merged 8 commits into from
Sep 29, 2022
Merged

Add diagnostics #820

merged 8 commits into from
Sep 29, 2022

Conversation

tonynajjar
Copy link
Contributor

@tonynajjar tonynajjar commented Sep 26, 2022 •
edited

image

@tonynajjar tonynajjar mentioned this pull request Sep 26, 2022
Closed
@tonynajjar tonynajjar marked this pull request as ready for review September 26, 2022 11:33
bmagyar
bmagyar approved these changes Sep 28, 2022
View reviewed changes
Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

This sounds pretty neat to me!

destogl
destogl requested changes Sep 28, 2022
View reviewed changes
Copy link
Member

@destogl destogl left a comment •
edited

Choose a reason for hiding this comment

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

Thanks for the great feature!

I propose different names of the callback method to make it clear what it is. What do you think?

controller_manager/include/controller_manager/controller_manager.hpp Outdated Show resolved Hide resolved
controller_manager/src/controller_manager.cpp Outdated Show resolved Hide resolved
controller_manager/src/controller_manager.cpp Outdated Show resolved Hide resolved
controller_manager/src/controller_manager.cpp Outdated Show resolved Hide resolved
@tonynajjar
Copy link
Contributor Author

@destogl your proposals make sense, will apply them

@destogl
Copy link
Member

destogl commented Sep 28, 2022

@destogl your proposals make sense, will apply them

Sorry for killing formatting...

@tonynajjar
Copy link
Contributor Author

Sorry for killing formatting...

No problem I was already fighting with it anyway 😅. I think it should be fine now

@tonynajjar tonynajjar requested review from destogl and removed request for progtologist, erickisos, VX792, duringhof and mcbed September 28, 2022 19:49
@tonynajjar
Copy link
Contributor Author

I don't know if you have flaky tests, let me know if there is something more to fix

@destogl destogl enabled auto-merge (squash) September 29, 2022 08:14
destogl
destogl approved these changes Sep 29, 2022
View reviewed changes
Copy link
Member

@destogl destogl left a comment

Choose a reason for hiding this comment

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

Thanks!

@destogl destogl merged commit b1f31a1 into ros-controls:master Sep 29, 2022
@destogl destogl linked an issue Oct 1, 2022 that may be closed by this pull request
Publish controller states to diagnostics #819
Closed
@destogl
Copy link
Member

destogl commented Oct 15, 2022

@megifyio backport humble

@tonynajjar
Copy link
Contributor Author

@megifyio

@destogl looks like you have a typo

@tonynajjar
Copy link
Contributor Author

Also don't forget to backport #836 with it!

@AndyZe
Copy link
Contributor

AndyZe commented Nov 7, 2022

Can you guys fill me in on what this changes? Previously, the controllers would publish a status topic, right? What does this add?

@AndyZe AndyZe mentioned this pull request Nov 7, 2022
Closed
@tonynajjar
Copy link
Contributor Author

tonynajjar commented Dec 6, 2022 •
edited

@megifyio backport humble

@destogl this was never executed, could we backport this?

@tonynajjar
Copy link
Contributor Author

@megifyio backport humble

@destogl this was never executed, could we backport this?

Kind reminder, we would like to not have to clone this repo anymore on Humble for this feature

@bmagyar
Copy link
Member

bmagyar commented Jan 24, 2023 •
edited

Sorry guys

@bmagyar
Copy link
Member

bmagyar commented Jan 24, 2023

@Mergifyio backport to humble

mergify bot pushed a commit that referenced this pull request Jan 24, 2023
@mergify
Add diagnostics for controllers activity (#820)
6d5068f 
(cherry picked from commit b1f31a1)

# Conflicts:
#	controller_manager/src/controller_manager.cpp
@mergify mergify bot mentioned this pull request Jan 24, 2023
Merged
@mergify
Copy link
Contributor

mergify bot commented Jan 24, 2023

backport to humble

✅ Backports have been created

destogl pushed a commit that referenced this pull request Jan 24, 2023
@mergify @bmagyar
Add diagnostics (backport #820) (#922)
dca10f4 
* Add diagnostics for controllers activity (#820)

(cherry picked from commit b1f31a1)

Co-authored-by: Tony Najjar <tony.najjar@logivations.com>
Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
schornakj added a commit to schornakj/ros2_control that referenced this pull request May 8, 2023
@schornakj
Revert "Add diagnostics (backport ros-controls#820) (ros-controls#922)"
cafa8c2 
This reverts commit dca10f4.

Signed-off-by: Joe Schornak <joe.schornak@gmail.com>
schornakj added a commit to PickNikRobotics/ros2_control that referenced this pull request May 8, 2023
@schornakj
Revert "Add diagnostics (backport ros-controls#820) (ros-controls#922)"
d71f753 
This reverts commit dca10f4.

Signed-off-by: Joe Schornak <joe.schornak@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndyZe AndyZe AndyZe left review comments

@destogl destogl destogl approved these changes

@bmagyar bmagyar bmagyar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Publish controller states to diagnostics
4 participants
@tonynajjar @destogl @AndyZe @bmagyar