Fix channel topic filtering - #475
Merged
Merged
Conversation
kstonekuan
approved these changes
Sep 10, 2026
mulkakhileshmj
added a commit
to mulkakhileshmj/hflow
that referenced
this pull request
Sep 10, 2026
The Episode.channel() tests kept passing with the derivation deleted once Hebbian-Robotics#475 passed the topic explicitly on that path, so nothing pinned this change. A new test drives Episode.iter_decoded_batches with only channel_ids, the remaining caller that reaches the reader with topics left None, and asserts the derived topic is what the mcap reader receives and that the underlying read narrows. The no-summary fallback is exercised too: a file written without chunking, statistics, summary offsets, or repeated channel records has no summary section at all, channels() refuses it with ValueError, and a channel id read falls back to the unconstrained scan while still returning only the requested channel. Measured on a truncated file before deciding the catch width: channels() raises mcap's RecordLengthLimitExceeded there, not ValueError, and every read of such a file surfaces the same error whether or not the derivation catches it, so the narrow catch is deliberate and the code comment now records that. The two-topic writer block moved into a helper the three tests share.
kstonekuan
added a commit
that referenced
this pull request
Sep 11, 2026
* perf: pass channel topics down to the MCAP reader Episode.channel() selected a channel by id but never turned that id into a topic filter, so the underlying MCAP reader yielded every stream in the file and hflow discarded the unrelated ones in Python. Reading a small state channel could decompress gigabytes of camera data for nothing. PythonMcapEpisodeReader.iter_batches() now derives the topics for the requested channel ids from the file summary and passes them to iter_messages(), so the reader can skip unrelated chunks. The channel id filter stays in place because several channels can share one topic, and files without a summary section fall back to the unconstrained read. Every channel_ids caller benefits, including iter_decoded_batches(). Two regression tests pin the behavior: one asserts the requested topic reaches the MCAP reader, the other asserts channel id selection still wins when two channels share a topic. Closes #470 * test: pin the reader-level topic derivation and its no-summary fallback The Episode.channel() tests kept passing with the derivation deleted once #475 passed the topic explicitly on that path, so nothing pinned this change. A new test drives Episode.iter_decoded_batches with only channel_ids, the remaining caller that reaches the reader with topics left None, and asserts the derived topic is what the mcap reader receives and that the underlying read narrows. The no-summary fallback is exercised too: a file written without chunking, statistics, summary offsets, or repeated channel records has no summary section at all, channels() refuses it with ValueError, and a channel id read falls back to the unconstrained scan while still returning only the requested channel. Measured on a truncated file before deciding the catch width: channels() raises mcap's RecordLengthLimitExceeded there, not ValueError, and every read of such a file surfaces the same error whether or not the derivation catches it, so the narrow catch is deliberate and the code comment now records that. The two-topic writer block moved into a helper the three tests share. * test(reader): pin that an explicit topics= is never replaced by the derived one Dropping the 'topics is None' half of the condition left the whole suite green. Episode.channel() passes both a topic and a channel id, and for that caller the derived topic equals the explicit one, so nothing noticed. Asking for one topic while naming a channel on another separates them: the reader must pass down what the caller asked for and yield nothing, not quietly read the other stream instead. --------- Co-authored-by: Kingston <kingston@hebbianrobotics.com>
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
2 participants
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.
fixes issue #470
Changes
Episode.channel()topic filtering.EpisodeReader.iter_batches()while retaining the channel_id filter.Verification
The script was run before and after the change.
Before the change
MCAP yielded all 9 messages because no topic filter was passed, including 8 unrelated /camera messages.
After the change
MCAP now receives the
/targettopic filter and yields only the requested topic, avoiding unnecessary reads of unrelated messages.