ros2 / ros2cli

ROS 2 command line interface tools
Apache License 2.0
173 stars 160 forks source link

`ros2 interface show` return a wrong result #905

Closed GoesM closed 4 months ago

GoesM commented 4 months ago

Bug report

Required Info:

Steps to reproduce issue

do the command like:

ros2 interface show geometry_msgs/msg/PoseWithCovarianceStamped 

Actual behavior

I'm within ros2-humble version , and it return a wrong result

# This expresses an estimated pose with a reference coordinate frame and timestamp

std_msgs/Header header
    builtin_interfaces/Time stamp
        int32 sec
        uint32 nanosec
    string frame_id
PoseWithCovariance pose
    Pose pose
        Point position
            float64 x
            float64 y
            float64 z
        Quaternion orientation
            float64 x 0
            float64 y 0
            float64 z 0
            float64 w 1
    float64[36] covariance

Such situation occurs almost for all types of topics.

Expected behavior

std_msgs/Header header should be wrong.

and the correct result should be std_msgs/msg/Header

Additional information

If it returns a wrong result, it would be more complex for users to fix wrong message-type into a right one.

And many tools for ros2 are developed according to this API like ros2_automatic_fuzz, it would do like :

ros2 interface show std_msgs/Header

and then, failed like:

Invalid name 'std_msgs/Header'. Expected three parts separated by '/'

because the command with the correct type should be like:

ros2 interface show std_msgs/msg/Header 
# Standard metadata for higher-level stamped data types.
# This is generally used to communicate timestamped data
# in a particular coordinate frame.

# Two-integer timestamp that is expressed as seconds and nanoseconds.
builtin_interfaces/Time stamp
    int32 sec
    uint32 nanosec

# Transform frame with which this data is associated.
string frame_id
fujitatomoya commented 4 months ago

basically ros2interface just prints the file contents as it described. that said, it does not append, prepend or modify the file contents to print. (except comments control) so it just prints this line, in the case of ros2 interface show geometry_msgs/msg/PoseWithCovarianceStamped,

https://github.com/ros2/common_interfaces/blob/0e0450bc8ccd90c885b3a4aba703b7546790c871/geometry_msgs/msg/PolygonInstanceStamped.msg#L4

if we want to see it like std_msgs/msg/Header as you suggested, we need to update geometry_msgs/msg/PolygonInstanceStamped.msg file directly to fix it.

2nd, ros2 interface show std_msgs/Header should fail (ValueError) since we need to get the interface file based on the interface argument. if it does not have msg, srv between package name and message file name, it cannot get the interface file because there would be the same file name for msg and srv under the same package.

https://github.com/ros2/rosidl_runtime_py/blob/3327b1dc488af514017c5446ffff12d102bbe94a/rosidl_runtime_py/get_interfaces.py#L156

std_msgs/Header header should be wrong.

this should not be wrong, and so many other packages have this kind of description without msg in between. this is already in the file (either action, srv or msg) description, so it must be msg file nested here. that is why, rosidl_adapter can return the message specification with package name and message, and replace / with /msg/ to parse it.

https://github.com/ros2/rosidl/blob/4ba0effa201030ae8f45597b29d4ca685b2d50a1/rosidl_adapter/rosidl_adapter/parser.py#L466

and

https://github.com/ros2/ros2cli/blob/a18bc6816ece36dad51a8ea5a2d7a216c473d477/ros2interface/ros2interface/verb/show.py#L71-L77

GoesM commented 4 months ago

Thank you for your patient explanation; it's very clear to me. ^_^

However, I have a further question: How can I use the command line to make the rosidl_adapter work so that I can get the expected feedback even without /msg/?

fujitatomoya commented 4 months ago

How can I use the command line to make the rosidl_adapter work so that I can get the expected feedback even without /msg/?

I am not sure about this question, can you rephrase it? rosidl_adapter is called to parse the file lines, not to get the interface file.

GoesM commented 4 months ago

uh I mean , Which API could let rosidl_adapter works ? or How can I use ros2 interface show with this function ?

fujitatomoya commented 4 months ago

Which API could let rosidl_adapter works ? or How can I use ros2 interface show with this function ?

ros2 interface show already uses this rosidl_adapter once it gets to read the file. (https://github.com/ros2/ros2cli/issues/905#issuecomment-2121476242) But for getting the idl file from ros2 interface show's type argument, it requires either msg, srv or action in between package name and type file name.

GoesM commented 4 months ago

I see. Greatly thank you for your patience.