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

bit order in reader #371

Open
yanshay opened this issue Oct 31, 2023 · 6 comments · May be fixed by #373
Open

bit order in reader #371

yanshay opened this issue Oct 31, 2023 · 6 comments · May be fixed by #373
Labels
enhancement New feature or request

Comments

@yanshay
Copy link

yanshay commented Oct 31, 2023

I saw several issues on bit order, but non are related to reader attribute, so didn't want to add to those more complexity.

I'm writing a reader that needs to read number of bits that's not aligned to byte boundary.
My issue is that I need to read Lsb0 and not Msb0 and seems like DekuRead::read is only Msb0.
Is there a way to read in in reader using Lsb0 (not by reading a byte and reversing, because my reads are not aligned on byte bounday)?

@wcampbell0x2a wcampbell0x2a added the enhancement New feature or request label Nov 3, 2023
@wcampbell0x2a
Copy link
Collaborator

I have a MR that should add features that you want here regarding to bit_order. Have a look at this example/test of using reader and writer and let me know. https://github.com/sharksforarms/deku/pull/373/files#diff-767ccaeaed694dbf38b8e3b5adb7e1d05a7c96f9fd76ac72b854a7b577a6352cR294

Also, if you have an example I can make sure the MR does what you need.

@yanshay
Copy link
Author

yanshay commented Nov 11, 2023

This is how I built my reader/writer, let me know if that serves as an example for reader/writer:
It represents a u8 with the number of bits, followed by bits based on the number in that u8, with LSB first. And then data is byte aligned, so I need to skip the extra bits.

        #[deku(
            reader = "boolvec_reader_lsb0(deku::rest)",
            writer = "boolvec_writer_lsb0(deku::output, channels_status)"
        )]
        channels_status: Vec<bool>,

Theoretically what I'd really want is not to use a reader/writer at all but rather use only deku features.
That would require the bit-order support and also the update-on-write I wrote about in #370 (comment) . That's the benedit of a writer as user of the struct I don't need to deal with updates, they happen automatically, but it's more programming to develop the serialization/deserialization.

Then it would look like:

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

Or preferably bool instead of Bit since Bit as u8 would allow filling more than just true/false, so that's a third feature, allowing bool to be represented as a single bit.

@wcampbell0x2a wcampbell0x2a linked a pull request Dec 27, 2023 that will close this issue
@simonft
Copy link

simonft commented Jan 7, 2024

If it's helpful, I'm also running into a place where having the ability to configure the bit order would be nice. I'm parsing the data described here, which — though it doesn't currently mention it — is in LSb 0.

It's eight 13-bit values packed into 13 bytes. I've got (messy, probably bad rust) code to parse it here. It looks like with the MR I'll be able to do something like this?

#[test]
fn test_probe_status_bit_order() {
    #[derive(DekuRead, PartialEq, Debug)]
    #[deku(bit_order = "ctx_lsb", ctx = "ctx_lsb: Order")]
    pub struct Temperature {
        #[deku(bits = "13")]
        raw_value: u16,
    }

    impl Temperature {
        pub fn new(raw_value: u16) -> Self {
            Temperature { raw_value }
        }
    }

    #[derive(DekuRead, PartialEq, Debug)]
    #[deku(bit_order = "lsb")]
    pub struct ProbeStatus {
        temperatures: [Temperature; 8],
    }

    let data = [
        0x4a, 0x63, 0x69, 0x2c, 0x8d, 0xa5, 0x31, 0x35, 0xaa, 0x46, 0xd5, 0xc0, 0x1a,
    ];

    assert_eq!(
        ProbeStatus {
            temperatures: [
                Temperature::new(842),
                Temperature::new(843),
                Temperature::new(843),
                Temperature::new(843),
                Temperature::new(851),
                Temperature::new(853),
                Temperature::new(853),
                Temperature::new(856),
            ]
        },
        ProbeStatus::try_from(data.as_slice()).unwrap()
    );
}

The rest of the data in that doc is described in LSb 0 but it doesn't present a parsing issue unless the data crosses bytes.

@wcampbell0x2a
Copy link
Collaborator

@simonft Did you already try the branch? I don't have time to make sure your asserts are correct, but I was getting the following error: (this could totally be from just my branch having a bug)

