-
Notifications
You must be signed in to change notification settings - Fork 483
Description
I had been following the discussions and eventual implementation of the design for enabling statically typed parameters. While I am glad to see this enhancement, one area I see lacking sufficient discussion in the design conversation and the current documentation is the behavior of undeclaring parameters. Let me first start by illustrating a simple user scenario:
There is a node which dynamically loads and unloads plugins, each of which declares its own parameters on the host node. These parameters would appropriately be either read-only or statically typed in order to protect from any undesired modifications. Those parameters will be accessible by other nodes in the system, and when the plugins are dynamically unloaded, we expect that clients of this node should have awareness that the plugins and hence their parameters are gone (e.g. it is not fetching values from a no longer active plugin).
Currently, a parameter which is either read-only or is statically typed (the default) cannot be undeclared. My initial impression was that this behavior was uninituitive, because to undeclare a parameter seems a different use case than attempting to modify it. Even further, setting a parameter to rclcpp::PARAMETER_NOT_SET seems like an additional use case, especially now that uninitialized parameters are allowed again. The main counterpoint I can see is that a parameter could be undeclared and subsequently declared again with a different type or different value (in case of read-only).
My current proposal is to allow only the host node to undeclare parameters. In #807, the idea was discussed to allow only the owner node to make modifications to a read-only parameter. Without going so far, I think it would be fair to at least give the host node the authority to undeclare any parameter (whether statically typed or read-only), while blocking remote nodes or command line from undeclaring these parameters (those that are read only or statically typed) via a set parameter request. I see an argument for removing the logic to allow the set_parameter feature to undeclare a parameter as well. Ideally I see a situation where, in case of the above example, a node plugin can declare its parameters on the host node when it is loaded, and undeclare its parameters when it is dynamically unloaded, while still offering its protection against statically typed modifications or read-only changes. The host node is already the arbiter for when a parameter is declared, so it makes sense to me that it should have the same capacity to undeclare them. This may be a particularly prevalent design pattern in lifecycle nodes, where system cycles may be regularly performed, and it may make more sense to declare/undeclare parameters than to continuosly check if a parameter has already been declared in order to avoid an error.
I appreciate any thoughts or other perspectives on this issue as well. I simpy have seen practical use cases where it would be preferred behavior to undeclare parameters that may still be read only or statically typed.