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

Fails silently for missing fields #28

Open
b-adkins opened this issue Aug 7, 2024 · 4 comments
Open

Fails silently for missing fields #28

b-adkins opened this issue Aug 7, 2024 · 4 comments

Comments

@b-adkins
Copy link

b-adkins commented Aug 7, 2024

I tried dynmsg to parse config files for controlling a real robot. There does not appear to be an option for it to raise an exception if a message field is missing.

I found this issue to be a dealbreaker, due to safety concerns, and so I will not be using this library.

Testing

On ROS 2 Humble, here is my parsing code.

geometry_msgs::msg::Pose pose;
yaml_string = 
"position: {'x': 0.979, 'y': -2.161}"
"orientation: {'x': 0., 'y': 0., 'z': 0., 'w': 1.}";
dynmsg::cpp::yaml_and_typeinfo_to_rosmsg(
    dynmsg::cpp::get_type_info({"geometry_msgs", "Pose"}),
    yaml_string,
    reinterpret_cast<void*>(&pose)
  );

Expected Behavior

The YAML would fail to be parsed into a geometry_msgs/msg/Pose, because it is missing a field. It would have to be via an exception, because there is no return code in the function interfaces nor in the return struct RosMessage_Cpp.

Actual behavior

It gets parsed into:

position:
  x: 0.979000
  y: -2.16100
  z: 0.00000
orientation:
  x: 0.00000
  y: 0.00000
  z: 0.00000
  w: 1.00000

using the default value of zero for the missing position z.

I find this to be incredibly dangerous when controlling a real robot. A typo in the config file that omits a key could fail silently during parsing and then send a robot to an unexpected wrong position.

@christophebedard
Copy link
Member

Feel free to submit a PR, but this repo isn't really being maintained, so it might never get merged.

find this to be incredibly dangerous when controlling a real robot.

I don't think this was ever meant to be used in production, at least not in its current state.

@sgarciav
Copy link

@b-adkins I agree with you that is is dangerous behavior. Did you ever submit this PR? If not, I can help work on this since I've run into the same issue.

@b-adkins
Copy link
Author

I decided not to use the library. It would be great if you could add this feature!

@sgarciav
Copy link

Sounds good! I'll see what I can do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants