-
Notifications
You must be signed in to change notification settings - Fork 308
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
Interface composition revised #285
Interface composition revised #285
Conversation
ResourceManager-based interfaces still need a default constructor.
6a95312
to
50b7790
Compare
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.
Sweet! 👍
I don't know if |
The cmake suggests those files are not getting installed although they are available to other packages from within a source-based installation. I think it is safe to remove. |
They get installed! The others need them to be included. In general the combined HW feature was not designed properly for reuse and maintenance :-/ |
Ah...I thought the What do you think about moving those header files into I wouldn't worry much about some hacky code breaking because those files are already marked internal and also pretty specific to this implementation. |
These are internal API files that are used (and implicitly exposed) by the public API. We could move |
The internal |
done, this is an API break ;) |
I approved, you merge ;) |
ros-controls/ros_control#285 (which is included in hardware_interface 0.13.0) allow us to use class `hardware_interface::HardwareResourceManager` API
As reported in PR2/pr2_mechanism#330 the changes introduced in #235 break hardware interfaces without default contructors.
This PR relaxes this by requiring the default constructor only for interfaces derived from
hardware_interface::HardwareResourceManager
.In addition I have cleaned-up the code a little bit and got rid of some warnings.
This breaks the API of
CheckIsResourceManager
.