Define the exterior parameter API

[tfoote]

AC:

• Determine the external communication API
  • message types
  • topic names service names and semantics

---

[tfoote]

Messages defined at: https://github.com/ros2/rcl_interfaces/tree/master/rcl_interfaces

Not compiling at the moment though.

[dirk-thomas]

We should add:

• querying of parts of the hierarchy
• events for add / update / delete
  • both for an example use case of a user interface

We need to clarify:

• if a leaf and a path can overlap?
  • how does that later map to complex parameter types if we would allow complex messages
• how to distinguish that in the API?

[tfoote]

Added Events message and topic.

Differentiated parameters and parameter groups. Allowed recursive or non-recursive queries.

Standard topics documented in README

[dirk-thomas]

The README mentions ParameterEvent but the message file is called ParamEvent.

[dirk-thomas]

Can a ParamEvent event message contain the same parameter in multiple of the fields?
If yes, what is the resolution order? If no, that should be states in the message.

[dirk-thomas]

The CMake file uses builtin_msgs which is not listed as a dependency in the manifest.

[dirk-thomas]

With the background of our recent discussion sub_groups (https://github.com/ros2/rcl_interfaces/blob/437f5b25539ccd2226b4a36c731d74f7446a4601/rcl_interfaces/srv/ListParamsByPrefix.srv#L10) makes sense. But I think it needs a couple of sentences (in the message or readme) to describe what it is.

[dirk-thomas]

A parameter should support the type binary data.

[tfoote]

• Change to HasParams (multiple)
• Shorten ListParams name.
• Provide event stream of descriptions only
• Specify events will never be batched. All changes are published.
• Can change them all to Parameter instead of Param
• Add support for byte array.

[tfoote]

I've updated it to conform to the review notes. @dirk-thomas @wjwwood @esteve please review

[dirk-thomas]

The GetParameters service should only pass a list of parameter names rather then a description which includes the type. Same for HasParameters.

[dirk-thomas]

Do we want to include an optional "description" for a parameter? I think this was a highly requested feature in ROS 1. I am asking because I don't like the current term ParameterDescription. It might mislead users about what it contains (not a textual description). I am not sure yet what a better term would be.

[dirk-thomas]

For consistency I would suggest renaming a field and the enums in ParameterDescription:
rename the field parameter_type to just type and enums from *_PARAMETER to *_TYPE.
This also applies to the fields in services, e.g. descriptions in ListParameters.

[dirk-thomas]

I would suggest renaming the topic parameter_updates to parameter_events since an event does not only contain updates.

[dirk-thomas]

The current service HasParameters requests a list of parameters but only receives a single boolean. Shouldn't it be a list of booleans?

[dirk-thomas]

The comment in the ParameterEventDescriptions message only mentions a single description. @tfoote Should I provide a PR for those since it might become a lot of tiny comments?

[dirk-thomas]

Looking at other solutions storing arbitrary typed information like QVariant I am not sure if the current separation (ParameterDescription / Parameter) makes sense. E.g. userland code can not pass a QVariant-like structure along (without using the full Parameter message containing more information then necessary). If we would follow QVariant style we would need a separate message wrapping the type and the value. That class could then also provide reasonable conversion functions.

[tfoote]

We've designed the parameters to have declared types. This will allow us to fully specify and verify the API in the future. And all code interacting with parameters will not require runtime type checking. That's why the type is part of the description.

Related to the description, I group that with the declaration of the API and also supporting declaring ranges for a dynamic reconfigure type GUI. We chose to defer that. And I think it should remain decoupled from the actual accessing.

I don't want to use just "type" as a field name as it's a keyword in some languages. For the const names
if (parameter_type == DOUBLE_PARAMETER) makes more sense to me than if ( parameter_type == DOUBLE_TYPE) It's clearly scoped to be a parameter, there's no worry about someone using a similarly named but differently ordered enum. I could also do DOUBLE_PARAMETER_TYPE but that's getting very verbose.
What do you mean about the definitions in ListParameters? There are no types defined there?

parameter_events seems fine.

The HasParameters I had as a parallel to setParameters with it being atomic. But I think you're right to let it be a full list of bools.

@dirk-thomas sure individual pr's for discussing specific things might make sense in the future. This is tough that we can't do inline/threaded comments on this.

I'll fix the plural descriptions.

I think considering providing the QVariant type of functionality makes sense in the developer API, but this API is for the nodes to talk behind the scenes it should never be converting or changing type, and we can't support that on the wire anyway.

[dirk-thomas]

The problem with requesting descriptions (names with types) is that you can not use the service GetParameters without calling ListParameters before if you only know that the parameter type is a number (but could be int or float).

Also you could invoke HasParameters with a parameter name and a type x and get the response False because the parameter only exists with a different type.

Therefore I would argue these services should only request the names and not the types. That by no means implies that we don't declare the type or that verification of the API becomes more difficult / impossible.

[dirk-thomas]

Regarding the description I would expect that future extensions (like a range or textual description of a parameter) would be added to the ParameterDescription message. And it is certainly not desired to call a service like GetParameters with all the information from this description message.

[dirk-thomas]

For the ListParameters service I referred to the response field parameter_descriptions which could be shortened to descriptions.

[gerkey]

@wjwwood suggests having HasParam return the type, perhaps in the ParameterDescription (if it exists), or false/not set (if it doesn't exist). But keep in mind efficiency of the interaction, so as not to unnecessarily slow down parameter-related queries.

[tfoote]

rcl_interfaces defined in several other follow up tickets.