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

RobotHWGroup Implementation #137

Closed

Conversation

kphawkins
Copy link
Contributor

I have implemented a RobotHWGroup which seems to match the spec we have been discussing. Currently, it doesn't support multiple identical interfaces across different RobotHW. It returns first the locally registered interface, or then one of the interfaces in one of the RobotHW, ordered by when each was registered.

@jenkins-auto-bot
Copy link

@kphawkins Awesome! One comment: to reduce the number of indirections between the derived classes, can you have the RobotHWGroup class completely override the implementation of get, instead of re-using the implemenation of get in RobotHW and have that implementation call through to your subclass. That would result in code that is easier to understand.

@kphawkins
Copy link
Contributor Author

Unfortunately, that is not possible. Template methods cannot be made virtual so a call toget<T>() inside of ControllerBase would always use the base version. That's why I used this convoluted technique.

@wmeeusse
Copy link

wmeeusse commented Jan 9, 2014

@kphawkins That makes sense, thanks for the explanation!

@kphawkins
Copy link
Contributor Author

Fixed interface aliasing issue: #138

@kphawkins kphawkins mentioned this pull request Jul 12, 2014
@bmagyar
Copy link
Member

bmagyar commented Oct 27, 2016

This I believe is now safe to close as #231 addressed this feature. Please open a new one if you have suggestions to improve that implementation.

@bmagyar bmagyar closed this Oct 27, 2016
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.

4 participants