dynamic_message_introspection icon indicating copy to clipboard operation
dynamic_message_introspection copied to clipboard

Fails silently for missing fields

Open b-adkins opened this issue 1 year ago • 4 comments

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.

b-adkins avatar Aug 07 '24 23:08 b-adkins

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.

christophebedard avatar Aug 19 '24 23:08 christophebedard

@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.

sgarciav avatar Dec 13 '24 20:12 sgarciav

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

b-adkins avatar Dec 13 '24 21:12 b-adkins

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

sgarciav avatar Dec 14 '24 00:12 sgarciav