bikeshedder / sunspec

Rust crate for accessing SunSpec compliant devices in a safe and convenient way.
Apache License 2.0
4 stars 3 forks source link

Serde support? #2

Closed jacobsvante closed 7 months ago

jacobsvante commented 7 months ago

Would be nice to be able to serialize the models. E.g. for displaying purposes.

bikeshedder commented 7 months ago

Yes. That makes an awful lot of sense. I'll add a serde feature. :+1:

jacobsvante commented 7 months ago

Sweet! 😀

bikeshedder commented 7 months ago

I just added serialization support and was wondering if I should use the original Sunspec identifiers for the JSON format or stick with the "rustified" names?

Sunspec

{
    "AID": "...",
    "StVnd": "...",
    "Evt": "GROUND_FAULT | TEST_FAILED",
    "Ctl": "AUTOMATIC",
}

Rust

{
    "aid": "...",
    "st_vnd": "...",
    "evt": "GroundFault | TestFailed",
    "ctl": "Automatic",
}

Personally I prefer the rust style as it's much easier on the eyes and it's just the "default" serialization of an already rustified model definition.

Do you have actual plans for using this?

jacobsvante commented 7 months ago

I agree that it's easier on the eyes. But personally I think serializing/deserializing should use the original names.

Right now my only use case is for printing out to the terminal, for inspection.

bikeshedder commented 7 months ago

You know that the models do support the Debug trait? You can just println!("{model:?}") to get it in a human readable form. And with :#? you even get some nice indentation for easier reading.

btw. bitflags really get serialized like that. I did expecting sth. like "evt": ["GroundFault", "TestFailed"] or "evt": {"GroundFault": true, "TestFailed": true } as that string format is much harder to parse. Maybe bitflags is the wrong data type to begin with. The bitfields in the sunspec models never overlap while bitflags would allow that.

jacobsvante commented 7 months ago

Yes I know! I do think serde_json/yaml output looks a tad prettier though ☺️

Regarding bitflags - does that not work in the other direction?

bikeshedder commented 7 months ago

The bitflags format works in both directions. It's just that it's stringly typed inside the JSON and parsing it with a different language will be a bit annoying. As there are some vendor specific things inside those flags I think it's best to just use it like that and accept the default serialization format bitflags uses.

jacobsvante commented 7 months ago

Okay good! I agree with you that it's not optimal. Feels a bit dirty. But since that's bitflags default approach I think it's best to follow that approach.

bikeshedder commented 7 months ago

Renaming struct fields and enum variants was a no-brainer. Though the bitflags don't seam to support renaming.

In the mean time I'll add serialization with "rustified" names. If anyone needs a JSON format with the actual SunSpec identifiers we need to decide how to serialize Enums and Bitfields. The string format is probably not the best to begin with.

jacobsvante commented 7 months ago

Sounds like a good start!

I would love to see a new release with this as well as tokio-modbus 0.11 support!

bikeshedder commented 7 months ago

I just released the sunspec 0.4.0 crate on crates.io:

jacobsvante commented 7 months ago

Sweet. Thank you very much for this and for a thoroughly useful library!