-
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
Move messages and services into separate package or repo #20
Comments
I can do this, but which dependencies are you concerned about? |
Are we sure this is necessary, is there a compelling case for splitting these up? I would prefer to not make new packages unless there is a good reason. Normally I would agree that splitting up message generation would make sense, but in this case the |
Well, basically I'm trying to keep the dependency of the app manager on the capability server/capabilities low. Preferable would be no dependency at all. I'm already taking care in the implementation, that the app manager runs fine even without a capability server being there. The capabilities package looks very light-weight and as you said, brings in very few dependencies. So, I'm not sure, if this is really an issue. Looking further, I don't see many nodes ending up depending on the capabilities package (nor a capablities_msgs package). The only nodes doing so would probably be similar in functionality to the app manager. Can you think of other use cases? |
Well, I am looking at bringing in bondpy too, now, but the app server will have to depend on the capabilities python API anyways, or copy the code where I use bondpy to create a bond. Either way it looks like to implement this reference counting correctly the app server will have to depend on bondpy (and probably already depends on rospy and roslaunch). So, like I said I am happy to do this if/when you decide we need it. The only other system that will use the capabilities that I can think of would be:
|
I'm gonna close this for now, if in the future you think this is required please just comment here and I'll reopen it. |
in order minimise dependencies, e.g. for the app manager.
The text was updated successfully, but these errors were encountered: