-
Notifications
You must be signed in to change notification settings - Fork 0
Open
Labels
Description
Problem
Currently, S3MessageStream::next_message() returns minimal data:
pub async fn next_message(&mut self) -> Option<Result<(ChannelInfo, Vec<u8>), FatalError>>This is inconsistent with the local file reading path (RoboReader) which returns complete metadata via DecodedMessageResult:
pub struct DecodedMessageResult {
pub message: DecodedMessage,
pub channel: ChannelInfo,
pub log_time: Option<u64>,
pub publish_time: Option<u64>, // ← Missing in S3 stream
pub sequence: Option<u64>, // ← Missing in S3 stream
}Background
After integrating the mcap crate via McapS3Adapter, we discovered that S3 streaming returns only (ChannelInfo, Vec<u8>) while local readers return full RawMessage with timestamps and sequence numbers.
The current design uses a "lowest common denominator" approach since:
- MCAP: has
log_time,publish_time,sequence - BAG: has only
log_time - RRD: has synthetic timestamps (not real)
Why This Matters
Users need these fields for:
- Latency analysis:
publish_time - log_timedelays - Drop detection: sequence gaps
- Format conversion: Rewriter needs full metadata
- Consistency: Same API whether local or S3
Proposed Solution
Enhance S3MessageStream to return RawMessage (or a new type) with optional metadata:
pub async fn next_message(&mut self) -> Option<Result<(RawMessage, ChannelInfo), FatalError>>
pub struct RawMessage {
pub channel_id: u16,
pub log_time: Option<u64>, // Some for MCAP/BAG, None for RRD
pub publish_time: Option<u64>, // Some for MCAP, None for BAG/RRD
pub data: Vec<u8>,
pub sequence: Option<u64>, // Some for MCAP, None for BAG/RRD
}Labels
- enhancement
- s3
- good first issue
Reactions are currently unavailable