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

Single controller template (ROS Melodic) #329

Conversation

mikepurvis
Copy link
Contributor

@mikepurvis mikepurvis commented May 1, 2018

Successor to #302. Not a clean rebase for Melodic, so just making a separate PR for comparison purposes.

See discussion in the original PR. IMO this is a worthwhile change to get in for Melodic— it deprecates MultiInterfaceController, but otherwise should be fully backward compatible with source.

@mikepurvis mikepurvis force-pushed the single-controller-template-melodic branch from 891e4f4 to 9f941d3 Compare May 1, 2018 16:23
@mikepurvis mikepurvis force-pushed the single-controller-template-melodic branch from 9f941d3 to 5ba09cb Compare May 1, 2018 16:32
@mikepurvis mikepurvis changed the title WIP: Single controller template Single controller template (ROS Melodic) May 2, 2018
@bmagyar
Copy link
Member

bmagyar commented Sep 29, 2018

hi @mikepurvis , do you think this is ready to be rolled out?

…ks for gcc and clang. Original error is: (#1)

robothw_interfaces.h:60:68: error: typedef 'value_type' cannot be referenced with a class specifier.
@mikepurvis
Copy link
Contributor Author

@bmagyar I'd still love to see this go upstream at some point. We've been running it since it was proposed ~18 months ago, and it's a nice quality of life improvement taking advantage of variadic templates and enabling a more sane path to composite controllers.

That said, it looks like it has merge issues again, so I'll probably push yet another branch that includes these changes and the ones from #301. The branches don't integrate completely cleanly, so keeping them separate for the sake of upstream review ease isn't worth the ongoing pain for us.

@bmagyar
Copy link
Member

bmagyar commented Aug 1, 2019

Happy to merge a working version into melodic, please keep them coming :)

@mikepurvis
Copy link
Contributor Author

Closing in favour of #387.

@mikepurvis mikepurvis closed this Aug 1, 2019
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