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

Add SerializeBits derive macro #74

Open
pizzart opened this issue Aug 13, 2023 · 9 comments
Open

Add SerializeBits derive macro #74

pizzart opened this issue Aug 13, 2023 · 9 comments

Comments

@pizzart
Copy link

pizzart commented Aug 13, 2023

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
Copy link
Collaborator

pickx commented Aug 14, 2023

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
Copy link
Owner

@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
Copy link
Contributor

widberg commented Sep 25, 2023

I have a rough draft of this at https://github.com/widberg/bilge using danlehmann/arbitrary-int#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
Copy link
Owner

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

@widberg
Copy link
Contributor

widberg commented Oct 16, 2023

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
Copy link

dicta commented Mar 27, 2024

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
Copy link
Contributor

widberg commented Mar 27, 2024

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
Copy link
Owner

@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
Copy link

dicta commented Mar 28, 2024

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:

  • interop with csv and serde_json crates, ensuring that field names are handled
  • bitfields containing a mix of arbitrary-int and standard u8/u16/etc types
  • enums (the error path indicating to use Serialize instead of SerializeBits for enums is nice!)
  • structs containing multiple bitfield members, some of which derive SerializeBits and others do not
  • single reserved member

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

  • DeserializeBits (at all)
  • #[bitsize(N)] where N is an arbitrary-int type, not a standard-width integer
  • multiple reserved members in a single bitfield

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

5 participants