From 72ce22247c17c0ea2e4e297ed857d4f355273b7a Mon Sep 17 00:00:00 2001 From: Lidang-Jiang Date: Wed, 29 Apr 2026 08:53:20 +0800 Subject: [PATCH] Use absolute imports for generated local type support Signed-off-by: Lidang-Jiang --- rosidl_generator_py/CMakeLists.txt | 4 ++ rosidl_generator_py/msg/Duration.msg | 1 + .../msg/DurationArraySequence.msg | 2 + rosidl_generator_py/package.xml | 1 + rosidl_generator_py/resource/_msg.py.em | 71 ++++++++++--------- .../resource/_msg_check_fields.py.em | 36 +++++++++- rosidl_generator_py/test/test_interfaces.py | 45 ++++++++++++ 7 files changed, 124 insertions(+), 36 deletions(-) create mode 100644 rosidl_generator_py/msg/Duration.msg create mode 100644 rosidl_generator_py/msg/DurationArraySequence.msg diff --git a/rosidl_generator_py/CMakeLists.txt b/rosidl_generator_py/CMakeLists.txt index e5ff834f..e0d0f723 100644 --- a/rosidl_generator_py/CMakeLists.txt +++ b/rosidl_generator_py/CMakeLists.txt @@ -14,6 +14,7 @@ ament_python_install_package(${PROJECT_NAME}) if(BUILD_TESTING) find_package(ament_cmake_pytest REQUIRED) + find_package(builtin_interfaces REQUIRED) find_package(rmw REQUIRED) find_package(rosidl_cmake REQUIRED) @@ -47,8 +48,11 @@ if(BUILD_TESTING) ${test_interface_files_MSG_FILES} # Cases not covered by test_interface_files msg/BuiltinTypeSequencesIdl.idl + msg/Duration.msg + msg/DurationArraySequence.msg msg/StringArrays.msg msg/Property.msg + DEPENDENCIES builtin_interfaces ADD_LINTER_TESTS SKIP_INSTALL ) diff --git a/rosidl_generator_py/msg/Duration.msg b/rosidl_generator_py/msg/Duration.msg new file mode 100644 index 00000000..cbe51b2c --- /dev/null +++ b/rosidl_generator_py/msg/Duration.msg @@ -0,0 +1 @@ +builtin_interfaces/Duration data diff --git a/rosidl_generator_py/msg/DurationArraySequence.msg b/rosidl_generator_py/msg/DurationArraySequence.msg new file mode 100644 index 00000000..e49baa20 --- /dev/null +++ b/rosidl_generator_py/msg/DurationArraySequence.msg @@ -0,0 +1,2 @@ +builtin_interfaces/Duration[2] array_data +builtin_interfaces/Duration[] sequence_data diff --git a/rosidl_generator_py/package.xml b/rosidl_generator_py/package.xml index 2f46115d..a1d8cee4 100644 --- a/rosidl_generator_py/package.xml +++ b/rosidl_generator_py/package.xml @@ -53,6 +53,7 @@ ament_index_python ament_lint_auto ament_lint_common + builtin_interfaces python3-numpy python3-pytest rmw diff --git a/rosidl_generator_py/resource/_msg.py.em b/rosidl_generator_py/resource/_msg.py.em index a8682df0..e093a3e6 100644 --- a/rosidl_generator_py/resource/_msg.py.em +++ b/rosidl_generator_py/resource/_msg.py.em @@ -30,6 +30,27 @@ from rosidl_parser.definition import NamespacedType from rosidl_parser.definition import SIGNED_INTEGER_TYPES from rosidl_parser.definition import UnboundedSequence from rosidl_parser.definition import UNSIGNED_INTEGER_TYPES + + +def get_importable_namespaced_type( + type_: NamespacedType, + action_goal_suffix: str = ACTION_GOAL_SUFFIX, + action_result_suffix: str = ACTION_RESULT_SUFFIX, + action_feedback_suffix: str = ACTION_FEEDBACK_SUFFIX, + camel_to_snake=convert_camel_case_to_lower_case_underscore, +) -> tuple[str, str]: + joined_type_namespaces = '.'.join(type_.namespaces) + if ( + type_.name.endswith(action_goal_suffix) or + type_.name.endswith(action_result_suffix) or + type_.name.endswith(action_feedback_suffix) + ): + action_name, _ = type_.name.rsplit('_', 1) + lower_case_name = camel_to_snake(action_name) + module_name = f'{joined_type_namespaces}._{lower_case_name}' + else: + module_name = joined_type_namespaces + return module_name, f'{module_name}.{type_.name}' }@ @{ import_type_checking = False @@ -182,22 +203,13 @@ for member in message.structure.members: type_.name.endswith(SERVICE_REQUEST_MESSAGE_SUFFIX) ): continue - if ( - type_.name.endswith(ACTION_GOAL_SUFFIX) or - type_.name.endswith(ACTION_RESULT_SUFFIX) or - type_.name.endswith(ACTION_FEEDBACK_SUFFIX) - ): - action_name, suffix = type_.name.rsplit('_', 1) - typename = (*type_.namespaces, action_name, action_name + '.' + suffix) - else: - typename = (*type_.namespaces, type_.name, type_.name) - importable_typesupports.add(typename) + importable_typesupports.add(get_importable_namespaced_type(type_)) }@ -@[for typename in sorted(importable_typesupports)]@ +@[for module_name, type_name in sorted(importable_typesupports)]@ - from @('.'.join(typename[:-2])) import @(typename[-2]) - if @(typename[-1])._TYPE_SUPPORT is None: - @(typename[-1]).__import_type_support__() + import @(module_name) + if @(type_name)._TYPE_SUPPORT is None: + @(type_name).__import_type_support__() @[end for]@ @@classmethod @@ -370,15 +382,10 @@ if isinstance(type_, AbstractNestedType): self.@(member.name) = @(member.name) if @(member.name) is not None else @(message.structure.namespaced_type.name).@(member.name.upper())__DEFAULT @[ else]@ @[ if isinstance(type_, NamespacedType) and not isinstance(member.type, AbstractSequence)]@ -@[ if ( - type_.name.endswith(ACTION_GOAL_SUFFIX) or - type_.name.endswith(ACTION_RESULT_SUFFIX) or - type_.name.endswith(ACTION_FEEDBACK_SUFFIX) - )]@ - from @('.'.join(type_.namespaces))._@(convert_camel_case_to_lower_case_underscore(type_.name.rsplit('_', 1)[0])) import @(type_.name) -@[ else]@ - from @('.'.join(type_.namespaces)) import @(type_.name) -@[ end if]@ +@{ +module_name, type_name = get_importable_namespaced_type(type_) +}@ + import @(module_name) @[ end if]@ @[ if isinstance(member.type, Array)]@ @[ if isinstance(type_, BasicType) and type_.typename == 'octet']@ @@ -392,7 +399,8 @@ if isinstance(type_, AbstractNestedType): else: self.@(member.name) = @(member.name) @[ else]@ - self.@(member.name) = @(member.name) if @(member.name) is not None else [@(get_python_type(type_))() for x in range(@(member.type.size))] +@{default_type = type_name if isinstance(type_, NamespacedType) else get_python_type(type_)}@ + self.@(member.name) = @(member.name) if @(member.name) is not None else [@(default_type)() for x in range(@(member.type.size))] @[ end if]@ @[ end if]@ @[ elif isinstance(member.type, AbstractSequence)]@ @@ -405,6 +413,8 @@ if isinstance(type_, AbstractNestedType): self.@(member.name) = @(member.name) if @(member.name) is not None else bytes([0]) @[ elif isinstance(type_, BasicType) and type_.typename in CHARACTER_TYPES]@ self.@(member.name) = @(member.name) if @(member.name) is not None else chr(0) +@[ elif isinstance(type_, NamespacedType)]@ + self.@(member.name) = @(member.name) if @(member.name) is not None else @(type_name)() @[ else]@ self.@(member.name) = @(member.name) if @(member.name) is not None else @(get_python_type(type_))() @[ end if]@ @@ -504,15 +514,10 @@ if isinstance(member.type, (Array, AbstractSequence)): @[ end if]@ @[ end if]@ @[ if isinstance(type_, NamespacedType)]@ -@[ if ( - type_.name.endswith(ACTION_GOAL_SUFFIX) or - type_.name.endswith(ACTION_RESULT_SUFFIX) or - type_.name.endswith(ACTION_FEEDBACK_SUFFIX) - )]@ - from @('.'.join(type_.namespaces))._@(convert_camel_case_to_lower_case_underscore(type_.name.rsplit('_', 1)[0])) import @(type_.name) -@[ else]@ - from @('.'.join(type_.namespaces)) import @(type_.name) -@[ end if]@ +@{ +module_name, type_name = get_importable_namespaced_type(type_) +}@ + import @(module_name) @[ end if]@ @{ diff --git a/rosidl_generator_py/resource/_msg_check_fields.py.em b/rosidl_generator_py/resource/_msg_check_fields.py.em index 50ad4128..ece3b37f 100644 --- a/rosidl_generator_py/resource/_msg_check_fields.py.em +++ b/rosidl_generator_py/resource/_msg_check_fields.py.em @@ -3,6 +3,9 @@ from rosidl_parser.definition import AbstractGenericString from rosidl_parser.definition import AbstractNestedType from rosidl_parser.definition import AbstractSequence from rosidl_parser.definition import Array +from rosidl_parser.definition import ACTION_FEEDBACK_SUFFIX +from rosidl_parser.definition import ACTION_GOAL_SUFFIX +from rosidl_parser.definition import ACTION_RESULT_SUFFIX from rosidl_parser.definition import BasicType from rosidl_parser.definition import BOOLEAN_TYPE from rosidl_parser.definition import INTEGER_TYPES @@ -11,8 +14,35 @@ from rosidl_parser.definition import FLOATING_POINT_TYPES from rosidl_parser.definition import SIGNED_INTEGER_TYPES from rosidl_parser.definition import UNSIGNED_INTEGER_TYPES from rosidl_parser.definition import NamespacedType +from rosidl_pycommon import convert_camel_case_to_lower_case_underscore from rosidl_generator_py.generate_py_impl import get_python_type from rosidl_generator_py.generate_py_impl import SPECIAL_NESTED_BASIC_TYPES + + +def get_importable_namespaced_type( + type_: NamespacedType, + action_goal_suffix: str = ACTION_GOAL_SUFFIX, + action_result_suffix: str = ACTION_RESULT_SUFFIX, + action_feedback_suffix: str = ACTION_FEEDBACK_SUFFIX, + camel_to_snake=convert_camel_case_to_lower_case_underscore, +) -> tuple[str, str]: + joined_type_namespaces = '.'.join(type_.namespaces) + if ( + type_.name.endswith(action_goal_suffix) or + type_.name.endswith(action_result_suffix) or + type_.name.endswith(action_feedback_suffix) + ): + action_name, _ = type_.name.rsplit('_', 1) + lower_case_name = camel_to_snake(action_name) + module_name = f'{joined_type_namespaces}._{lower_case_name}' + else: + module_name = joined_type_namespaces + return module_name, f'{module_name}.{type_.name}' +}@ +@{ +python_type = get_python_type(type_) +if isinstance(type_, NamespacedType): + _, python_type = get_importable_namespaced_type(type_) }@ if self._check_fields: @[ if isinstance(member.type, AbstractNestedType)]@ @@ -61,8 +91,8 @@ from rosidl_generator_py.generate_py_impl import SPECIAL_NESTED_BASIC_TYPES @{assert_msg_suffixes.insert(1, 'with length %d' % member.type.size)}@ @[ end if]@ @[ end if]@ - all(isinstance(v, @(get_python_type(type_))) for v in value) and -@{assert_msg_suffixes.append("and each value of type '%s'" % get_python_type(type_))}@ + all(isinstance(v, @(python_type)) for v in value) and +@{assert_msg_suffixes.append("and each value of type '%s'" % python_type)}@ @[ if isinstance(type_, BasicType) and type_.typename in SIGNED_INTEGER_TYPES]@ @{ nbits = int(type_.typename[3:]) @@ -106,7 +136,7 @@ bound = 1.7976931348623157e+308 "The '@(member.name)' field must be string value " \ 'not longer than @(type_.maximum_size)' @[ elif isinstance(type_, NamespacedType)]@ - isinstance(value, @(type_.name)), \ + isinstance(value, @(python_type)), \ "The '@(member.name)' field must be a sub message of type '@(type_.name)'" @[ elif isinstance(type_, BasicType) and type_.typename == 'octet']@ (isinstance(value, (bytes, bytearray, memoryview)) and diff --git a/rosidl_generator_py/test/test_interfaces.py b/rosidl_generator_py/test/test_interfaces.py index 64225d9c..d3ae7083 100644 --- a/rosidl_generator_py/test/test_interfaces.py +++ b/rosidl_generator_py/test/test_interfaces.py @@ -14,6 +14,7 @@ import array import math +import pathlib import sys import numpy @@ -25,6 +26,8 @@ from rosidl_generator_py.msg import BuiltinTypeSequencesIdl from rosidl_generator_py.msg import Constants from rosidl_generator_py.msg import Defaults +from rosidl_generator_py.msg import Duration +from rosidl_generator_py.msg import DurationArraySequence from rosidl_generator_py.msg import Nested from rosidl_generator_py.msg import StringArrays from rosidl_generator_py.msg import Strings @@ -269,6 +272,48 @@ def test_constructor() -> None: Strings(unknown_field='test', check_fields=True) +def test_namespaced_field_imports_are_absolute() -> None: + duration_module = sys.modules[Duration.__module__] + duration_file = duration_module.__file__ + assert duration_file is not None + duration_source = pathlib.Path(duration_file).read_text(encoding='utf-8') + + assert 'from builtin_interfaces.msg import Duration' not in duration_source + assert 'import builtin_interfaces.msg' in duration_source + assert 'builtin_interfaces.msg.Duration._TYPE_SUPPORT' in duration_source + + from builtin_interfaces.msg import Duration as BuiltinDuration + + Duration.__import_type_support__() + + msg = Duration(check_fields=True) + assert isinstance(msg.data, BuiltinDuration) + + +def test_namespaced_array_sequence_fields_are_absolute() -> None: + duration_module = sys.modules[DurationArraySequence.__module__] + duration_file = duration_module.__file__ + assert duration_file is not None + duration_source = pathlib.Path(duration_file).read_text(encoding='utf-8') + + assert 'from builtin_interfaces.msg import Duration' not in duration_source + assert 'import builtin_interfaces.msg' in duration_source + + from builtin_interfaces.msg import Duration as BuiltinDuration + + msg = DurationArraySequence(check_fields=True) + assert all(isinstance(value, BuiltinDuration) for value in msg.array_data) + assert [] == msg.sequence_data + + msg.array_data = [BuiltinDuration(sec=1), BuiltinDuration(sec=2)] + msg.sequence_data = [BuiltinDuration(sec=3)] + + with pytest.raises(AssertionError): + msg.array_data = [BuiltinDuration(), object()] + with pytest.raises(AssertionError): + msg.sequence_data = [object()] + + def test_constants() -> None: assert Constants.BOOL_CONST is True assert bytes([50]) == Constants.BYTE_CONST