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

having a field with 12 bits #29

Open
delfick opened this issue Jun 11, 2018 · 6 comments
Open

having a field with 12 bits #29

delfick opened this issue Jun 11, 2018 · 6 comments

Comments

@delfick
Copy link

delfick commented Jun 11, 2018

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

@rudib
Copy link
Member

rudib commented Jun 13, 2018

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.

@delfick
Copy link
Author

delfick commented Jun 13, 2018

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.

@janbraiins
Copy link

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?

@TimDeve
Copy link

TimDeve commented Feb 23, 2020

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

@delfick
Copy link
Author

delfick commented Feb 24, 2020

nah, I had better things to work on :)

@TimDeve
Copy link

TimDeve commented Feb 24, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants