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

Attempts at more terse fixint opt-ins #96

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pwfff
Copy link

@pwfff pwfff commented Mar 12, 2023

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(),
         }
     }
 }
  1. 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 {
-    packet_id: LE<u32>,
-    controller_count: LE<u32>,
+    packet_id: u32,
+    controller_count: u32,
 }
 
 impl Handler<RequestControllerCountResponse> for RequestControllerCount {
@@ -47,8 +48,8 @@ impl Handler<RequestControllerCountResponse> for RequestControllerCount {
 
     fn handle(&self, controller: &Controller) -> RequestControllerCountResponse {
         RequestControllerCountResponse {
-            packet_id: 0.into(),
-            controller_count: controller.controller_count().into(),
+            packet_id: 0,
+            controller_count: controller.controller_count(),
         }
     }
 }

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 🥲

@netlify
Copy link

netlify bot commented Mar 12, 2023

👷 Deploy Preview for cute-starship-2d9c9b processing.

Name Link
🔨 Latest commit babc5b6
🔍 Latest deploy log https://app.netlify.com/sites/cute-starship-2d9c9b/deploys/640e198e777fac0008cad2f2

@netlify
Copy link

netlify bot commented Mar 12, 2023

Deploy Preview for cute-starship-2d9c9b canceled.

Name Link
🔨 Latest commit babc5b6
🔍 Latest deploy log https://app.netlify.com/sites/cute-starship-2d9c9b/deploys/640e198e777fac0008cad2f2

@pwfff
Copy link
Author

pwfff commented Mar 12, 2023

Oh, and this is re: #95

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

Successfully merging this pull request may close these issues.

1 participant