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

Avoid f32 fields for representing quantities like money and meter values #109

Open
huntc opened this issue Nov 1, 2024 · 4 comments · May be fixed by #110
Open

Avoid f32 fields for representing quantities like money and meter values #109

huntc opened this issue Nov 1, 2024 · 4 comments · May be fixed by #110

Comments

@huntc
Copy link

huntc commented Nov 1, 2024

Unfortunately, the interpretation of the JSON schema number type here is f32. This is not correct due to the potential loss of fractional data. This becomes very important with SampledValueType and its value field, which represents a metering value. Inaccurate conveyancing of meter values can lead to incorrect billing calculations.

Section 2.1.3 of the OCPP v2.0.1 spec, Primitive Datatypes, declares the following:

For data being reported by the Charging Station, the full resolution of the source data must be preserved. The decimal sent towards the Charging Station SHALL NOT have more than six decimal places.

I'd suggest replacing most occurences of f32 with Decimal per https://crates.io/crates/fpdec.

I'm happy to raise a PR if there's agreement here.

@huntc
Copy link
Author

huntc commented Nov 1, 2024

Also, here is our own, perhaps more constrained fixed-point library: https://github.com/cuprous-au/fixed-point. We use this on an EV charging system we developed for a customer.

@tommymalmqvist
Copy link
Member

tommymalmqvist commented Nov 1, 2024

Hello again @huntc - Great find and I'd gladly take a PR.

About which lib to use, I'm not sure. https://crates.io/crates/rust_decimal Seems to be very used in this space.

I could not find a crate for fixed-point, it is not published yet?

fpdec look promising but it's not used as much as rust_decimal from what I can see in the download section.

@huntc
Copy link
Author

huntc commented Nov 1, 2024

Yeah, we’ve not published fixed-point. I like the recommendation you came up with though. I’ll create a PR for us over the weekend. Cheers.

@huntc huntc linked a pull request Nov 2, 2024 that will close this issue
@huntc
Copy link
Author

huntc commented Nov 2, 2024

You can now see a PR for this - I applied the PR to my own code base and it required very little change.

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 a pull request may close this issue.

2 participants