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

Remove the need of a provider for capability dependency in another provider #38

Merged
merged 2 commits into from Nov 10, 2013

Conversation

wjwwood
Copy link
Member

@wjwwood wjwwood commented Nov 10, 2013

Currently, when specifying a capability dependency for a provider, a provider needs to be specified for that dependency. I'd like to make this optional.

I.e. I'd like to do this:

%YAML 1.1

---
name: tb_bringup
spec_version: 1
spec_type: provider
implements: turtlebot2_capabilities/TurtleBotBringup
launch_file: 'launch/tb_bringup.launch'
depends_on:
  'std_capabilities/DifferentialMobileBase'

But currently, I need to do this:

depends_on:
  'std_capabilities/DifferentialMobileBase':
    provider: 'kobuki_capabilities/kobuki_differential_mobile_base'

Otherwise I get an error, when this provider is started:

[ERROR] [WallTime: 1384062569.854299] RuntimeError: Capability Provider 'None' not found

IMO, setting (default) providers is the job of the capability server launcher or the service client.

@bit-pirate
Copy link
Contributor Author

Addition: This setting is also overriding the defaults.

@wjwwood
Copy link
Member

wjwwood commented Nov 10, 2013

It is supposed to use the default in that case so this is a bug.

However, since the robot developer is providing all of the provider files it should not be a problem to be specific, in fact I would encourage being specific except for were you explicitly need the flexibility.

I'll get this fixed.

@wjwwood
Copy link
Member

wjwwood commented Nov 10, 2013

In your above case I wouldn't think that you intend the tb_bringup provider to work with just any std_capabilities/DifferentialMobileBase provider, but instead it should be explicitly using the turtlebot's provider for that.

@wjwwood
Copy link
Member

wjwwood commented Nov 10, 2013

Upgraded to a pull request.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 177a3f9 on issue_38 into 4764d73 on master.

wjwwood added a commit that referenced this pull request Nov 10, 2013
Remove the need of a provider for capability dependency in another provider
@wjwwood wjwwood merged commit dd1c243 into master Nov 10, 2013
@wjwwood wjwwood deleted the issue_38 branch November 10, 2013 06:54
@bit-pirate
Copy link
Contributor Author

In your above case I wouldn't think that you intend the tb_bringup provider to work with just any std_capabilities/DifferentialMobileBase provider, but instead it should be explicitly using the turtlebot's provider for that.

No, I actually would like to allow any provider. In this specific example, the provider could be a kobuki-, create- or roomba_differential_mobile_base provider. Isn't that the idea of the caps/interfaces?

@bit-pirate
Copy link
Contributor Author

However, since the robot developer is providing all of the provider files it should not be a problem to be specific, in fact I would encourage being specific except for were you explicitly need the flexibility.

That's true. But in this example the developer focuses on implementing the provider of the TurtleBotBringup capability. It might be the same person, who implements the provider for the DifferentialMobileBase, but this should be a requirement. Without this requirement, we would be able to compose complex sets of capabilities using existing capabilities + providers (if they fit) as building blocks.

@wjwwood
Copy link
Member

wjwwood commented Nov 10, 2013

No, I actually would like to allow any provider. In this specific example, the provider could be a kobuki-, create- or roomba_differential_mobile_base provider. Isn't that the idea of the caps/interfaces?

Yes it is, I wasn't aware you were trying to achieve that, this is one of those cases where you explicitly need the flexibility.

There are two cases where you would use a provider which depends on another interface:

  • You are composing an interface using another interface
  • You are implementing an interface using another interface's implementation

In the first case you want to not specify a provider in the dependency because you don't care what provider is used as long as it implements the interface you depend on.

In the second case you probably do care which provider is being used because you don't actually need anything declared in the interface you depend on, you just need the things which are side effects of running its provider.

I don't really understand the case you have laid out above. Without understanding how you have laid out of everything it would be hard for me to understand it, but I would have made the provider for DifferentialMobileBase depend on TurtlebotBringup as a necessity (the implementation lives there), not the other way around. In fact without using TurtlebotBringup as a sort of phony implementation capability I don't understand why you need it anyways? What app will depend on TurtlebotBringup? What functionality does it provide, which is not already described in DifferentialMobileBase or some other interfaces?

@bit-pirate
Copy link
Contributor Author

  • You are composing an interface using another interface

That is what I am doing.

In the first case you want to not specify a provider in the dependency because you don't care what provider is used as long as it implements the interface you depend on.

Agreeing.

I don't really understand the case you have laid out above.

This picture might better explain this than my words.

20131111_turtlebot_capabilities

I broke the TurtleBotBringup apart and moved general/reusable functionalities out into their own capabilities, e.g. RobotStatePublisher and Diagnostics. TurtleBotBringup will probably be an empty place holder in the end, which just specifies a couple of capabilities the TB needs.

So, TurtleBotBringup depends on the DifferentialMobileBase capability, put the provider might be from Kobuki, Create or Roomba depending on the situation.

Does this clarify my need for the optional provider parameter/specification?

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.

None yet

3 participants