Write parameters design document about design

[vooon]

Not sure where better to ask.

Does new param system allow me to expose parameters from connected device?
e.g. following schematic:

Where node cache stores additional device-specific information (SET operation require that).
Or i can not partially rebuild node param server?

[tfoote]

@vooon I'm not sure I understand your question enough to answer it. Could you explain what you're calling out as a device, ros, and node cache?

[vooon]

• Device some external thing connected to computer (with it's own parameters).
  In my case it is flight computer.
• ROS bunch of nodes that uses unified protocol.
• Node cache - place where node that handles communication with device store parameter values

I want to see following sequence:

1. ROS node request to set some parameter
2. driver node reencode and send that request to device
3. device responds with applied value
4. driver node update cache & fire event for other nodes

[tfoote]

@vooon Supporting setting the parameter on an external device as you mention would work fine. In your drawing though both the setting and getting from the main ROS system would go to the node cache, and it would talk to the device. I'd suggest considering it more of a proxy for the device, in the realm of a device driver rather than a peer.

[vooon]

But if set first changes cache it may inconsistent with actual device value. Device e.g. may reject new value. So for time between driver request and reply cache will store wrong data.
But really that not big deal, now that works similar with roscore param storage.

But one thing not clear to me: would it be possible to reimplement parameter storage & request handlers? Last time i saw rclcpp sources that part is private, so i can't.
Thing that i don't like in my current implementation is double cache: one for additional metadata (and value in device format) and other for value (in roscore, but same may happen with ros2 if i can't extend cache storage type).

[jack-oquin]

I have device response issues with camera1394 similar to what @vooon describes. Various ones of the thousands of IIDC-compliant cameras support particular modes and option settings. There is no reasonable way for the driver to tell which ones without querying the device and waiting for a response. That works with dynamic_reconfigure, but it takes time.

I can't tell whether the discussion about immediately responding to parameter updates is consistent with lengthy validations of that nature.

[tfoote]

We are working on designing the spec for what it should be, the current implementation in rclcpp is a prototype designed to validate our spec. Anything in the current implementation which does not meet the spec is up for ticketing and updating. The interface spec should be independant of the implementation. It should also work for our other future client libraries such as rclc, rclpy etc.

You should be able to implement your own node which provides the parameter API and uses the hardware on the backend for storage. Validation and acceptance of changes, and queries should be able to be delegated to the hardware if you don't want to do any caching.

I can't tell whether the discussion about immediately responding to parameter updates is consistent with lengthy validations of that nature.

I think I would consider waiting for validation and returning "immediately" after is fine. I haven't envisioned any commitment to a specific timing for responding, as depending on your network layer etc there may be large delays anyway for any remote operation.

If there is something that becomes operationally problematic I suspect that you could do things like reading all the parameters at startup and caching them in the node. But that would be a case by case basis and specific to any device driver.

[wjwwood]

Seems to me that you could accomplish what you've described with our proposed design. You can imagine some code like this:

  node->on_parameter_change("my_device_parameter", [&device_obj] (ParameterVariant parameter) {
    if (!device_obj.set_my_parameter(parameter.as_string())) {
      return false;  // Failed to set it on device, do not allow "cache" to change.
    }
    return true;  // Parameter accepted, store in "cache"
  });

Any time up until the above function returns true, if someone asks the node for that parameter it will be the old value. If you were really concerned about race conditions, you could override all get functions as well and synchronize them with this function using a lock.

I don't know if it is called out specifically in the design doc at the moment, but it should be possible to define user functions for the getting of parameters so that you can ensure the parameter being returned is in sync with what is on the device or doesn't happen while it is actively being set. Though this might not be the best idea for performance depending on how often you get the parameter value and how long it takes to check that the parameter is in sync with the device.

[jack-oquin]

I am not much concerned with the latency or bandwidth of parameter setting, nor do I have a problem with other nodes getting the previous value while an update is being validated.

