fix(interfaces): strip self-package qualifier from same-package .msg field types#17
Merged
mhidalgo-rai merged 4 commits intoJul 6, 2026
Conversation
…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
self-requested a review
June 11, 2026 12:30
mhidalgo-rai
previously approved these changes
Jun 11, 2026
mhidalgo-rai
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the patch @xiangguomin. and sorry for the delay to review.
LGTM!
Collaborator
|
@xiangguomin looks like CI isn't happy |
Contributor
|
Looks like its just a linting problem in CI. Please install |
Collaborator
|
@xiangguomin friendly ping |
Contributor
Author
Sorry for the late. I was not checking the for a while. Just pushed the fix. Hopefully it can pass. |
Contributor
Author
do we need to manually trigger the ci jobs? |
Contributor
Author
|
Fixed the unit test failure, please help trigger ci job. @mhidalgo-rai |
Contributor
Author
|
Great! Finally got it merged! Thanks @mhidalgo-rai for the help. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
dump_message_specification()emits fully qualified type references for fieldswhose type belongs to the same package as the message being serialised. For
example,
proto2ros/List.msgis generated as:This violates the rosidl convention: same-package references must be bare
(
Value[] values), while cross-package references use thepkg/Typeform.When the generated
.msgfiles are consumed by a Bazelros2_interface_librarytarget, rosidl's type-description generator fails with:
because it looks up
proto2rosin its include map and cannot find the package'sown entry there.
Fix
In
dump_message_specification(), after serialising each field to a string,check whether
field.type.pkg_namematches the enclosing message's package(
message_spec.base_type.pkg_name). If so, strip the self-package prefix fromthe 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 fixtest_dump_preserves_cross_package_qualifier— verifies cross-package refs are untouchedtest_dump_preserves_primitive_fields— verifies messages with no cross-package fields are unaffectedGolden fixture updates
proto2ros_tests::test_message_generationdoes a byte-for-bytefilecmpbetween freshly generated
.msgfiles and the checked-in reference copies inproto2ros_tests/test/generated/. Those reference files encoded the old,buggy self-qualified output (e.g.
proto2ros_tests/Matrix k), so this PRregenerates the 14 affected
.msgfixtures to match the corrected,bare-type output (
Matrix k) now produced bydump_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-filesto satisfy CI linting (blackreformatted a line in
test_interfaces.py).