diff --git a/rosidl_adapter/cmake/rosidl_adapt_interfaces.cmake b/rosidl_adapter/cmake/rosidl_adapt_interfaces.cmake index 0d703c99e..b8835ae66 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(ament_cmake_core REQUIRED) # for get_executable_path @@ -48,6 +50,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 2cfd58295..6e2b3ff36 100644 --- a/rosidl_adapter/rosidl_adapter/__init__.py +++ b/rosidl_adapter/rosidl_adapter/__init__.py @@ -12,12 +12,14 @@ # See the License for the specific language governing permissions and # limitations under the License. +from rosidl_adapter.parser import DEFAULT_ALLOW_LEGACY_FIELD_NAMES -def convert_to_idl(package_dir, package_name, interface_file, output_dir): +def convert_to_idl(package_dir, package_name, interface_file, output_dir, *, allow_legacy_field_naming=DEFAULT_ALLOW_LEGACY_FIELD_NAMES): 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 bbcd50845..8e4b49d03 100644 --- a/rosidl_adapter/rosidl_adapter/main.py +++ b/rosidl_adapter/rosidl_adapter/main.py @@ -20,6 +20,7 @@ from rosidl_adapter import convert_to_idl +from rosidl_adapter.parser import DEFAULT_ALLOW_LEGACY_FIELD_NAMES def main(argv=sys.argv[1:]): @@ -38,6 +39,10 @@ def main(argv=sys.argv[1:]): '--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) @@ -52,7 +57,8 @@ def main(argv=sys.argv[1:]): 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 b02b7b5bd..4fe8afc07 100644 --- a/rosidl_adapter/rosidl_adapter/msg/__init__.py +++ b/rosidl_adapter/rosidl_adapter/msg/__init__.py @@ -12,11 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -from rosidl_adapter.parser import parse_message_string +from rosidl_adapter.parser import parse_message_string, DEFAULT_ALLOW_LEGACY_FIELD_NAMES from rosidl_adapter.resource import expand_template -def convert_msg_to_idl(package_dir, package_name, input_file, output_dir): +def convert_msg_to_idl(package_dir, package_name, input_file, output_dir, *, allow_legacy_field_naming=DEFAULT_ALLOW_LEGACY_FIELD_NAMES): + assert package_dir.is_absolute() assert not input_file.is_absolute() assert input_file.suffix == '.msg' @@ -25,7 +26,7 @@ def convert_msg_to_idl(package_dir, package_name, input_file, output_dir): 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 41ed6bf20..b9ab32c5f 100644 --- a/rosidl_adapter/rosidl_adapter/parser.py +++ b/rosidl_adapter/rosidl_adapter/parser.py @@ -67,12 +67,15 @@ '$') VALID_FIELD_NAME_PATTERN = 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 = re.compile('^[A-Za-z][A-Za-z0-9_]*$') VALID_MESSAGE_NAME_PATTERN = 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]*$') VALID_CONSTANT_NAME_PATTERN = re.compile('^[A-Z]([A-Z0-9_]?[A-Z0-9]+)*$') +# 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 @@ -115,8 +118,16 @@ def is_valid_package_name(name): 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): +def is_valid_field_name(name, allow_legacy_field_naming=DEFAULT_ALLOW_LEGACY_FIELD_NAMES): + if allow_legacy_field_naming: + return is_valid_legacy_field_name(name) try: m = VALID_FIELD_NAME_PATTERN.match(name) except TypeError: @@ -345,12 +356,12 @@ def __str__(self): class Field: - def __init__(self, type_, name, default_value_string=None): + def __init__(self, type_, name, default_value_string=None, *, allow_legacy_field_naming=DEFAULT_ALLOW_LEGACY_FIELD_NAMES): 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)) @@ -462,7 +473,7 @@ def extract_file_level_comments(message_string): return file_level_comments, file_content -def parse_message_string(pkg_name, msg_name, message_string): +def parse_message_string(pkg_name, msg_name, message_string, *, allow_legacy_field_naming=DEFAULT_ALLOW_LEGACY_FIELD_NAMES): fields = [] constants = [] last_element = None # either a field or a constant @@ -518,7 +529,9 @@ def parse_message_string(pkg_name, msg_name, message_string): try: fields.append(Field( Type(type_string, context_package_name=pkg_name), - field_name, default_value_string)) + field_name, 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 51663cd39..6808460e1 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) @@ -127,9 +127,21 @@ 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) + #list(APPEND _arg ALLOW_LEGACY_FIELD_NAMES) + message(WARNING "Allowing legacy field names") + else() + message(FATAL_ERROR NO Legacy field names) + endif() + + #${_rosidl_apt_interfaces_opts} + rosidl_adapt_interfaces( _idl_adapter_tuples "${_adapter_arguments_file}" + ${_rosidl_apt_interfaces_opts} TARGET ${target} ) endif()