packed_struct.rs icon indicating copy to clipboard operation
packed_struct.rs copied to clipboard

having a field with 12 bits

Open delfick opened this issue 7 years ago • 6 comments

Hi,

I have a struct that looks like

#[derive(PackedStruct, Debug, Copy, Clone, PartialEq, Default)]
#[packed_struct(bit_numbering = "lsb0", size_bytes = "8")]
pub struct Header {
    #[packed_field(bits = "15:0", endian = "lsb")]
    pub size: u16,
    #[packed_field(bits = "27:16", endian = "lsb")]
    pub protocol: u16,
    #[packed_field(bits = "28:28", endian = "lsb")]
    pub addressable: bool,
    #[packed_field(bits = "29:29", endian = "lsb")]
    pub tagged: bool,
    #[packed_field(bits = "31:30", endian = "lsb")]
    _reserved1: ReservedZero<packed_bits::Bits2>,
    #[packed_field(bits = "63:32", endian = "lsb")]
    pub source: u32,
}

The problem though is I get this errors

error[E0277]: the trait bound `packed_struct::types::bits::Bits12: packed_struct::types::bits::BitsFullBytes` is not satisfied
 --> src/header.rs:3:10
  |
3 | #[derive(PackedStruct, Debug, Copy, Clone, PartialEq, Default)]
  |          ^^^^^^^^^^^^ the trait `packed_struct::types::bits::BitsFullBytes` is not implemented for `packed_struct::types::bits::Bits12`
  |
  = note: required because of the requirements on the impl of `packed_struct::PackedStruct<[u8; 2]>` for `packed_struct::types::LsbInteger<u16, packed_struct::types::bits::Bits12, packed_struct::types::Integer<u16, packed_struct::types::bits::Bits12>>`

error[E0277]: the trait bound `packed_struct::types::bits::Bits12: packed_struct::types::bits::BitsFullBytes` is not satisfied
 --> src/header.rs:3:10
  |
3 | #[derive(PackedStruct, Debug, Copy, Clone, PartialEq, Default)]
  |          ^^^^^^^^^^^^ the trait `packed_struct::types::bits::BitsFullBytes` is not implemented for `packed_struct::types::bits::Bits12`
  |
  = note: required because of the requirements on the impl of `packed_struct::PackedStruct<[u8; 2]>` for `packed_struct::types::LsbInteger<u16, packed_struct::types::bits::Bits12, packed_struct::types::Integer<u16, packed_struct::types::bits::Bits12>>`
  = note: required by `packed_struct::PackedStruct::unpack`

error[E0614]: type `packed_struct::types::LsbInteger<u16, packed_struct::types::bits::Bits12, packed_struct::types::Integer<u16, packed_struct::types::bits::Bits12>>` cannot be dereferenced
 --> src/header.rs:3:10
  |
3 | #[derive(PackedStruct, Debug, Copy, Clone, PartialEq, Default)]
  |          ^^^^^^^^^^^^

It seems the problem is because the second two bytes are split over 4 fields (12 bits, 1 bit, 1 bit, 2 bits)

is it possible at all to have a PackedStruct with BitsPartialBytes fields ?

Thanks

delfick avatar Jun 11 '18 09:06 delfick

LSB integers with arbitrary bit widths are currently not supported as I haven't really encountered them in my usages. The first thing to precisely define and document would be the per-bit layout for such fields. If you can share your expectations of how this should work based on real-world protocols or devices, that would be of great help.

This is properly implemented for MSB integer fields.

rudib avatar Jun 13 '18 09:06 rudib

Fair enough.

I'm implementing the LIFX binary protocol in rust as a learning exercise for rust (already made this library in python). In this case the 12 bits for the protocol field is just a normal 16 bit field with the last 4 bits stolen by the next three fields.

Though I did get a bit further than in my initial example by making the whole struct msb0 and the protocol field look like

    #[packed_field(bits = "16:27", endian = "lsb")]
    pub protocol: Integer<u16, packed_bits::Bits16>,

Though if I then do something like

        let header = super::Header {
            size: 36,
            protocol: 1024.into(),
            addressable: true,
            tagged: true,
            source: 0,
            ..super::Header::default()
        };
        let packd = header.pack();
        let s: Vec<String> = packd.iter().map(|n| format!("{:08b}", n)).collect();
        println!("{:?}", s.join(""));

I get

0010010000000000000000000100110000000000000000000000000000000000 instead of 0010010000000000000000000010110000000000000000000000000000000000

Which looks like the bit for the 1024 is in the wrong place.

So I did something like

        let p: Integer<u16, packed_bits::Bits16> = 1024.into();
        let s: Vec<String> = p.to_lsb_bytes()
            .iter()
            .map(|n| format!("{:08b}", n))
            .collect();
        println!("{:?}", s.join(""));

and I get 0000000000000100 instead of 0000000000100000 and I'm not sure why.

delfick avatar Jun 13 '18 10:06 delfick

I have been arguing about the bit layout of LSB structures, too in #36. In my opinion the bit numbering in lsb0 mode is completely counterintuitive and for example in the example below effectively prevents to specify such a layout. Normally, in little endian architectures/protocols and nobody really cares about lsb0/msb0 bit numbering, always assumes the following one

  • byte 0 denotes bits 7:0,
  • byte 1 denotes bits 15:8 (marking the indices from left to right).

However, packed_struct marks bits are numbered in a weird order ((endian="lsb" and bit_numbering="lsb0")

  • byte 0 denotes bits 15:8
  • byte 1 denotes bits 7:0

The only intuitive mode that kinda matches what we need is (endian="msb" and bit_numbering="lsb0"), however, than you have to reverse the byte order of the sequence generated by pack() call. This is fine if your structure is top level. However nesting such a structure with another 'packed_struct' prevents this usage.

A good example is below, it's a real world example from the new bitcoin mining protocol (Stratum V2). It is technically impossible to use packed_struct for this structure in case it will be nested. There is no way to describe an LSB ordering that occupies bits 14:0 for extension field and bit 15 for channel_msg flag. In packed_struct terms these would be bits [6:0,15:8] for the extension and bit 7 for the flag. As you see this is a complete mess

/// Example
/// value = ExtensionType {
///   extension: 0x1234.into()
///   channel_msg: true
/// }
/// The resulting byte stream should be [0x34, 0x82]

#[derive(PackedStruct, PartialEq, Debug)]
// This bit numbering configuration doesn't yield the desired result
#[packed_struct(bit_numbering = "lsb0", size_bytes = "2", endian = "lsb")]
pub struct ExtensionType {
    #[packed_field(bits = "14:0")]
    pub extension: Integer<u16, packed_bits::Bits16>,
    #[packed_field(bits = "15")]
    pub channel_msg: bool,
}

#[test]
fn test_myextension_type() {
    let a = ExtensionType {
        extension: 0x1234.into(),
        channel_msg: true
    };

    let packed = a.pack();
    assert_eq!(&[0x34, 0x82], &packed, "mismatch received: {}", a);

    let unpacked = ExtensionType::unpack(&packed).unwrap();
    assert_eq!(a, unpacked);
}

@rudib Would the above specification be enough to implement this?

janbraiins avatar Nov 28 '19 21:11 janbraiins

Also trying to implement the Lifx protocol, did you get any further @delfick ?

TimDeve avatar Feb 23 '20 18:02 TimDeve

nah, I had better things to work on :)

delfick avatar Feb 24 '20 01:02 delfick

Here's an example using c2rust_bitfields if you're interested.

TimDeve avatar Feb 24 '20 12:02 TimDeve