From a348329cc6503bd7e87cc396f338e5a0b4ac6b51 Mon Sep 17 00:00:00 2001 From: Ryan Friedman <25047695+Ryanf55@users.noreply.github.com> Date: Fri, 1 Nov 2024 20:56:34 -0600 Subject: [PATCH] Allow legacy field names in rosidl_generate_interfaces Signed-off-by: Ryan Friedman <25047695+Ryanf55@users.noreply.github.com> --- .../cmake/rosidl_adapt_interfaces.cmake | 13 +++++++--- rosidl_adapter/rosidl_adapter/__init__.py | 7 ++--- rosidl_adapter/rosidl_adapter/main.py | 8 +++++- rosidl_adapter/rosidl_adapter/msg/__init__.py | 6 ++--- rosidl_adapter/rosidl_adapter/parser.py | 26 ++++++++++++++----- .../cmake/rosidl_generate_interfaces.cmake | 11 +++++++- 6 files changed, 54 insertions(+), 17 deletions(-) diff --git a/rosidl_adapter/cmake/rosidl_adapt_interfaces.cmake b/rosidl_adapter/cmake/rosidl_adapt_interfaces.cmake index 6c91facbb..90644586e 100644 --- a/rosidl_adapter/cmake/rosidl_adapt_interfaces.cmake +++ b/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 diff --git a/rosidl_adapter/rosidl_adapter/__init__.py b/rosidl_adapter/rosidl_adapter/__init__.py index 7351d577f..deb4260a9 100644 --- a/rosidl_adapter/rosidl_adapter/__init__.py +++ b/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 diff --git a/rosidl_adapter/rosidl_adapter/main.py b/rosidl_adapter/rosidl_adapter/main.py index 759159c2a..c5d75ef20 100644 --- a/rosidl_adapter/rosidl_adapter/main.py +++ b/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) diff --git a/rosidl_adapter/rosidl_adapter/msg/__init__.py b/rosidl_adapter/rosidl_adapter/msg/__init__.py index 1798ba4ae..83f079350 100644 --- a/rosidl_adapter/rosidl_adapter/msg/__init__.py +++ b/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() diff --git a/rosidl_adapter/rosidl_adapter/parser.py b/rosidl_adapter/rosidl_adapter/parser.py index aeee4b933..ef9f1bb7b 100644 --- a/rosidl_adapter/rosidl_adapter/parser.py +++ b/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( diff --git a/rosidl_cmake/cmake/rosidl_generate_interfaces.cmake b/rosidl_cmake/cmake/rosidl_generate_interfaces.cmake index f491ef7ea..3fb0c8411 100644 --- a/rosidl_cmake/cmake/rosidl_generate_interfaces.cmake +++ b/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()