ros2 / rosidl

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

Allow legacy field names in rosidl_generate_interfaces #834

Open Ryanf55 opened 6 days ago

Ryanf55 commented 6 days ago

Purpose

Propose an implementation of relaxing ROSIDL constraints on field names per this ROS Discourse proposal from a while back.

https://discourse.ros.org/t/relaxing-ros2-topic-service-field-name-restrictions/6371/8

Use Case

Happy Halloween! Do you deal with scary :ghost: large ROS1 code bases that use cameCase for message field names? I do!

I have a rather large codebase that I need to migrate from ROS 1 to ROS 2. It is very much "legacy" code, so it's largely untested and unmaintained. All of the message fields names use camelCase, and sed operations don't work because variables names are reused to non-ROS messages. I don't want to cause regressions porting to ROS 2, so minimizing the code change is crucial. I am very risk averse and also in a resource bind because ROS 1 is EOL soon. Having to deal with variable naming changes is the last thing I want to bother with when porting to ROS 2. It's at least 2000 LOC of potential copy-paste typos, with many similarly named fields, such as lat and lat that are easily mixed up.

I am proposing this on humble because that's what I tested this on. If the maintainers like the implementation, I can forward port to rolling (clean up the code, finish the implementation, get CI happy, etc).

I think yellow CMake warnings about legacy field names at compile time are a good thing - open to removing those.

Demo

Follow this guide, with some changes: https://docs.ros.org/en/humble/Tutorials/Beginner-Client-Libraries/Single-Package-Define-And-Use-Interface.html#create-a-package

CMakeLists

change to this, noticing the new ALLOW_LEGACY_FIELD_NAMES option.

 rosidl_generate_interfaces(${PROJECT_NAME} ALLOW_LEGACY_FIELD_NAMES 
  ${msg_files}
)

Add these so we can test the new versions package.xml


  <depend>rosidl_cmake</depend>
  <depend>rosidl_adapter</depend>

AddressBook.msg

Change phone_type to phoneType

uint8 phoneType

Finally, build and test:

colcon build --packages-up-to more_interfaces --allow-overriding rosidl_adapter rosidl_cmake --event-handlers=console_cohesion+