Mainly, I want the driver to be able to interrupt its data streaming (where necessary) and check with the device (when necessary) before accepting or rejecting the update request. Given the separate discussion of node states, that may sometimes require that driver to temporarily become inactive (or whatever).

In practice, some parameters can be updated while the driver is actively streaming, while others require the device to be closed and re-opened.

[wjwwood]

I think that's different from what @vooon was asking (if I understand it correctly).

What you're talking about is definitely possible. Your driver can simply reject changes to parameters which require the device to be stopped before changing. Then it is up to the caller to put the node into an inactive state, change the parameters, and then put it back into active. And since the rejection of setting the parameter can carry a string message, it should be obvious what the issue is from the configuration GUI so long as the driver gives a good error message.

[jack-oquin]

Does that mean the node cannot put itself into an inactive state while updating parameters? I can see how people with strict, provable, real-time response requirements might want system-level control over whether those kinds of configuration requests are allowed.

But, restarting is essentially what people currently do with dynamic_reconfigure. Each parameter is marked whether or not it can be updated while streaming. I would not expect an external requester node to understand those details, especially not a user-interface requester.

I suppose that kind of information could be stored in the parameter server, but that seems somewhat heavy-handed and inflexible compared to the simple dynamic_reconfigure paradigm.

[wjwwood]

There's no reason a node could not request to put itself into an inactive state, but it remains to be seen if that's a good design pattern.

But, restarting is essentially what people currently do with dynamic_reconfigure. Each parameter is marked whether or not it can be updated while streaming. I would not expect an external requester node to understand those details, especially not a user-interface requester.

I've found uses of dynamic_reconfigure to be even stranger, where you can only change parameter is no one is subscribed (which is essentially when not streaming). At least with our proposed system the UI could show the user the reason string for the rejected change, and that string could tell them what they need to do in order to change the parameter. And I think that if you've written a custom node to change the parameters of a device driver, in many cases it will be ok to have it know whether or not the node's state needs to be changed before certain parameters can be changed.

I suppose that kind of information could be stored in the parameter server, but that seems somewhat heavy-handed and inflexible compared to the simple dynamic_reconfigure paradigm.

I don't see that the dynamic_reconfigure offers a solution where our proposed design does not. Can you explain what you mean by that?

[jack-oquin]

I suppose that kind of information could be stored in the parameter server, but that seems somewhat heavy-handed and inflexible compared to the simple dynamic_reconfigure paradigm.

I don't see that the dynamic_reconfigure offers a solution where our proposed design does not. Can you explain what you mean by that?

I just wanted to say that the dynamic_reconfigure interface is simpler, not that the new proposal is inflexible. There are advantages and disadvantages to defining the parameter level in the parameter server instead of in each node.

The extra flexibility was due to the fact that each node gets to define its own definition of parameter "levels" unknown outside the node. That is expressed as a bit mask with all the modified parameters of a single request or-ed together.

In all the instances I can think of, there are only two or three different level bits defined, and the only important distinction is between parameters that can be changed while streaming and those which cannot. See: drtiver_base/SensorLevels.msg.

So, I am not claiming that the extra flexibility is important.

[jack-oquin]

And I think that if you've written a custom node to change the parameters of a device driver, in many cases it will be ok to have it know whether or not the node's state needs to be changed before certain parameters can be changed.

That approach would be asking for trouble. Parameters frequently get added to address some obscure problem with the device. Remembering to update all relevant "custom configuration nodes" at that time seems unlikely, especially when different projects have their own configuration nodes for a shared driver.

[vooon]

@wjwwood That's ok, but inefficient when i have 400+ parameters in device and want handle on_parameter_change for all of it.
Also seems that now callback (lambda in example) do not have parameter name in arguments -> more problems (no way to pass same handler function).

[tfoote]

Closing this as the draft is posted. For feedback/suggestions for the implementation please ticket rclcpp.