Send a subset of protobuf descriptors #86
Conversation
|
Great how you fixed your own extension. Didn't check the descriptor size. Thank you. |
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes protobuf descriptor handling by sending only the subset of descriptors needed for a specific message type, rather than the entire descriptor pool. This reduces verbosity and prevents confusion in ecal_mon_gui when parsing messages.
Key changes:
- Filter descriptor pool to include only relevant file descriptors for the specific message type
- Remove source_code_info from descriptors to eliminate unnecessary comments from original .proto files
- Add proper type name extraction using the message descriptor
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| .to_string(), | ||
| ); | ||
|
|
||
| // Filter the pool to the set of file decriptors needed |
There was a problem hiding this comment.
There's a typo in the comment: 'decriptors' should be 'descriptors'.
| // Filter the pool to the set of file decriptors needed | |
| // Filter the pool to the set of file descriptors needed |
| descriptor_pool | ||
| .add_file_descriptor_proto(file_descriptor_proto) | ||
| .unwrap(); |
There was a problem hiding this comment.
Using unwrap() here could cause a panic if adding the file descriptor fails. Consider handling this error more gracefully or at least documenting when this could fail.
Summary
Updates the protobuf descriptor to send only the subset of descriptors needed to describe a given protobuf publisher message.
I had a internal crate that had a large number of protobuf message definitions all under the same protobuf package. My naive initial implementation would send the entire descriptor pool of every message for every publisher 😬. Besides being overly verbose it would confuse ecal_mon_gui when trying to parse the messages.
There was also a large amount of comments from the original .proto (prost_types::FileDescriptorProto::source_code_info) that was being send along with the descriptor which is not needed. Also the documentation states it can be removed. https://docs.rs/prost-types/latest/prost_types/struct.FileDescriptorProto.html#structfield.source_code_info Now the descriptor more closely aligns to the form of C++ ecal_sample_person_send.
Related Issues / Discussions
Related to #80
Checklist
cargo check,cargo test)