---- test_temp stdout ----
thread 'test_temp' panicked at tests/bit_order.rs:369:5:
assertion failed: `(left == right)`
  left: `ProbeStatus {
    temperatures: [
        Temperature {
            raw_value: 0x34a,
        },
        Temperature {
            raw_value: 0x34b,
        },
        Temperature {
            raw_value: 0x34b,
        },
        Temperature {
            raw_value: 0x34b,
        },
        Temperature {
            raw_value: 0x353,
        },
        Temperature {
            raw_value: 0x355,
        },
        Temperature {
            raw_value: 0x355,
        },
        Temperature {
            raw_value: 0x358,
        },
    ],
}`,
 right: `ProbeStatus {
    temperatures: [
        Temperature {
            raw_value: 0x34a,
        },
        Temperature {
            raw_value: 0x369,
        },
        Temperature {
            raw_value: 0x34b,
        },
        Temperature {
            raw_value: 0x3a5,
        },
        Temperature {
            raw_value: 0x335,
        },
        Temperature {
            raw_value: 0x355,
        },
        Temperature {
            raw_value: 0x1d5,
        },
        Temperature {
            raw_value: 0x358,
        },
    ],
}`

@simonft
Copy link

simonft commented Jan 9, 2024

I hadn't, but I just did. I'm able to reproduce the test case failure, and I've updated the original snippet to be an actual working test case. Trying to figure out what's going wrong is breaking my brain a bit.

Edit: ignore the below, I think I made a mistake.
I created what I think is a simpler test case which grabs 13 bits twice, each that should be all 0s except for the 9th bit (zero indexed). The final assertion fails for me with:

assertion `left == right` failed
  left: BitTest { raw_value1: 512, raw_value2: 512, raw_value3: 0 }
 right: BitTest { raw_value1: 512, raw_value2: 64, raw_value3: 0 }

I'm not 100% sure whether 512 or 64 is the correct answer here, but my intuition is they shouldn't be different.

#[test]
fn test_bit_order_13() {
    #[derive(DekuRead, PartialEq, Debug)]
    #[deku(bit_order = "lsb")]
    pub struct BitTest {
        #[deku(bits = "13")]
        raw_value1: u16,
        #[deku(bits = "13")]
        raw_value2: u16,
        #[deku(bits = "6")]
        raw_value3: u16,
    }

    let data = vec![0b00000000, 0b00000010, 0b01000000, 0b00000000];

    let string_data = data
        .iter()
        .map(|f| (format!("{:08b}", f).chars().rev().collect()))
        .collect::<Vec<String>>()
        .join("");

    println!("string_data: {}", string_data);

    assert_eq!(string_data[0..13], string_data[13..26]);
    assert_eq!(string_data.chars().nth(9).unwrap(), '1');

    assert_eq!(
        BitTest {
            raw_value1: 2_u16.pow(9),
            raw_value2: 2_u16.pow(9),
            raw_value3: 0
        },
        BitTest::try_from(data.as_slice()).unwrap()
    );
}

@wcampbell0x2a
Copy link
Collaborator

I hadn't, but I just did. I'm able to reproduce the test case failure, and I've updated the original snippet to be an actual working test case. Trying to figure out what's going wrong is breaking my brain a bit.

Yes. doing bit_order is a brain bender.

Edit: ignore the below, I think I made a mistake. I created what I think is a simpler test case which grabs 13 bits twice, each that should be all 0s except for the 9th bit (zero indexed). The final assertion fails for me with:

assertion `left == right` failed
  left: BitTest { raw_value1: 512, raw_value2: 512, raw_value3: 0 }
 right: BitTest { raw_value1: 512, raw_value2: 64, raw_value3: 0 }

I'm not 100% sure whether 512 or 64 is the correct answer here, but my intuition is they shouldn't be different.

#[test]
fn test_bit_order_13() {
    #[derive(DekuRead, PartialEq, Debug)]
    #[deku(bit_order = "lsb")]
    pub struct BitTest {
        #[deku(bits = "13")]
        raw_value1: u16,
        #[deku(bits = "13")]
        raw_value2: u16,
        #[deku(bits = "6")]
        raw_value3: u16,
    }

    let data = vec![0b00000000, 0b00000010, 0b01000000, 0b00000000];

    let string_data = data
        .iter()
        .map(|f| (format!("{:08b}", f).chars().rev().collect()))
        .collect::<Vec<String>>()
        .join("");

    println!("string_data: {}", string_data);

    assert_eq!(string_data[0..13], string_data[13..26]);
    assert_eq!(string_data.chars().nth(9).unwrap(), '1');

    assert_eq!(
        BitTest {
            raw_value1: 2_u16.pow(9),
            raw_value2: 2_u16.pow(9),
            raw_value3: 0
        },
        BitTest::try_from(data.as_slice()).unwrap()
    );
}

Ah man, hopefully I didn't cause you much headache, I forgot to apply 62c1ddb when I rebased this branch once! I added your test as an example of it working! 32d4588.

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

Successfully merging a pull request may close this issue.

3 participants