Adding information for array parameters

[ayrton04]

This PR is intended to start a conversation about how we can (or perhaps whether we should) support array parameters in ROS2. It follows a request from @wjwwood in the rclcpp repo.

Please note that this PR does not address all of the points that were requested, but it meant to act as a starting point upon which we can iterate as I receive feedback.

I have proposed that we add support for the following array types:

• bool[]
• int64[]
• float64[]
• string[]

The use case for string[] feels a little weak to me, so I can understand if we want to remove that.

I would also advocate that, when we start discussing the constant names in rcl_interfaces, we consider renaming PARAMETER_BYTES to PARAMETER_BYTE_ARRAY for consistency.

[ayrton04]

I rearranged these to match the order of specification in rcl_interfaces as in the rclcpp code.

[ayrton04]

This link was going to articles/articles/interface_definition.html.

[Karsten1987]

nitpick: A . (dot) is missing here.

[ayrton04]

As in, referring to the current directory? Will add.

[ayrton04]

Or did you mean a full stop/period at the end of the sentence?

[wjwwood]

I think he means punctuation.

[Karsten1987]

yes, I meant punctuation.

[dirk-thomas]

If there is a byte[] type should there be a byte type too?

The current version of the idl article mentions byte as a candidate for removal (see http://design.ros2.org/articles/interface_definition.html#primitive-field-types). I don't think that is reasonable but I just wanted mentioned it here.

[ayrton04]

Interesting. I would argue that having byte[] is a nice feature, which would necessitate that we keep byte. I guess we could use the int64[] parameter type to load arrays of bytes, just as we might use int64[] to load parameters that are meant to be stored internally as int32[].

This raises another question: do we need to explicitly support arrays of every numerical primitive, or should we limit it to the 64-bit versions, and force users to cast them after the parameters are loaded?

[tfoote]

With parameters serializing to and from yaml we can't easily support numbers of different depths. So I'd advocate only supporting only the 64bit numeric types.

Sending byte arrays through parameters is pretty unportable. It requires both sides to implement their own serialization and deserialization without any container or metadata. Technically we support anything that can be transported as an XmlRpcValue however in practice we only really support strings, integers, booleans and doubles for parameters. I'd suggest that we not support byte or byte arrays as parameters. In addition to requiring serialization. It's something that non of our toolchain for interacting with parameters can easily support, such as rendering, diffing, or inputting for a gui like dynamic_reconfigure_gui.

[wjwwood]

We only support 64-bit ints and floats for parameters right? So I don't think we need 32-bit arrays of them. The bytes[] type is specifically for binary data. Personally I don't think we need a byte type, because you can just use an int64 for decimal values and a length 1 string for a single character.

Parameters are, in my opinion, about conveying values for configuration not wire efficiency or disk storage efficiency, so not having 32-bit values and/or a byte type is ok.

[wjwwood]

Maybe it should just be left as bytes so people are not confused as to where the byte type is.

[ayrton04]

So I agree completely. I'm happy to support the 64-bit types and let users worry about how to interpret the parameters.

What's the verdict on whether or not bytes[] lives?

[Karsten1987]

nitpick: A . (dot) is missing here.

[Karsten1987]

Should it still be possible to query single elements from array parameters?

let's say in yaml:

robot:
  joints:
    - joint1
    - joint2
    - joint3

Should we have one parameter robot/joints which will return the three values? If so, how would nested arrays look like?

joint_map:
  -
    - joint1
    - joint2
  -
    - my_joint1
    - my_joint2

I am not saying we necessarily support this - you could get around all this by using maps instead of nested arrays - but it's maybe worth mentioning it.

[ayrton04]

I don't actually feel strongly one way or another about this. Anyone else?

[tfoote]

@Karsten1987 are you asking whether you could get a subtree as a "parameter"? If so no. You wouldn't get 3 values.
If you render your first yaml into parameters it would be:

robot/joints/joint1: value1
robot/joints/joint2: value2
robot/joints/joint3: value3

If you ask for parameter robot/joints you'll get a response no such parameter. Though you could ask for a listing of parameters in the namespace robot/joints to get the list of the three above.

Edit: This was assuming that you were leaving off the values in the display, not that joint1 joint2 and joint3 were the values since I was expecting. More like:

robot:
  joints:
    - joint1: value1
    - joint2: value2
    - joint3: value3

Something weird did happen. I think I commented on several posts inline not in a review. And clearly @Karsten1987 saw some of them. But now looking at it I see all my comments are in a pending review. Not sure how that happened. I'm posting them all now.

[wjwwood]

Actually @Karsten1987 if you wanted to do that I would recommend using new keys:

joint_map:
  keys:
    - joint1
    - joint2
  values:
    - my_joint1
    - my_joint2

Which would become parameters like joint_map/keys -> [joint1, joint2]. Or you could do something like this:

joint_map:
  joint1: my_joint1
  joint2: my_joint2

Which would become parameters like joint_map/joint1 -> my_joint1, etc...

---

I think a more general use for this would be a matrix, e.g. a 3 by 3:

my_matrix: [[1, 2, 3], [4, 5, 6], [7, 8, 9]]

Essentially should there be lists of lists.

I feel like this is a critical feature, but I could be convinced otherwise I think.

[Karsten1987]

I actually think my first yaml example should be rendered as one parameter with a collection of three values. https://symfony.com/doc/current/components/yaml/yaml_format.html#collections

There is no direct key-value pair, but rather a list of three values. So the parameter I'd expect would be robot/joints --> [joint1, joint2, joint3]

@wjwwood I also would recommend using maps/key-value pairs instead, I solely wanted to point out that yaml supports such a syntax and we should mention how we would render it.

[wjwwood]

Also why did you @tfoote? I'm confused.

[Karsten1987]

Hmm, must be something weird with github. I also see your comment twice here in that thread. I thought there was a comment prior to mine.

[wjwwood]

I didn't see @tfoote's comment until just now... weird I explicitly looked for it before...

[ayrton04]

So for ROS 1, I've been interpreting arrays as matrices by just reshaping them. It puts the onus on the package authors to interpret the parameters correctly, but I don't personally see an issue with that. How much complexity are we adding by supporting lists of lists? Would we support arbitrary list depths (e.g., lists of lists of lists of lists)?

[wjwwood]

It looks like ROS 1 only supports one dimensional lists as well as only supporting homogeneous lists (can someone confirm that in this document? with links to docs or code if possible).

That would be a hint that maybe supporting the same set of features is sufficient for ROS 2 as well. I'm interested to see what others think.

If we only support homogenous lists, then supporting indefinite dimensionality to the lists seems like a limited amount of overhead, but I could be wrong about that. It might require some prototyping and brainstorming to find the corner cases that bite.

[wjwwood]

Thanks for starting on this, I think we need to figure out if we want lists of lists or not and then we'll be ready to start an implementation.

[wjwwood]

We only support 64-bit ints and floats for parameters right? So I don't think we need 32-bit arrays of them. The bytes[] type is specifically for binary data. Personally I don't think we need a byte type, because you can just use an int64 for decimal values and a length 1 string for a single character.

Parameters are, in my opinion, about conveying values for configuration not wire efficiency or disk storage efficiency, so not having 32-bit values and/or a byte type is ok.

[wjwwood]

I think he means punctuation.

[wjwwood]

Maybe it should just be left as bytes so people are not confused as to where the byte type is.

[wjwwood]

Actually @Karsten1987 if you wanted to do that I would recommend using new keys:

joint_map:
  keys:
    - joint1
    - joint2
  values:
    - my_joint1
    - my_joint2

Which would become parameters like joint_map/keys -> [joint1, joint2]. Or you could do something like this:

joint_map:
  joint1: my_joint1
  joint2: my_joint2

Which would become parameters like joint_map/joint1 -> my_joint1, etc...

---

I think a more general use for this would be a matrix, e.g. a 3 by 3:

my_matrix: [[1, 2, 3], [4, 5, 6], [7, 8, 9]]

Essentially should there be lists of lists.

I feel like this is a critical feature, but I could be convinced otherwise I think.

[ayrton04]

Any further thoughts on list dimensionality?

[mikaelarguedas]

Sorry github did weird things on that thread so I'll comment on verious part in a single monolithic comment.

If there is a byte[] type should there be a byte type too?

I think that renaming bytes[] to byte[] in the design makes sense as it will be consistent with the types we define for our IDL and that we use to defin parameters values in code:
https://github.com/ros2/rcl_interfaces/blob/7710246fda772035ccdd32455542089c2f0da6c0/rcl_interfaces/msg/ParameterValue.msg#L13.

I dont think there is a need for a byte type parameter, as @wjwwood pointed out:

you can just use an int64 for decimal values and a length 1 string for a single character

---

It looks like ROS 1 only supports one dimensional lists as well as only supporting homogeneous lists (can someone confirm that in this document? with links to docs or code if possible).

It looks like the ROS1 allows one dimensional heterogeneous lists:
e.g from rosparam documentation http://wiki.ros.org/rosparam#rosparam_set

Setting a list with one as a string, integer, and float:
$ rosparam set /foo "['1', 1, 1.0]"

The roscpp parameters tutorial defines that lists in parameters can be of any XmlRpcValue type where I think the lilsts are of type XmlRpcValue::TypeArray
Looking at some code, the array type can take different types for its elements:
https://github.com/ros/ros_comm/blob/8b741ac9232a9fadefb19e7fdee654225619d2f8/utilities/xmlrpcpp/test/TestValues.cpp#L265-L270

I am not convinced that heterogenity is a critical feature for parameters though and would be fine if we don't support it in ROS 2

---

Any further thoughts on list dimensionality?

1. In the case where we allow only homogenous lists:
   One challenge I see with multi dimensional lists is that we would need a type "list" that would not be fully described as it could contain a list of any type. So we would either need to be able to introspect list-like structures or to be able to describe them externally.

2. In the case where multi-dimensional lists are built as a collection of homogeneous lists:
   Should we allow the construction of heterogeneous lists of homogeneous lists?
   e.g. :

my_multidimensional_list: [[1, 2, 3], [1.0, 3.14, 42.0]]

Again I dont think there is much value here.

3. Case of multidimensional heterogeneous lists:
   Same question as 1, but slightly less convenient because we also need to introspect the type of each element of each sublist inside the multidimensional list

I can see 1 being convenient for users.

But as @ayrton04 said

I've been interpreting arrays as matrices by just reshaping them. It puts the onus on the package authors to interpret the parameters correctly, but I don't personally see an issue with that.

So I'm fine targeting only one-dimension lists of homogeous types. But supporting multidimensional lists is doable and I could be convinced it's good enough of a feature to justify the additional complexity.

[mikaelarguedas]

Sorry @ayrton04 as the previous comment didnt give you a clear path forward.

Discussing it internally yesterday we decided to side with only one-dimensional homogeneous lists for now (due to the unnecessary complexity of the introspection needed to handle multidimensional and heterogeneity within a list). We can revisit in the future if this happens to be too much of a burden from at the application level.

On the implementation side we will need to enforce that the lists are checked for homogeneity at parsing / loading time, as heterogeneous lists are valid yaml definitions and the type of the parameter is not explicitly stated in the yaml description, we cannot "guess" and convert the values to the right type.

[ayrton04]

OK, thanks for the feedback! I'll update the documentation to suit, and then get moving on the code.

In the internal discussion, was there a final decision on bytes[] vs byte[]?

[wjwwood]

I think byte[] (read as “byte array”) was the consensus.

As a side point I think bytes might make sense, though I’m not in favor of it, to me bytes[] would imply a two dimensional array, and is therefore misleading.

[ayrton04]

Agreed. OK, will update this document.

[mikaelarguedas]

lgtm, thanks @ayrton04 for iterating on this

[ayrton04]

TY all for the feedback!

[mikaelarguedas]

I'll wait for someone else from @ros2/team to appprove before merging