jamesmunns / postcard

A no_std + serde compatible message library for Rust
Apache License 2.0
943 stars 91 forks source link

Attempts at more terse `fixint` opt-ins #96

Open pwfff opened 1 year ago

pwfff commented 1 year ago

Two standalone commits here:

  1. An attempt at just making the field attribute a bit more terse, but I think this is basically just the 'wrapper' y'all had before? Still have to annotate every field, plus now I have to use into() everywhere. Diff for my project looks like this:

    diff --git a/device/src/messaging/packets.rs b/device/src/messaging/packets.rs
    index 0d22758..02c80ab 100644
    --- a/device/src/messaging/packets.rs
    +++ b/device/src/messaging/packets.rs
    @@ -1,4 +1,5 @@
    use arrayref::array_ref;
    +use postcard::fixint::*;
    use serde::{Deserialize, Serialize};
    
    use super::wrapper::MAX_MESSAGE_SIZE;
    @@ -34,10 +35,8 @@ pub struct RequestControllerCount {}
    
    #[derive(Serialize, Deserialize, Debug, Eq, PartialEq)]
    pub struct RequestControllerCountResponse {
    -    #[serde(with = "postcard::fixint::le")]
    -    packet_id: u32,
    -    #[serde(with = "postcard::fixint::le")]
    -    controller_count: u32,
    +    packet_id: LE<u32>,
    +    controller_count: LE<u32>,
    }
    
    impl Handler<RequestControllerCountResponse> for RequestControllerCount {
    @@ -48,8 +47,8 @@ impl Handler<RequestControllerCountResponse> for RequestControllerCount {
    
     fn handle(&self, controller: &Controller) -> RequestControllerCountResponse {
         RequestControllerCountResponse {
    -            packet_id: 0,
    -            controller_count: controller.controller_count(),
    +            packet_id: 0.into(),
    +            controller_count: controller.controller_count().into(),
         }
     }
    }
  2. A macro based on this post I found: https://users.rust-lang.org/t/proc-macro-attribute-to-add-a-serde-deserialize-with-to-every-field-in-a-struct/71231 This seems... fragile. I did find a serde issue/PR where they wanted to do something similar, but it's old, not very active, and had a lot of feedback I didn't quite grok. This does manage to achieve exactly what I was looking for though, so I have a copy in my own codebase that I'm proceeding with. Changes to my project look like this:

    
    diff --git a/device/src/messaging/packets.rs b/device/src/messaging/packets.rs
    index 02c80ab..b7bdeb9 100644
    --- a/device/src/messaging/packets.rs
    +++ b/device/src/messaging/packets.rs
    @@ -1,5 +1,5 @@
    use arrayref::array_ref;
    -use postcard::fixint::*;
    +use corsairust_macros::all_fields_with;
    use serde::{Deserialize, Serialize};
    
    use super::wrapper::MAX_MESSAGE_SIZE;
    @@ -33,10 +33,11 @@ pub trait Handler<R: Serialize> {
    
    pub struct RequestControllerCount {}

+#[all_fields_with("postcard::fixint::le")]

[derive(Serialize, Deserialize, Debug, Eq, PartialEq)]

pub struct RequestControllerCountResponse {

There's probably a better way to make this stuff happen at the library level? Like ideally use postcard::fixint::le::* would just magically configure it for your whole module, but that magic is beyond me still :smiling_face_with_tear:

netlify[bot] commented 1 year ago

Deploy Preview for cute-starship-2d9c9b processing.

Name Link
Latest commit babc5b6291a19ed46c0d7337706a32938580c628
Latest deploy log https://app.netlify.com/sites/cute-starship-2d9c9b/deploys/640e198e777fac0008cad2f2
netlify[bot] commented 1 year ago

Deploy Preview for cute-starship-2d9c9b canceled.

Name Link
Latest commit babc5b6291a19ed46c0d7337706a32938580c628
Latest deploy log https://app.netlify.com/sites/cute-starship-2d9c9b/deploys/640e198e777fac0008cad2f2
pwfff commented 1 year ago

Oh, and this is re: https://github.com/jamesmunns/postcard/issues/95