Implement functions to get parameters from global arguments #176
Conversation
nnmm
force-pushed
the
resolve_parameter_overrides
branch
from
3f2dd4e to
dcccd03
Compare
| .into_iter() | ||
| .map(u8::from) | ||
| .sum(); | ||
| assert_eq!(num_active, 1); |
There was a problem hiding this comment.
This assertion will fail on empty list parameters, but neither of the big 2 client libraries can handle empty lists either.
There was a problem hiding this comment.
No problem with failing on empty lists. We should get clarification from rclcpp and rclpy what the expected behavior should be.
There was a problem hiding this comment.
Yeah, I will try to find out more about this edge case. What I know is that launch_ros forbids empty lists because they're not supported (yet), so I think this is expected and we're definitely fine for now.
| String(String), | ||
| /// An array of u8. | ||
| /// | ||
| /// YAML example: Not possible to specify as YAML. |
There was a problem hiding this comment.
Correct me if I'm wrong with this comment.
There was a problem hiding this comment.
Do you mean bytes (i.e. binary data) or unsigned integers? The former needs to be encoded as base64, whereas the latter is not possible to enforce signed integers in YAML.
There was a problem hiding this comment.
Bytes. Can you specify how exactly to specify them? E.g. the following is base64-encoded, but parses to a string, not a byte array:
cargo run --bin minimal_subscriber -- --ros-args --param foo:="eW9sbw=="
There was a problem hiding this comment.
I meant that they can be encoded, but there's no way syntactically to differentiate bytes from strings.
| .into_iter() | ||
| .map(u8::from) | ||
| .sum(); | ||
| assert_eq!(num_active, 1); |
There was a problem hiding this comment.
No problem with failing on empty lists. We should get clarification from rclcpp and rclpy what the expected behavior should be.
nnmm
force-pushed
the
resolve_parameter_overrides
branch
from
dcccd03 to
fcde06d
Compare
|
@esteve This one could use another review too ;) |
nnmm
force-pushed
the
resolve_parameter_overrides
branch
from
fcde06d to
00e86f9
Compare
nnmm
force-pushed
the
resolve_parameter_overrides
branch
from
00e86f9 to
cc0ed1f
Compare
|
I added a test to ensure that the precedence of |
The parameters are not accessible by users yet – I'll do that in a separate PR, since I want to keep them easy to review.