Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Communicate double / double array parameters with type info, explicitly cast when set from integer #256

Merged
merged 2 commits into from Aug 21, 2023

Conversation

achim-k
Copy link
Collaborator

@achim-k achim-k commented Aug 8, 2023

Public-Facing Changes

Communicate double / double array parameters with type info, explicitly cast when set from integer

Description

Implements foxglove/ws-protocol#519

  • When sending parameter values to the client, the server will include type information in case a parameter is of type PARAMETER_DOUBLE ("type": "float64") or an PARAMETER_ARRAY of PARAMETER_DOUBLE ("type": "float64_array")
  • When receiving a setParameters request from the client, the server checks if the parameter type field is present and attempts to cast the parameter value (or values in case it's an array) to a double in case the type is float64 / float64_array. This is mainly relevant for ROS2 as setting a double parameter to an integer will raise an exception. In ROS1, one can change the parameter type on the fly

@defunctzombie
Copy link
Contributor

In reviewing this PR I realized I should have read the spec changes more clearly. I have some questions here: foxglove/ws-protocol#519 (comment)

In what direction do we need to communicate this type information? Is this from server -> client (studio)?

@achim-k achim-k requested a review from jtbandes August 14, 2023 21:11
achim-k added a commit to foxglove/studio that referenced this pull request Aug 16, 2023
**User-Facing Changes**
Not worth being mentioned in release notes

**Description**
When setting a float parameter to e.g. `1.0`, Javascript automatically
makes it to `1` and the server would deduce this as a integer parameter.
Remembering the parameter type that is communicated by the server, and
sending it along with a `setParameter` request fixes this.

See also foxglove/ws-protocol#519 and
foxglove/ros-foxglove-bridge#256

Fixes #6559
@achim-k achim-k force-pushed the achim/support_float_parameter_types branch from 362ca07 to 9c3b50b Compare August 21, 2023 17:52
@achim-k achim-k merged commit c12ec1b into main Aug 21, 2023
11 checks passed
@achim-k achim-k deleted the achim/support_float_parameter_types branch August 21, 2023 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants