hecatia-elegua / bilge

Use bitsized types as if they were a feature of rust.
Apache License 2.0
171 stars 17 forks source link

Add `SerializeBits` derive macro #74

Open pizzart opened 1 year ago

pizzart commented 1 year ago

The normal Serialize macro serializes the struct into a struct with the field "value":

{"flags": {"value":3}}

Feature request: add a derive macro implementing Serialize on the struct and its fields behind a serde feature flag, something like:

#[bitsize(8)]
#[derive(SerializeBits)]
struct Flags {
    paused: u1,
    looping: u1,
    stereo: u1,
    other: u5,
}

so that it serializes to:

{"flags": {"paused": 1, "looping": 1, "stereo": 0, "other": 0}}
pickx commented 1 year ago

from what I can see, this would first require adding serde support to arbitrary-int.

are we looking for Deserialize support here as well? I think we can just deserialize to a temporary struct and then use the Struct::new().

hecatia-elegua commented 1 year ago

@pizzart if you're up to it, you can try building this. We can skip over serde support in arbitrary-int by just saving as the underlying type, or you could add that support as well (should be easy).

widberg commented 1 year ago

I have a rough draft of this at https://github.com/widberg/bilge using https://github.com/danlehmann/arbitrary-int/pull/38. It doesn't play nice with padding and reserved fields yet. I think these fields should not be serialized so I will probably skip them like the generated new implementation. Some cleanup is also in order.

hecatia-elegua commented 1 year ago

@widberg nicely done, feel free to open a draft PR :)

widberg commented 1 year ago

I was waiting for the arbitrary-int pr to go through so I wouldn't have my git repo as a dependency but I'll open a draft pr so it's easier to find and update it later.

dicta commented 7 months ago

The arbitrary-int PR looks like it was merged and released as part of v1.2.7, is there more work that needs to be done to revive this feature? This would be very nice to have, specifically the serialize side of things.

widberg commented 7 months ago

I switched the Cargo.toml dependency from my arbitrary-int git repo to v1.2.7 on crates.io and switched the PR from draft to open. My tests are passing but please test my repo and let me know if there are any problems.

hecatia-elegua commented 7 months ago

@dicta you might know a bit more about serde than me, take a look at the code in #84. Or even try it with your usecase via git repo dependency. I will merge it soon.

dicta commented 7 months ago

I've been testing this today and it's working well so far for my usecase -- which is just using SerializeBits, not the deserialize side.

Features I'm currently exercising are:

Important features I haven't yet looked at so can't comment on status: