Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is there a built in way to read/write a vector of bits/bool? #370

Open
yanshay opened this issue Oct 28, 2023 · 3 comments
Open

Is there a built in way to read/write a vector of bits/bool? #370

yanshay opened this issue Oct 28, 2023 · 3 comments
Labels
enhancement New feature or request

Comments

@yanshay
Copy link

yanshay commented Oct 28, 2023

In the structure I read/write there's a section that represent bits. It begins with a u8 that include the number of bits and then as many u8 as needed (rounded up) that include the bits:

#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
struct Container {
    qty: u8,
    #[deku(bytes_read = "((qty+7)/8)")]
    bits_data: Vec<u8>, 
}

So for example if qty is 12, there are two bytes with the 12 bits of data (and 4 empty).
I am able to read/write it into Vec as shown above but it's inconvenient later to deal with the bits.

  1. I couldn't find a way to read/write into a u32 for example (assuming I know it may be at most 32 bits)
  2. I couldn't find a way to read/write into a Vec, it tried to read the u8 value into the bool rather than read the bits one by one into the bools (which looks like a bug to me)
  3. I would even prefer to read/write into BitVec for easier access and optimal memory footprint.
    I could probably do it with reader/writer, but is there a built in way to do either of the two options I mentioned?
@wcampbell0x2a
Copy link
Collaborator

Away from my computer, but this should work:

use deku::prelude::*;

#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
struct Bit(#[deku(bits = "1")] u8);

#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
struct Container {
    qty: u8,
    #[deku(count = "qty")]
    bits_data: Vec<Bit>,
}

I would even prefer to read/write into BitVec for easier access and optimal memory footprint.

Interesting idea, I'd support a MR for this.

@yanshay
Copy link
Author

yanshay commented Oct 29, 2023

Thanks, that works. I also added padding as in the code below to align on bytes boundary. Its also required for try_form not to fail on too much data. See below.

#[derive(Debug, PartialEq, DekuRead, DekuWrite)]
struct BitContainer {
    qty: u8,
    #[deku(count = "qty", pad_bits_after="(qty+7)/8*8-qty")]
    bits_data: Vec<Bit>,
    field_after: u8
}

What's still missing is automatic filling of qty on write, so I won't need to fill it in the code.

  1. I can't do it using writer, because the pad_bits_after would not work. On read I need padding to refer to the read qty but on write I need it to refer to the length of bits_data. (Interestingly, from my tests, on write deku ignores the 'qty' and decides how many bits to save based on the Vec<Bit> size, I think this is worth explaining in the documentation)
  2. I can solve this using update on the qty field. But that's error prone since it requires client code to remember to update prior to save. I think there would be value in having an attribute update_on_write that executes right before write and has access to self that can calculate automatically fields that can be deduced from other fields
  3. I can do it using writer/reader for only bits_data and expose only bits_data with qty taken care of behind the scene.
  4. Could I define a new type type MyVecBit = Vec<Bit> and implement relevant traits to do the same as 4, only seamlessly from the person defining the structure? Any examples for that? Because this pattern will be used in multiple structures and I don't want to have verbose definitions all over

@wcampbell0x2a
Copy link
Collaborator

wcampbell0x2a commented Oct 31, 2023

I think there would be value in having an attribute update_on_write that executes right before write and has access to self that can calculate automatically fields that can be deduced from other fields

Agreed, this would need to be a breaking change, as DekuWrite doesn't mutate self.

@wcampbell0x2a wcampbell0x2a added the enhancement New feature or request label Dec 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants