Allow legacy field names in rosidl_generate_interfaces [rolling]

rosidl_adapter/cmake/rosidl_adapt_interfaces.cmake

@@ -30,11 +30,13 @@
 # @public
 #
 function(rosidl_adapt_interfaces idl_var arguments_file)
-  cmake_parse_arguments(ARG "" "TARGET" ""
-    ${ARGN})
+  set(options ALLOW_LEGACY_FIELD_NAMES)
+  set(oneValueArgs TARGET)
+  set(multiValueArgs "")
+  cmake_parse_arguments(ARG "${options}" "${oneValueArgs}" "${multiValueArgs}" ${ARGN})
   if(ARG_UNPARSED_ARGUMENTS)
     message(FATAL_ERROR "rosidl_adapt_interfaces() called with unused "
-      "arguments: ${ARG_UNPARSED_ARGUMENTS}")
+      "arguments: ${ARG_UNPARSED_ARGUMENTS}. ALLOW_LEGACY? '${ARG_ALLOW_LEGACY_FIELD_NAMES}'")
   endif()
 
   find_package(Python3 REQUIRED COMPONENTS Interpreter)
@@ -46,6 +48,11 @@ function(rosidl_adapt_interfaces idl_var arguments_file)
     --arguments-file "${arguments_file}"
     --output-dir "${CMAKE_CURRENT_BINARY_DIR}/rosidl_adapter/${PROJECT_NAME}"
     --output-file "${idl_output}")
+  if(ARG_ALLOW_LEGACY_FIELD_NAMES)
+    list(APPEND cmd --allow-legacy-field-naming)
+    MESSAGE(WARNING Allowing legacy arguments.)
+  endif()
+
   execute_process(
     COMMAND ${cmd}
     OUTPUT_QUIET

rosidl_adapter/rosidl_adapter/__init__.py

@@ -13,14 +13,15 @@
 # limitations under the License.
 
 from pathlib import Path
-
+from rosidl_adapter.parser import DEFAULT_ALLOW_LEGACY_FIELD_NAMES
 
 def convert_to_idl(package_dir: Path, package_name: str, interface_file: Path,
-                   output_dir: Path) -> Path:
+                   output_dir: Path, *, allow_legacy_field_naming: bool=DEFAULT_ALLOW_LEGACY_FIELD_NAMES) -> Path:
     if interface_file.suffix == '.msg':
         from rosidl_adapter.msg import convert_msg_to_idl
+
         return convert_msg_to_idl(
-            package_dir, package_name, interface_file, output_dir / 'msg')
+            package_dir, package_name, interface_file, output_dir / 'msg', allow_legacy_field_naming=allow_legacy_field_naming)
 
     if interface_file.suffix == '.srv':
         from rosidl_adapter.srv import convert_srv_to_idl

rosidl_adapter/rosidl_adapter/main.py

@@ -21,6 +21,7 @@
 
 
 from rosidl_adapter import convert_to_idl
+from rosidl_adapter.parser import DEFAULT_ALLOW_LEGACY_FIELD_NAMES
 
 
 def main(argv: List[str] = sys.argv[1:]) -> None:
@@ -39,6 +40,10 @@ def main(argv: List[str] = sys.argv[1:]) -> None:
         '--output-file', required=True,
         help='The output file containing the tuples for the generated .idl '
              'files')
+    legacy_field_name_action = "store_true" if not DEFAULT_ALLOW_LEGACY_FIELD_NAMES else "store_false"
+    parser.add_argument(
+        '--allow-legacy-field-naming', required=False, action=legacy_field_name_action,
+        help='Allow legacy ROS1 style field names that use PascalCase, camelCase, and Pascal_With_Underscores')
     args = parser.parse_args(argv)
     output_dir = pathlib.Path(args.output_dir)
     output_file = pathlib.Path(args.output_file)
