osrf / capabilities

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

Allow provider specs to contain remapping rules #36

Closed wjwwood closed 10 years ago

wjwwood commented 10 years ago

This was proposed as a solution to the providers which need to rename things in order to satisfy semantic interface names.

See: #22

wjwwood commented 10 years ago

Example from @bit-pirate:

%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'
wjwwood commented 10 years ago

@bit-pirate, so there is a remappings section already for each remapping in the depends_on section, see this test provider:

https://github.com/osrf/capabilities/blob/master/test/unit/specs/providers/navigation_nav_stack.yaml#L10

Could this do what you want?

bit-pirate commented 10 years ago

For TB this would work, since most things will depend on the BringUp capability.

However, I would prefer to make the remapping parameter generally available (not tied to depends_on). Does anything speak against this?

wjwwood commented 10 years ago

I don't think so, I will add the option for a remappings section independent of the depends_on section.

In that case do we need the optional remappings section for the depends_on section too?

bit-pirate commented 10 years ago

In that case do we need the optional remappings section for the depends_on section too?

I'd say no.

wjwwood commented 10 years ago

Upgraded to a pull request.

@bit-pirate for review

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling fe171b337ad5923142917631edfaf844318a95b4 on issue_36 into da4d263751a86557e64c3a346f745aaac1b95b6a on master.

bit-pirate commented 10 years ago

Works like a charm. I have already implemented it in the app manager. Please merge it at your convenience.