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

Replacing postcard with encdec #261

Merged
merged 5 commits into from
Nov 22, 2022
Merged

Conversation

ryan-summers
Copy link
Member

@ryan-summers ryan-summers commented Nov 21, 2022

This PR replaces postcard with encdec. encdec is more suitable for our precise storage requirements.

I tested this with my current Booster and confirmed that my settings were retained. I also updated parameters and verified that they stuck

I have a PR for the encdec changes needed at ryankurte/rust-encdec#2

Copy link
Member

@jordens jordens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't there be a generic impl encode/decode for IntoPrimitive/(try) FromPrimitive?
Otherwise looks good to me.
Would encdec instead of serde have been nicer for minimq?

@ryan-summers
Copy link
Member Author

ryan-summers commented Nov 21, 2022

Can't there be a generic impl encode/decode for IntoPrimitive/(try) FromPrimitive? Otherwise looks good to me.

Yeah, we likely can. I wonder if a blanket impl would be considered a trait impl on foreign types though? Or did you mean adding the blanket impl to encdec directly? That might be more complicated

Would encdec instead of serde have been nicer for minimq?

Maybe (probably)? It's hard to say for sure without trying it. Serde was definitely a bit awkward with minimq and other wire protocols and has a few hacky workarounds to the restrictive serde data model. It would be an interesting investigation to try out the Encode/Decode API instead.

@jordens
Copy link
Member

jordens commented Nov 21, 2022

Sure. It would need to be in encdec or num_enum. I would have thought that generic impl to be simple. Why would it be complicated?

Copy link
Member

@jordens jordens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway. This discussion above can also be resolved independently.

@ryan-summers
Copy link
Member Author

I spawned an issue in ryankurte/rust-encdec#3 for that discussion. It would be nice to have a derive for simple enums for sure.

@ryan-summers ryan-summers merged commit 87f19be into main Nov 22, 2022
@ryan-summers ryan-summers deleted the feature/settings-serialization branch November 22, 2022 10:37
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.

2 participants