Open LeonardMH opened 1 year ago
This (unsurprisingly) parses correctly, but loses the bitfield definitions for the u32 type, that's the one I can't figure out.
#[derive(Debug, Copy, Clone, PartialEq, DekuRead, DekuWrite)]
#[deku(endian = "little")]
struct TestPacket {
f0: u32,
#[deku(bits = "10", pad_bits_after = "6")]
f4: u16,
f5: u16,
f6: u16,
f7: u16,
}
Sorry @LeonardMH there hasn't been any progress on this.
My current recommendation is to massage your LSB data into MSB, if possible, before feeding it to deku. However this may not be possible/ideal. An alternative is to reverse the bits and have your struct defined in reverse... also not ideal.
Finally, you could use map
to reverse the bits in place after parsing. Maybe map = "|field: u32| -> Result<_, DekuError> { Ok(field.reverse_bits()) }"
To help debug, there's a logging
feature, initialize logging and compile with the logging feature, here's example output from your example.
env_logger::init();
$ RUST_LOG=trace cargo run --example test --features logging
[2023-06-20T13:25:41Z TRACE test] Reading: TestPacket::f1 from [0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 0, 1, 1, 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 1, 1, 0, 1, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0]
[2023-06-20T13:25:41Z TRACE test] Reading: TestPacket::f2 from [0, 1, 1, 1, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 1, 1, 0, 1, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0]
[2023-06-20T13:25:41Z TRACE test] Reading: TestPacket::f3 from [1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 1, 1, 0, 1, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0]
[2023-06-20T13:25:41Z TRACE test] Reading: TestPacket::f4 from [0, 1, 1, 1, 0, 0, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0]
[2023-06-20T13:25:41Z TRACE test] Reading: TestPacket::f5 from [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0]
[2023-06-20T13:25:41Z TRACE test] Reading: TestPacket::f6 from [0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0]
[2023-06-20T13:25:41Z TRACE test] Reading: TestPacket::f7 from [0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0]
thread 'main' panicked at 'assertion failed: `(left == right)`
left: `TestPacket { f1: 769, f2: 114, f3: 130, f4: 115, f5: 0, f6: 1, f7: 1 }`,
right: `TestPacket { f1: 1, f2: 567, f3: 568, f4: 115, f5: 0, f6: 1, f7: 1 }`', examples/test.rs:60:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
I'm a bit confused by your test case, for example the first 10 bits [0, 0, 0, 0, 0, 0, 0, 1, 1, 1]
, no matter how you read it, won't be 1? Could you clarify?
Thanks for the quick response, hopefully I will be able to figure this out now that I know how to access the logging feature.
As for 'the first 10 bits being 1', that's not exactly what I expect, in my mind how this would work is that the first 32-bits would be interpreted as a little endian u32
and the definition I gave in TestStruct
would then apply to that u32
. I tried to clarify that in my comments in the code example.
[0x01, 0xDC, 0x88, 0x23] -> u32-> 0x2388DC01
If you take the 'first 10 bits' of that u32
in LSb first order f1
would be 1. But like I said, I know that's probably not how it works since you are working with BitVecs directly and going MSb first, the example I showed was my 'ideal' layout as it exactly matches the C struct definition. The layout I really thought should work based on my understanding of how deku
reads the data is:
#[derive(Debug, Copy, Clone, PartialEq, DekuRead, DekuWrite)]
#[deku(endian = "little")]
struct TestPacket {
// Word 1: Bytes 1-4
#[deku(bits = 10, pad_bits_before = "2")]
f3: u32,
#[deku(bits = 10)]
f2: u32,
#[deku(bits = 10)]
f1: u32,
// Bytes 5-6
#[deku(bits = 10, pad_bits_after = "6")]
f4: u16,
f5: u16, // Bytes 7-8
f6: u16, // Bytes 9-10
f7: u16, // Bytes 11-12
}
But that fails with:
thread 'main' panicked at 'assertion failed: `(left == right)`
left: `TestPacket { f3: 263, f2: 712, f1: 776, f4: 115, f5: 0, f6: 1, f7: 1 }`,
right: `TestPacket { f3: 568, f2: 567, f1: 1, f4: 115, f5: 0, f6: 1, f7: 1 }`', src/main.rs:44:5
I'll keep poking around with the logging and see if I can wrap my head around what order the individual bits are being passed in. As for this:
Sorry @LeonardMH there hasn't been any progress on this.
No need for apologies, this library is very impressive and almost exactly what I want. I have hundreds of these types of structs that I need to define so if I can get this working you'll save me hours and hours of writing manual bitshifts and masks by hand, I can deal with a little bit of weirdness. It would just be a nice quality of life upgrade.
I forked the repo and changed all instances of Msb0 to Lsb0 just to test that out, I am starting to think Msb0 is fundamentally incompatible with how I want to parse this data. If I can get that working I will look at what you have already done on the bit_order
branch a few years ago and see if I can get something PR worthy working on top of master
.
Unfortunately, I'm seeing one more weird thing I hope you can help with. When I switch to Lsb0, f1
parses correctly, and the bitvecs that get printed out look like they should parse as expected, but they don't. Here's the trace:
[2023-06-20T19:55:04Z TRACE test] Reading: TestPacket::f1 from [1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 0, 1, 1, 0, 0, 0, 1, 0, 0, 0, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1, 1, 0, 0, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
[2023-06-20T19:55:04Z TRACE test] Reading: TestPacket::f2 from [1, 1, 1, 0, 1, 1, 0, 0, 0, 1, 0, 0, 0, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1, 1, 0, 0, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
[2023-06-20T19:55:04Z TRACE test] Reading: TestPacket::f3 from [0, 0, 0, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1, 1, 0, 0, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
[2023-06-20T19:55:04Z TRACE test] Reading: TestPacket::f4 from [1, 1, 0, 0, 1, 1, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
[2023-06-20T19:55:04Z TRACE test] Reading: TestPacket::f5 from [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
[2023-06-20T19:55:04Z TRACE test] Reading: TestPacket::f6 from [1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
[2023-06-20T19:55:04Z TRACE test] Reading: TestPacket::f7 from [1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
And here's the output:
left: `TestPacket { f1: 1, f2: 32823, f3: 32824, f4: 115, f5: 0, f6: 1, f7: 1 }`,
right: `TestPacket { f1: 1, f2: 567, f3: 568, f4: 115, f5: 0, f6: 1, f7: 1 }`', src/main.rs:46:5
Just looking at f2
, if you take those next ten bits it should be parsing [1, 1, 1, 0, 1, 1, 0, 0, 0, 1]
, which is 0b1000110111
or 0x237, exactly what I am looking for. So why would this end up being parsed as 32823 == 0x8037? It's so close...
Ok, so that issue is probably as a result of the change from Msb0 to Lsb0, I added a bunch of debug tracing in the ImplDekuReadBytes
and ImplDekuReadBits
and narrowed this specific failure down to how ImplDekuReadReadBits
creates a new bitvec and adds padding.
As it was:
// Create a new BitVec from the slice and pad un-aligned chunks
// i.e. [10010110, 1110] -> [10010110, 00001110]
let bits: BitVec<u8, Lsb0> = {
let mut bits = BitVec::with_capacity(bit_slice.len() + pad);
// Copy bits to new BitVec
bits.extend_from_bitslice(bit_slice);
// Force align
//i.e. [1110, 10010110] -> [11101001, 0110]
bits.force_align();
// Some padding to next byte
let index = if input_is_le {
bits.len() - (8 - pad)
} else {
0
};
for _ in 0..pad {
bits.insert(index, false);
}
// Pad up-to size of type
for _ in 0..(MAX_TYPE_BITS - bits.len()) {
if input_is_le {
bits.push(false);
} else {
bits.insert(0, false);
}
}
bits
};
The issue is specifically in the for _ in 0..pad
loop, changing that to:
for _ in pad..0 {
bits.insert(index, false);
}
Results in my test passing. I'm just going to proceed with my forked version for now, but I will see if I can understand deku
a little bit better and open a PR that adds proper support for Lsb0 parsing.
EDIT: To be clear on what exactly is passing, I have changed all instances of Msb0 to Lsb0 and changed the direction of the bit padding for loop in primitive::ImplDekuReadBits
, then with this struct definition (which exactly matches the C struct definition, so "ideal" in my mind):
#[derive(Debug, Copy, Clone, PartialEq, DekuRead, DekuWrite)]
struct TestPacket {
// Word 1: Bytes 1-4
#[deku(bits = 10)]
f1: u32,
#[deku(bits = 10)]
f2: u32,
#[deku(bits = 10, pad_bits_after = "2")]
f3: u32,
// Bytes 5-6
#[deku(bits = 10, pad_bits_after = "6")]
f4: u16,
f5: u16, // Bytes 7-8
f6: u16, // Bytes 9-10
f7: u16, // Bytes 11-12
}
Everything is parsed as expected.
I don't have time to read your entire message, but the following hack works, although it looks like you have a sort of solution.
use bitvec::view::BitView;
use bitvec::{prelude::Msb0, slice::BitSlice, vec::BitVec};
use deku::prelude::*;
use std::convert::TryFrom;
#[derive(Debug, Copy, Clone, PartialEq, DekuRead, DekuWrite)]
#[deku(endian = "big")]
struct Word1 {
#[deku(bits = 10, pad_bits_before = "2")]
f3: u16,
#[deku(bits = 10)]
f2: u16,
#[deku(bits = 10)]
f1: u16,
}
impl Word1 {
fn read(rest: &BitSlice<u8, Msb0>) -> Result<(&BitSlice<u8, Msb0>, Word1), DekuError> {
let (rest, value) = u32::read(rest, ())?;
let bytes = value.to_be_bytes();
let (_, word) = Word1::from_bytes((&bytes, 0))?;
Ok((rest, word))
}
fn write(output: &mut BitVec<u8, Msb0>, word: &Word1) -> Result<(), DekuError> {
todo!();
}
}
#[derive(Debug, Copy, Clone, PartialEq, DekuRead, DekuWrite)]
#[deku(endian = "little")]
struct TestPacket {
// Word 1: Bytes 1-4
#[deku(
reader = "Word1::read(deku::rest)",
writer = "Word1::write(deku::output, &self.word)"
)]
word: Word1,
// Bytes 5-6
#[deku(bits = 10, pad_bits_after = "6")]
f4: u16,
f5: u16, // Bytes 7-8
f6: u16, // Bytes 9-10
f7: u16, // Bytes 11-12
}
fn main() {
// Test Data as Hex
let test_data = vec![
0x01, 0xDC, 0x88, 0x23, // Word 1: Bytes 1-4
0x73, 0x00, // Bytes 5-6
0x00, 0x00, // Bytes 7-8
0x01, 0x00, // Bytes 9-10
0x01, 0x00,
]; // Bytes 11-12
let test_packet = TestPacket::try_from(test_data.as_ref()).unwrap();
// Word 1: Bytes 1-4 become u32 -> word1 = 0x2388DC01
let expect_packet = TestPacket {
word: Word1 {
f3: 0x238, // (word1 >> 20) & 0x3FF
f2: 0x237, // (word1 >> 10) & 0x3FF
f1: 1, // word1 & 0x3FF
},
f4: 0x73, // 0x0073 & 0x3FF
f5: 0, // 0x0000 & 0xFFFF
f6: 1, // 0x0001 & 0xFFFF
f7: 1, // 0x0001 & 0xFFFF
};
assert_eq!(test_packet, expect_packet);
}
Yeah I definitely don't want to do custom implementations, I have a lot of these types of structs to define and I'd like to cut out boilerplate as much as possible. Switching to LSb-first parsing and fixing one bug that introduces in the ImplDekuReadBits
macro works as I expect, so I'm just going to use my fork for now. I will try to get a proper solution introduced back to the mainline branch when I can.
Yeah I definitely don't want to do custom implementations, I have a lot of these types of structs to define and I'd like to cut out boilerplate as much as possible. Switching to LSb-first parsing and fixing one bug that introduces in the
ImplDekuReadBits
macro works as I expect, so I'm just going to use my fork for now. I will try to get a proper solution introduced back to the mainline branch when I can.
Agree! It would be a great addition.
> thread 'main' panicked at 'assertion failed: `(left == right)`
> left: `TestPacket { f1: 769, f2: 114, f3: 130, f4: 115, f5: 0, f6: 1, f7: 1 }`,
> right: `TestPacket { f1: 1, f2: 567, f3: 568, f4: 115, f5: 0, f6: 1, f7: 1 }`', examples/test.rs:60:5
> note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Hey @sharksforarms, considering that this library is for working with binary data, could the left:
and right:
be represented in hexadecimal format. @wcampbell0x2a has assert_hex, which could help.
is there still interest in moving this forward? I'm currently using @LeonardMH 's deku fork for a parsing project where bits are Lsb0 ordered, and I would love to see this functionality included in the main version of deku. If there's interest, I could try figuring out a PR.
I have an update on that in this issue: https://github.com/sharksforarms/deku/issues/134#issuecomment-1720212050
This has no MR, is based on a couple of un-merged MRs, and doesn't support writing yet.
So far in my limited testing it works for reading!
I have an update on that in this issue: #134 (comment)
This has no MR, is based on a couple of un-merged MRs, and doesn't support writing yet.
So far in my limited testing it works for reading!
That being said, I haven't tested this issues example.
I did see that issue as well, but the changes that your fork introduces are much larger, and I haven't looked at them in too much detail yet :)
I did see that issue as well, but the changes that your fork introduces are much larger, and I haven't looked at them in too much detail yet :)
I pushed the changes to a MR: https://github.com/sharksforarms/deku/pull/367
Hello, I was looking forward to using
deku
and hit my first roadblock when I learned (the hard way) that bitfields are parsed in MSb-first order (as referenced on issue #134). This isn't exactly the issue I am trying to resolve, but I bring it up because:So on to the real problem, I am trying to decode a data stream coming from a microcontroller, the data comes to me directly over the wire as it is represented in memory on the MCU, which is represented by the following (C) bitfield struct:
To simplify the issue, let's look at one failing case, the data I get over the wire is
[0x01, 0xDC, 0x88, 0x23, 0x73, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00]
. I have written a minimal test case in Rust which, other than being a bit over-prescriptive with endianess is pretty close to how I would "ideally" define this struct:This fails with the following:
But ok, that's somewhat expected given that the parsing happens in MSb-first order, I'm just not sure on what scale that MSb-first applies so I tried various combinations of reordering the bitfields, changing the
endian
setting, changing betweenpad_bits_after
andpad_bits_before
, explicitly definingbits
even on the u16 types that don't need it, etc. My testing probably hasn't been exhaustive, but it has been exhausting and at this point I think I have tried everything that "seems like it should work", but I haven't been able to find the magic.Can anyone help me wrap my head around this? Am I just missing something obvious? If nothing else, is it possible to do any debugging of how
deku
is parsing the data sent to it?