Channel groups (legacy layers) #147
Draft
Conversation
This file contains 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
Owner
johannesvollmer
commented
Jan 3, 2022
•
johannesvollmer
commented
- add separate tests for the channel name parser and collector
- add builder methods
- test for specific channels in particular
- add user channelfilter?
- revisit public interface and add convenience methods
- examples
- Guide Update
Open
This was
linked to
issues
Jan 3, 2022
Open
johannesvollmer
commented
Jan 4, 2022
| let new_child_groups = HashMap::with_capacity(self.child_groups.len()); | ||
| let child_groups = self.child_groups.into_iter() | ||
| .map(|(name, child)| Ok((name, child.try_map(&mut mapper)?))) | ||
| .try_fold( |
There was a problem hiding this comment.
johannesvollmer
commented
Jan 4, 2022
| }) | ||
| impl<Channels> ChannelsWriter for GroupChannelsWriter<Channels> where Channels: ChannelsWriter { | ||
| fn extract_uncompressed_block(&self, header: &Header, block_index: BlockIndex, output_block_data: &mut [u8]) { | ||
| for channels_group in &self.all_channel_groups { |
There was a problem hiding this comment.
Each subgroup will write to the same few first bytes, overwriting each other. Incorporate some kind of offset?
johannesvollmer
commented
Jan 11, 2022
| GroupChannelsWriter { channels_list: channels } | ||
| GroupChannelsWriter { | ||
| all_channel_groups: self.all_channel_groups() | ||
| .map(|channel_group: &ChannelGroup| panic!("this uses relative names but expects absolute names, and all will write first byte")/*channel_group.create_channel_group_writer(header)*/) |
There was a problem hiding this comment.
SpecificChannels currently does a lookup based on the channel name, in order to find the byte offset, for each channel. This cannot work as the meta data channels have absolute names while the SpecificChannels don't
johannesvollmer
commented
Jan 21, 2022
| Ok(ChannelGroupsReader { | ||
| channels: channel_groups, | ||
| indexed_channels, | ||
| let reader = self.read_channels.create_channels_reader(header, &group_selected_channel_indices); |
There was a problem hiding this comment.
At this point, meta data contains fully qualified names, but the specific channels and any channels must work with relative local names. Pass some kind of string map instead of only indices?
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.