osrf / capabilities

Implements the concept of capabilities as part of the robots-in-concert system.
Other
8 stars 26 forks source link

Support for providers implementing multiple interfaces #22

Closed bit-pirate closed 10 years ago

bit-pirate commented 10 years ago

For example the Kobuki driver aka kobuki_node (could) implements multiple interfaces, e.g. DifferentialMobileBase, Gyro, Button, LED etc.

Do we already cover such cases?

wjwwood commented 10 years ago

Look at how TB2Bringup is implemented.

wjwwood commented 10 years ago

You can have each of those capabilities just depend on another capability, say KobukiBringup, which is responsible for starting the kobuki_node.

wjwwood commented 10 years ago

I updated the title to Support for providers implementing multiple interfaces from Support for nodes providing multiple interfaces.

wjwwood commented 10 years ago

This feature would significantly complicate the internal implementation of the capability server. I would prefer to leave it out for now as there is a way to represent this using another capability interface. This might be a feature worth supporting in the future, but from my perspective it is syntactic sugar, which I would not prioritize over other issues with the system.

Thoughts?

bit-pirate commented 10 years ago

I agree with your approach. I'll give more feedback once I get around implementing your previous suggestion.

bit-pirate commented 10 years ago

I implemented an initial set of caps for Kobuki (https://github.com/yujinrobot/kobuki/tree/capabilities_integration/kobuki_capabilities)

So far I didn't run into any show stoppers when following your tb2 example.

I'd like to leave this issue here open for a bit longer until I can implement the full support for the Kobuki base and link upcoming related issues to here.

bit-pirate commented 10 years ago

Just ran into a new issue related to this, which is about remapping.

In the above Kobuki example there are several interfaces, which are actually all implemented by the kobuki_node, but currently represented by several "fake" providers all depending on the KobukiBringup interface (and provider). With this approach I see the issue that the provider's remappings can't be applied except for the one started first.

For example, starting the DifferentialMobileBase provider first will launch the kobuki node with the remappings for that interface. Remappings for the other interfaces, e.g. LED, can't be applied after that.

How could we solve this? updated/corrected by the next comment

This is still valid: Question: Shouldn't "fake" providers all be launched (set to be running), when their dependency is launched? That would represent the reality, e.g. starting the kobuki_node (KobukiBringup) already enables several capabilities.

However, I'm not sure, if this idea would catch all corner cases.

bit-pirate commented 10 years ago

Sorry, I forgot that we decided for now that the providers adhere to the interfaces. I.e. they are responsible for providing the topics specified in the interface - either by default or via roslaunch remappings.

So, adjusting the above statement: For a provider implementing multiple interfaces the current approach would mean the remappings necessary for all implemented interfaces need to be applied right from the start.

However, how will we then cope with semantic interfaces? E.g. KobukiBringup implements LED as well as the semantic interfaces LED1 and LED2.

Currently, one solution would be to set up a default remapping from /mobile_base/commands/led1 to /led for LED and LED1 specifying /led as its topic, while LED2 specifies /mobile_base/commands/led2. Doesn't look like a clean solution to me though.

wjwwood commented 10 years ago

I just spent this morning digging into options for remapping a running node. The current system doesn't allow for this.

I think the best option for remapping topics is to use the relay topic tool:

http://wiki.ros.org/topic_tools/relay

I believe this is actually really efficient because it doesn't deserialize the message before passing it along.

However, that only allows us to deal with topics. I can imagine a system for doing the same with parameters, but services, actions, and dynamic parameters seem problematic.

A final option is to add specific support for this in the Kobuki node. We could have a service on it like ~remap which would take topic names to and from and internally would perform the remapping. This could also be applied to services and actions, but still not dynamic parameters. But this doesn't scale very well.

All of these options sound like bad options, but the last one seems most complete. You could even imagine that we setup a library to make providing these services easy and transparent, but that will likely take a bit of time to implement.

Thoughts?

bit-pirate commented 10 years ago

I just spent this morning digging into options for remapping a running node. The current system doesn't allow for this.

Sounds to me like this topic should be on the list for ROS 2.0. Are there any plans addressing this?

I think the best option for remapping topics is to use the relay topic tool:

http://wiki.ros.org/topic_tools/relay

I believe this is actually really efficient because it doesn't deserialize the message before passing it along.

What about bandwidth? Relaying high bandwidth topics, such as point clouds, would put extra load on the system, no? The TB laptops already have a hard time coping with the Kinect data.

A final option is to add specific support for this in the Kobuki node. We could have a service on it like ~remap which would take topic names to and from and internally would perform the remapping. This could also be applied to services and actions, but still not dynamic parameters. But this doesn't scale very well.

I'm not in favour of this custom solution either. Also, this situation will happen more often in the future, I believe, e.g. create node, probably also navigation and moveit.

One option could be to restart the node responsible for the conflicting topics with the correct remappings, whenever new capabilities are started. This might work for now, since we limit ourself to one app at a time. Later it probably would play havoc with the running apps , i.e. each new app would require the running ones to restart, since their capabilities are restarted.

Another option would be using a feature, which we have discarded for now. That is allowing providers and semantic providers to specify their own topics for the interface they implement. E.g. The provider for LED specifies /mobile_base/command/led1 for /led instead of remapping at start to /led. The provider for KobukiLED1 would to the same and the provider for KobukiLED2 would specify /mobile_base/command/led2 for /led.

Positive side effects: This would remove the requirement for remappings done by the capability server and it would be already supported by the app manager.

What do you think?

wjwwood commented 10 years ago

Sounds to me like this topic should be on the list for ROS 2.0. Are there any plans addressing this?

Yes, a more introspectable and dynamic computation graph is near the top of the list, but this is not likely something that will be implemented in the near term, as it implies changing the entire middleware discovery, negotiation and connection protocol.

What about bandwidth? Relaying high bandwidth topics, such as point clouds, would put extra load on the system, no? The TB laptops already have a hard time coping with the Kinect data.

Certainly, but sending data over the local loop back with TCP is nearly as fast as shared memory (Assuming you also copy into and out of shared memory like you do for read and write buffers on the socket). The real cost you are used to seeing is the serialization/deserialization of these large data structures, which would be skipped in this case.

Either way this is an incomplete solution at best and doesn't address at all nodelets.

One option could be to restart the node responsible for the conflicting topics with the correct remappings, whenever new capabilities are started.

This also doesn't address requests for both the LED capability and the LED1 semantic capability at the same time. In that case you don't want to remap, but really you want to setup something like topic aliases or symbolic links so that the kobuki node can service both subscribers.

Another option would be using a feature, which we have discarded for now. That is allowing providers and semantic providers to specify their own topics for the interface they implement. E.g. The provider for LED specifies /mobile_base/command/led1 for /led instead of remapping at start to /led. The provider for KobukiLED1 would to the same and the provider for KobukiLED2 would specify /mobile_base/command/led2 for /led.

This sounds like a reasonable solution for the app server in the case where the consumers are outside of the rosmaster that the capability is on, i.e. communication is establish through the app server's flipping of topics.

I am willing to go down that route, what would that need to look like? Would this only be for semantic capabilities? I don't suppose all providers would need to provide this mapping, if they naturally fulfill the interface?

bit-pirate commented 10 years ago

in the case where the consumers are outside of the rosmaster that the capability is on, i.e. communication is establish through the app server's flipping of topics.

To me this seems also needed for the case, when using the same rosmaster in order to solve the remapping issue.

I don't suppose all providers would need to provide this mapping, if they naturally fulfill the interface?

Correct. That's why I would make this remapping optional.

Would this only be for semantic capabilities?

No. Should be usable for both normal and semantic capabilities.

I am willing to go down that route, what would that need to look like?

I guess, adding remapping parameters to the provider specification would suffice.

E.g.

%YAML 1.1
---
name: kobuki_led
spec_version: 1
spec_type: provider
description: "Implements the LED capability for Kobuki's default LED, i.e. LED 1."
implements: kobuki_capabilities/LED
depends_on:
    'kobuki_capabilities/KobukiBringup':
        provider: 'kobuki_capabilities/kobuki_bringup'
remappings:
  topics:
    'led': '/mobile_base/commands/led1'
bit-pirate commented 10 years ago

I implemented most parts on the app manager side to support provider remappings. I'll finish that as soon as there is support for them in the capability server.

wjwwood commented 10 years ago

Ok, I'll probably spend tonight and tomorrow morning on implementing the provider remappings.

bit-pirate commented 10 years ago

:+1:

wjwwood commented 10 years ago

Closing in favor of https://github.com/osrf/capabilities/issues/36