osrf / capabilities

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

Prun caps without default provider #25

Closed bit-pirate closed 10 years ago

bit-pirate commented 10 years ago

Currently the capability server aborts, if multiple providers for an interface are available, but no default is set.

While this behaviour is all right when only few capabilities are available, it starts to annoy, when lots of capabilities with providers are discovered. In that case one has to set a default provider for each interface even though it is not used.

Suggestion: Instead of aborting the start of the capability server, we just prune the capability from the available list and through a warning.

On the flip side implementing this suggestion might lead to a noise capability server in the future, when the ROS community starts to produce various interfaces. At that point most of use would need to live with always getting lots of warnings.

This makes me think, if we should change the way capabilities are discovered. I like the current approach with exporting the capabilities in the package.xml a lot, since it is so convenient. However, looking at the above issue we might need to modify or completely change it. E.g. the app manager only parses given app lists.

wjwwood commented 10 years ago

Rather than change discovery, I would implement a white list, which could be another ROS parameter to the capabilities_server.

There are three cases to consider:

I would suggest this behavior, respectively:

In any of those cases, you could suppress interfaces using a black and/or white list.

bit-pirate commented 10 years ago

capability interface has more than one provider but no default is given

My it's just me being lazy, but couldn't that be just a warning as well? I'm getting easily annoyed by being forced to apply changes to some files to make a node start. :smile:

In any of those cases, you could suppress interfaces using a black and/or white list. So in case of many interfaces being locally installed, I would just added the packages containing the caps interfaces I use to the white list. Then all others would automatically be ignored and their related warnings would vanish. Correct?

wjwwood commented 10 years ago

I can do that, but I am thinking towards a finished system, where this case is likely to be an error, e.g. the user added a provider to a commonly used interface and now there is only a warning to inform them that the system no longer knows how to start the capability.

In the final system the solution is just always provide a default provider by ROS param.

I could make an option to elevate this back to an error which I would recommend robot developers use on the deployed system, but could be relaxed for development. Something like ~pedantic, which defaults to false, but should be set to true for production.

Does that sound like a good compromise?

bit-pirate commented 10 years ago

Something like ~pedantic, which defaults to false, but should be set to true for production.

Sounds like a good compromise - only I would use a different param name, maybe something more descriptive, such as missing_default_provider_is_error (a bit long though).

wjwwood commented 10 years ago

Ok, I'll implement this tomorrow.

wjwwood commented 10 years ago

Upgraded to a pull request.

coveralls commented 10 years ago

Coverage Status

Coverage remained the same when pulling 370895d2b6e2d70a8cdb1dba7215aa35bd6e2104 on issue_25 into 69ed148772cc9148e65818b7a47b75b3e93b9063 on master.

bit-pirate commented 10 years ago

@wjwwood is that white/black list still on your plan? I would appreciate this being available (not urgent though).

wjwwood commented 10 years ago

@bit-pirate I don't have an issue open for that, can you open one? I would open it, but I would like your take on how it should behave anyways.

bit-pirate commented 10 years ago

Sure. Just did.