@@ -53,7 +58,8 @@ def main(argv: List[str] = sys.argv[1:]) -> None:
         basepath, relative_path = non_idl_tuple.rsplit(':', 1)
         abs_idl_file = convert_to_idl(
             pathlib.Path(basepath), args.package_name,
-            pathlib.Path(relative_path), output_dir)
+            pathlib.Path(relative_path), output_dir,
+            allow_legacy_field_naming=args.allow_legacy_field_naming)
         idl_tuples.append((output_dir, abs_idl_file.relative_to(output_dir)))
 
     output_file.parent.mkdir(exist_ok=True)

rosidl_adapter/rosidl_adapter/msg/__init__.py

@@ -15,12 +15,12 @@
 from pathlib import Path
 from typing import Final, Optional, Union
 
-from rosidl_adapter.parser import BaseType, parse_message_string, Type
+from rosidl_adapter.parser import BaseType, parse_message_string, Type, DEFAULT_ALLOW_LEGACY_FIELD_NAMES
 from rosidl_adapter.resource import expand_template, MsgData
 
 
 def convert_msg_to_idl(package_dir: Path, package_name: str, input_file: Path,
-                       output_dir: Path) -> Path:
+                       output_dir: Path, *, allow_legacy_field_naming: bool=DEFAULT_ALLOW_LEGACY_FIELD_NAMES) -> Path:
     assert package_dir.is_absolute()
     assert not input_file.is_absolute()
     assert input_file.suffix == '.msg'
@@ -29,7 +29,7 @@ def convert_msg_to_idl(package_dir: Path, package_name: str, input_file: Path,
     print(f'Reading input file: {abs_input_file}')
     abs_input_file = package_dir / input_file
     content = abs_input_file.read_text(encoding='utf-8')
-    msg = parse_message_string(package_name, input_file.stem, content)
+    msg = parse_message_string(package_name, input_file.stem, content, allow_legacy_field_naming=allow_legacy_field_naming)
 
     output_file = output_dir / input_file.with_suffix('.idl').name
     abs_output_file = output_file.absolute()

rosidl_adapter/rosidl_adapter/parser.py

@@ -38,6 +38,8 @@
 ACTION_RESULT_SERVICE_SUFFIX: Final = '_Result'
 ACTION_FEEDBACK_MESSAGE_SUFFIX: Final = '_Feedback'
 
+DEFAULT_ALLOW_LEGACY_FIELD_NAMES: bool = True
+
 PRIMITIVE_TYPES: Final = [
     'bool',
     'byte',
@@ -69,7 +71,7 @@
     '$')
 VALID_FIELD_NAME_PATTERN: Final = VALID_PACKAGE_NAME_PATTERN
 # relaxed patterns used for compatibility with ROS 1 messages
-# VALID_FIELD_NAME_PATTERN = re.compile('^[A-Za-z][A-Za-z0-9_]*$')
+RELAXED_FIELD_NAME_PATTERN: Final = re.compile('^[A-Za-z][A-Za-z0-9_]*$')
 VALID_MESSAGE_NAME_PATTERN: Final = re.compile('^[A-Z][A-Za-z0-9]*$')
 # relaxed patterns used for compatibility with ROS 1 messages
 # VALID_MESSAGE_NAME_PATTERN = re.compile('^[A-Za-z][A-Za-z0-9]*$')
@@ -84,6 +86,9 @@ class Annotations(TypedDict, total=False):
         comment: List[str]
         unit: str
 
+# By default, ROS 2 does not allow legacy field names.
+# https://docs.ros.org/en/rolling/Concepts/Basic/About-Interfaces.html#field-names
+DEFAULT_ALLOW_LEGACY_FIELD_NAMES=False
 
 class InvalidSpecification(Exception):
     pass
@@ -127,8 +132,16 @@ def is_valid_package_name(name: str) -> bool:
         raise InvalidResourceName(name)
     return m is not None and m.group(0) == name
 
+def is_valid_legacy_field_name(name):
+    try:
+        m = RELAXED_FIELD_NAME_PATTERN.match(name)
+    except TypeError:
+        raise InvalidResourceName(name)
+    return m is not None and m.group(0) == name
 
-def is_valid_field_name(name: str) -> bool:
+def is_valid_field_name(name: str, *, allow_legacy_field_naming: bool=DEFAULT_ALLOW_LEGACY_FIELD_NAMES) -> bool:
+    if allow_legacy_field_naming:
+        return is_valid_legacy_field_name(name)
     try:
         m = VALID_FIELD_NAME_PATTERN.match(name)
     except TypeError:
@@ -362,12 +375,12 @@ def __str__(self) -> str:
 class Field:
 
     def __init__(self, type_: 'Type', name: str,
-                 default_value_string: Optional[str] = None) -> None:
+                 default_value_string: Optional[str] = None, *, allow_legacy_field_naming: bool=DEFAULT_ALLOW_LEGACY_FIELD_NAMES) -> None:
         if not isinstance(type_, Type):
             raise TypeError(
                 "the field type '%s' must be a 'Type' instance" % type_)
         self.type = type_
