Make handling of registered InterfaceManagers in InterfaceManager transparent #456
Conversation
There was a problem hiding this comment.
I don't work with combined_robot_hw so I'm not totally familiar with it or with the use-cases for this PR, but from your description in #452, this seems like a very safe and useful change.
Couple small requested changes, but overall looks great. Big +1 for adding tests. @bmagyar I would definitely like to hear your thoughts since you are much more familiar with the combined robot hw than I.
hardware_interface/include/hardware_interface/internal/interface_manager.h
Show resolved
Hide resolved
hardware_interface/include/hardware_interface/internal/interface_manager.h
Outdated
Show resolved
Hide resolved
hardware_interface/include/hardware_interface/internal/interface_manager.h
Outdated
Show resolved
Hide resolved
All InterfaceManager now handle registered InterfaceManagers transparently. This allows chains of multiple InterfaceManagers registered to each other to work corectly, mostly relevant for registering a manager from a combined_robot_hw RobotHW. Resolves ros-controls#452
Previously, trying to combine two non-ResourceManager interfaces yielded two identical error messages.
RobertWilbrandt
force-pushed
the
inconsistent_interfaces_452
branch
from
5bec84e
to
5488cf1
Compare
|
Thanks for looking through this. I tried applying all requested changes, and since there was some time since the initial pr i went ahead and rebased to current |
hardware_interface/include/hardware_interface/internal/interface_manager.h
Outdated
Show resolved
Hide resolved
hardware_interface/include/hardware_interface/internal/interface_manager.h
Outdated
Show resolved
Hide resolved
|
I applied the documentation changes. Regarding the formatting: Are there any guidelines for this? I tried keeping line-lengths etc consistent for the touched files, but other files are formatted completely different. Is there any preferred clang-format config (e.g. moveit)? |
|
It actually looks all good to me I was just hoping for a little more feedback from others. Let's go ahead with this though, I'll cherry-pick it to Noetic as well and push some releases |
This fixes #452. Most importantly, it handles registered
InterfaceManagerstransparently inside thegetNames()andgetInterfaceResources()functions (adding interfaces and resources from them and removing duplicates when needed). In addition, a test for this functionality is added and the documentation is updated (most of it didn't get updated since whenregisterInterfaceManager()was introduced). Finally, a small duplicate error message was removed.