Skip to content

Conversation

@cmeng-gao
Copy link
Contributor

@cmeng-gao cmeng-gao commented Oct 17, 2025

The websocket server's "protos" request handler was using descriptor->DebugString() which only outputs message definitions, causing top-level enums (like PixelFormatType) to be omitted from the generated proto definitions sent to clients.

This caused protobuf.js clients to fail parsing the definitions because messages referenced enum types that were never defined.

Changes:

  • Extract and include all top-level enums from each proto file
  • Use fileDescriptor->enum_type_count() to iterate enums
  • Add deduplication sets for both files and enums
  • Keep using descriptor->DebugString() for messages to avoid including import statements that would break single-file parsing

The generated proto now includes all necessary enum definitions while remaining compatible with protobuf.js single-file parsing.

Fixes missing PixelFormatType, SphericalCoordinatesType, and other top-level enums in the websocket proto definitions.

Summary

  • ✅ Fixes missing top-level enums (PixelFormatType, etc.)
  • ✅ Makes proto definitions parseable by protobuf.js
  • ✅ Maintains backward compatibility
  • ✅ No breaking changes

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Generated-by messages.

@cmeng-gao cmeng-gao requested a review from arjo129 as a code owner October 17, 2025 05:01
@github-actions github-actions bot added the 🪵 jetty Gazebo Jetty label Oct 17, 2025
The websocket server's "protos" request handler was using
descriptor->DebugString() which only outputs message definitions,
causing top-level enums (like PixelFormatType) to be omitted from
the generated proto definitions sent to clients.

This caused protobuf.js clients to fail parsing the definitions
because messages referenced enum types that were never defined.

Changes:
- Extract and include all top-level enums from each proto file
- Use fileDescriptor->enum_type_count() to iterate enums
- Add deduplication sets for both files and enums
- Keep using descriptor->DebugString() for messages to avoid
  including import statements that would break single-file parsing

The generated proto now includes all necessary enum definitions
while remaining compatible with protobuf.js single-file parsing.

Fixes missing PixelFormatType, SphericalCoordinatesType, and other
top-level enums in the websocket proto definitions.

Signed-off-by: cmeng <[email protected]>
@cmeng-gao cmeng-gao force-pushed the fix/websocket-proto-enums branch from 91a08d9 to 03676b3 Compare October 17, 2025 05:12
@azeey
Copy link
Contributor

azeey commented Oct 17, 2025

Thanks for the contribution! I believe this is your first time contributing, so can you please go through the checklist and check-off relevant items?

@cmeng-gao
Copy link
Contributor Author

Sorry, is there anything else I need to do regarding this MR?

@cmeng-gao
Copy link
Contributor Author

@arjo129

@arjo129 arjo129 requested a review from iche033 November 7, 2025 14:11
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

The CI failures should be unrelated. Changes look good to me.

@github-project-automation github-project-automation bot moved this from Inbox to In review in Core development Nov 7, 2025
@iche033 iche033 merged commit f53d4ab into gazebosim:gz-sim10 Nov 7, 2025
15 of 18 checks passed
@github-project-automation github-project-automation bot moved this from In review to Done in Core development Nov 7, 2025
@iche033
Copy link
Contributor

iche033 commented Nov 7, 2025

@Mergifyio backport main

@mergify
Copy link
Contributor

mergify bot commented Nov 7, 2025

backport main

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Nov 7, 2025
)

The websocket server's "protos" request handler was using
descriptor->DebugString() which only outputs message definitions,
causing top-level enums (like PixelFormatType) to be omitted from
the generated proto definitions sent to clients.

This caused protobuf.js clients to fail parsing the definitions
because messages referenced enum types that were never defined.

Changes:
- Extract and include all top-level enums from each proto file
- Use fileDescriptor->enum_type_count() to iterate enums
- Add deduplication sets for both files and enums
- Keep using descriptor->DebugString() for messages to avoid
  including import statements that would break single-file parsing

The generated proto now includes all necessary enum definitions
while remaining compatible with protobuf.js single-file parsing.

Fixes missing PixelFormatType, SphericalCoordinatesType, and other
top-level enums in the websocket proto definitions.

Signed-off-by: cmeng <[email protected]>
(cherry picked from commit f53d4ab)
iche033 pushed a commit that referenced this pull request Nov 10, 2025
)

The websocket server's "protos" request handler was using
descriptor->DebugString() which only outputs message definitions,
causing top-level enums (like PixelFormatType) to be omitted from
the generated proto definitions sent to clients.

This caused protobuf.js clients to fail parsing the definitions
because messages referenced enum types that were never defined.

Changes:
- Extract and include all top-level enums from each proto file
- Use fileDescriptor->enum_type_count() to iterate enums
- Add deduplication sets for both files and enums
- Keep using descriptor->DebugString() for messages to avoid
  including import statements that would break single-file parsing

The generated proto now includes all necessary enum definitions
while remaining compatible with protobuf.js single-file parsing.

Fixes missing PixelFormatType, SphericalCoordinatesType, and other
top-level enums in the websocket proto definitions.

Signed-off-by: cmeng <[email protected]>
(cherry picked from commit f53d4ab)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🪵 jetty Gazebo Jetty

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants