-
Notifications
You must be signed in to change notification settings - Fork 26
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
Redesign nodelet support #42
Comments
@wjwwood has there been any progress on this? It's currently the major showstopper for a proper implementations on the TurtleBot. |
Ok, so what did we decided we wanted to do?
Does that sum up the idea? |
Yes. |
So this is my plan for this:
Any problems with this? |
I would say that adding a dependency on |
No. Sounds good to me.
True. However, can't think of an easy way to split this up. |
Ok, I opened this pull request to address this: #47 @bit-pirate for review |
Sorry for the delay. I finally reviewed this PR and realised that I was to quick with the statement above. I'd like request two additions, which I don't see covered by the above PR
In order to solve the above two issues, I suggest we implement a process in which a capability can let the server know it is a nodelet and wants to be loaded into the server's nodelet manager. In a similar manner I need be able to retrieve the fully resolved name of the server's nodelet manager, so that apps can load nodelets into the same manager. Please re-open this issue until we have cleared out the above concerns. |
I envisioned that everyone would use the parameter to avoid hard coding the name of the nodelet manager. I don't understand what you mean here, can you expand on why this leads to hardcoding and what you want/expect end users to do instead?
There isn't one, but I can add one.
What does the
That information is passed as an argument to the launch file of capability providers, and once I add a service call, there will be a way to externally get the |
I think I have misunderstood your code - your parameter remapping of
No, that would suffice. I'll run a test with both capabilities and apps, once that service is implemented and then provide feedback. |
Excellent, I don't fault you for not getting the gist right away, there isn't exactly any documentation for this yet. 😄
I'll try to get that implemented today. |
@bit-pirate Ok, I got the service call implemented. Please give the master branch a try and see if it works for you. |
As discussed here: #31 (comment)
The text was updated successfully, but these errors were encountered: