Skip to content

fix(interfaces): strip self-package qualifier from same-package .msg field types#17

Merged
mhidalgo-rai merged 4 commits into
rai-opensource:mainfrom
xiangguomin:fix/strip-self-package-qualifier-in-msg-output
Jul 6, 2026
Merged

fix(interfaces): strip self-package qualifier from same-package .msg field types#17
mhidalgo-rai merged 4 commits into
rai-opensource:mainfrom
xiangguomin:fix/strip-self-package-qualifier-in-msg-output

Conversation

@xiangguomin

@xiangguomin xiangguomin commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Problem

dump_message_specification() emits fully qualified type references for fields
whose type belongs to the same package as the message being serialised. For
example, proto2ros/List.msg is generated as:

proto2ros/Value[] values

This violates the rosidl convention: same-package references must be bare
(Value[] values), while cross-package references use the pkg/Type form.
When the generated .msg files are consumed by a Bazel ros2_interface_library
target, rosidl's type-description generator fails with:

KeyError: 'proto2ros'

because it looks up proto2ros in its include map and cannot find the package's
own entry there.

Fix

In dump_message_specification(), after serialising each field to a string,
check whether field.type.pkg_name matches the enclosing message's package
(message_spec.base_type.pkg_name). If so, strip the self-package prefix from
the output line. Cross-package references and primitive fields (where
field.type.pkg_name is None) are unaffected.

Tests

Three unit tests added to proto2ros/output/test_interfaces.py:

  • test_dump_strips_self_package_qualifier — verifies the fix
  • test_dump_preserves_cross_package_qualifier — verifies cross-package refs are untouched
  • test_dump_preserves_primitive_fields — verifies messages with no cross-package fields are unaffected

Golden fixture updates

proto2ros_tests::test_message_generation does a byte-for-byte filecmp
between freshly generated .msg files and the checked-in reference copies in
proto2ros_tests/test/generated/. Those reference files encoded the old,
buggy self-qualified output (e.g. proto2ros_tests/Matrix k), so this PR
regenerates the 14 affected .msg fixtures to match the corrected,
bare-type output (Matrix k) now produced by dump_message_specification().
No other consumers read these files — they exist solely as the golden
snapshot for this test.

Lint

Also ran pre-commit run --all-files to satisfy CI linting (black
reformatted a line in test_interfaces.py).

…field types

rosidl's type-description generator fails with KeyError when a package's
own .msg files use fully qualified self-references such as
`proto2ros/Value[] values` (where the enclosing message also belongs to
`proto2ros`). The rosidl convention — followed by every other ROS package
— is to use bare type names for same-package references and fully qualified
names only for cross-package references.

Fix dump_message_specification() to strip the self-package prefix from a
field's serialised type string when field.type.pkg_name matches the
message's own package. Cross-package references and primitive fields are
unaffected.

Adds three unit tests covering: self-reference stripping, cross-package
preservation, and primitive (no-package) fields.
@mhidalgo-rai
mhidalgo-rai self-requested a review June 11, 2026 12:30
mhidalgo-rai
mhidalgo-rai previously approved these changes Jun 11, 2026

@mhidalgo-rai mhidalgo-rai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the patch @xiangguomin. and sorry for the delay to review.

LGTM!

@mhidalgo-rai

Copy link
Copy Markdown
Collaborator

@xiangguomin looks like CI isn't happy

@tcappellari-bdai

Copy link
Copy Markdown
Contributor

Looks like its just a linting problem in CI. Please install pre-commit and run pre-commit run --all-files

@mhidalgo-rai

Copy link
Copy Markdown
Collaborator

@xiangguomin friendly ping

@xiangguomin

Copy link
Copy Markdown
Contributor Author

@xiangguomin friendly ping

Sorry for the late. I was not checking the for a while. Just pushed the fix. Hopefully it can pass.

@xiangguomin

Copy link
Copy Markdown
Contributor Author

@xiangguomin friendly ping

Sorry for the late. I was not checking the for a while. Just pushed the fix. Hopefully it can pass.

do we need to manually trigger the ci jobs?

@xiangguomin

Copy link
Copy Markdown
Contributor Author

Fixed the unit test failure, please help trigger ci job. @mhidalgo-rai

@mhidalgo-rai mhidalgo-rai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mhidalgo-rai
mhidalgo-rai merged commit 68ae244 into rai-opensource:main Jul 6, 2026
5 checks passed
@xiangguomin

Copy link
Copy Markdown
Contributor Author

Great! Finally got it merged! Thanks @mhidalgo-rai for the help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants