Allow unsetting (deleting) parameters #145
Conversation
| if (j.find("value") == j.end()) { | ||
| p = Parameter(name); // Value is not set (undefined). | ||
| return; | ||
| } |
There was a problem hiding this comment.
I think it would be more explicit if the value was set to null (nlohmann::detail::value_t::null)
There was a problem hiding this comment.
I don't understand the comment here, null and undefined are not the same thing.
There was a problem hiding this comment.
I was referring to foxglove/ws-protocol#354 (comment), where we chose to use undefined (no value provided) instead of null.
achim-k
force-pushed
the
achim/allow_deleting_params
branch
from
c55e737
to
19b451d
Compare
| @@ -974,7 +974,14 @@ template <typename ServerConfiguration> | |||
| inline void Server<ServerConfiguration>::publishParameterValues( | |||
| ConnHandle hdl, const std::vector<Parameter>& parameters, | |||
| const std::optional<std::string>& requestId) { | |||
| nlohmann::json jsonPayload{{"op", "parameterValues"}, {"parameters", parameters}}; | |||
| // Filter out parameters which are not set. | |||
There was a problem hiding this comment.
The spec currently does not declare what parameter message to expect when requesting a non-existing / deleted parameter. Should it be
{
"op": "parameterValues",
"parameters": []
}or
{
"op": "parameterValues",
"parameters": [{ "name": "/deleted_param" }]
}There was a problem hiding this comment.
The second example is not valid JSON. There is no undefined literal in https://www.rfc-editor.org/rfc/rfc8259
There was a problem hiding this comment.
Adapted my first comment by removing the value field
There was a problem hiding this comment.
I think the first is better?
There was a problem hiding this comment.
OK, that's also how it is currently implemented
|
@jhurliman OK for you to merge as is? |
Public-Facing Changes
Description
Implements the specification update in foxglove/ws-protocol#354