search

ros2 / rosidl

Packages which provide the ROS IDL (.msg) definition and code generation.
Apache License 2.0
75 stars 125 forks source link

Remove unused splitting of .srv files in CMake #753

Closed sloretz closed 1 year ago

sloretz commented 1 year ago

It used to be that rosidl generators acted on ROS IDL (.msg/.srv/.action) files directly, and generated code from them. This removes now purposeless code that used to be part of that pipeline.

IIUC, how this used to work is the rosidl_generate_interfaces() macro would split .srv files into two: one for the request and one for the response. The generators would then generate code for the split .msg files. Notice in the past the request and response messages were put into a variable called _idl_files, meaning ROS IDL, and which was then passed to the generators.

This code seems to have survived the transition to OMG IDL, but it no longer has a purpose. Notice the split services are now put into a variable called _non_idl_files, (now meaning they're not OMG IDL files). These files aren't passed to the generators. They do get installed - but AFAIK that's just so other's can see what the original ROS IDL of a message was. Nothing actually uses these ..._Request.msg and ..._Response.msg files anymore.

With the OMG IDL pipeline, .srv files are converted to OMG IDL files. The service still gets split into two in a way. rosidl_adapter parses the .srv file to produce a python object representing the request and the response, and then uses one of the .msg templates for the request and response in the OMG IDL file, but the generators downstream only see that as an OMG IDL file containing Request and Response structs.

sloretz commented 1 year ago

CI (build: all packages test: --packages-above rosidl_cmake)

emersonknapp commented 1 year ago

lol omg i was so confused for 30 mins there just now. I literally just had a test start failing on me because I was using these. I was like "how is something possibly passing on Humble but not in Rolling??". I hadn't pulled locally, and Rpr was fine - but the CI started failing. https://github.com/ros2/rosbag2/pull/1341

Funny that you did this on the exact week I decided to start depending on it.

I see the motivation to clean up a dangling thing... now I need to reconsider how I'm going to handle getting "implicit subinterface" schemas more generally - I was just going to punt on actions and handle services since the .msg was already there.