-        if not is_valid_field_name(name):
+        if not is_valid_field_name(name, allow_legacy_field_naming=allow_legacy_field_naming):
             raise NameError(
                 "'{}' is an invalid field name. It should have the pattern '{}'".format(
                     name, VALID_FIELD_NAME_PATTERN.pattern))
@@ -481,7 +494,7 @@ def extract_file_level_comments(message_string: str) -> Tuple[List[str], List[st
 
 
 def parse_message_string(pkg_name: str, msg_name: str,
-                         message_string: str) -> MessageSpecification:
+                         message_string: str, *, allow_legacy_field_naming: bool=DEFAULT_ALLOW_LEGACY_FIELD_NAMES) -> MessageSpecification:
     fields: List[Field] = []
     constants: List[Constant] = []
     last_element: Union[Field, Constant, None] = None  # either a field or a constant
@@ -537,7 +550,8 @@ def parse_message_string(pkg_name: str, msg_name: str,
             try:
                 fields.append(Field(
                     Type(type_string, context_package_name=pkg_name),
-                    field_name, optional_default_value_string))
+                    field_name, optional_default_value_string,
+                    allow_legacy_field_naming=allow_legacy_field_naming))
             except Exception as err:
                 print(
                     "Error processing '{line}' of '{pkg}/{msg}': '{err}'".format(

rosidl_cmake/cmake/rosidl_generate_interfaces.cmake

@@ -52,7 +52,7 @@
 #
 macro(rosidl_generate_interfaces target)
   cmake_parse_arguments(_ARG
-    "ADD_LINTER_TESTS;SKIP_INSTALL;SKIP_GROUP_MEMBERSHIP_CHECK"
+    "ADD_LINTER_TESTS;SKIP_INSTALL;SKIP_GROUP_MEMBERSHIP_CHECK;ALLOW_LEGACY_FIELD_NAMES"
     "LIBRARY_NAME" "DEPENDENCIES"
     ${ARGN})
   if(NOT _ARG_UNPARSED_ARGUMENTS)
@@ -129,9 +129,18 @@ macro(rosidl_generate_interfaces target)
         PACKAGE_NAME "${PROJECT_NAME}"
         NON_IDL_TUPLES "${_non_idl_tuples}"
       )
+      set(_rosidl_apt_interfaces_opts)
+      if(_ARG_ALLOW_LEGACY_FIELD_NAMES)
+        set(_rosidl_apt_interfaces_opts ALLOW_LEGACY_FIELD_NAMES)
+        message(WARNING "Allowing legacy field names")
+      endif()
+
+      #${_rosidl_apt_interfaces_opts}
+
       rosidl_adapt_interfaces(
         _idl_adapter_tuples
         "${_adapter_arguments_file}"
+        ${_rosidl_apt_interfaces_opts}
         TARGET ${target}
       )
     endif()