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

INA226 JSON do not output leading spaces #1714

Merged
merged 2 commits into from
Jul 31, 2023
Merged

INA226 JSON do not output leading spaces #1714

merged 2 commits into from
Jul 31, 2023

Conversation

NorbertHD
Copy link
Contributor

Description:

Closes #1691

Checklist:

  • The pull request is done against the latest development branch
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • I accept the DCO.

@NorbertHD NorbertHD changed the title INA226 JSON do not output trailing spaces INA226 JSON do not output leading spaces Jul 15, 2023
@1technophile
Copy link
Owner

I think the code could be simplified to have:

    INA226data["volt"] = (float)volt;
    INA226data["current"] = (float)current;
    INA226data["power"] = (float)power;

And get rid of the dtostrf

@NorbertHD
Copy link
Contributor Author

  1. Why cast to float? volt, current and power are already of type float.
  2. With dtostrf you have exactly 2 decimal places. But with your proposal, you have rounding artifacts.
    e.g. 0.28 gets 0.280000001 or 3.43 gets 3.430000067 in the json data.

So how should we go on?

@1technophile
Copy link
Owner

  1. The casting was a way to specify ArduinoJson the type, but it may not be necessary
    https://arduinojson.org/v6/api/config/use_double/

  2. is it the responsibility of OMG to choose the number of decimals to display rather than the controller? Still trying to figure out this one. As it is use cases related, we could stick with the behavior of ZsensorHTU21, ZsensorLM75, ZsensorSHTC3, ZsensorDHT not rounding or round it like in ZsensorRN8209
    Open to discussion on this

@NorbertHD
Copy link
Contributor Author

NorbertHD commented Jul 31, 2023

I have updated the PR, I think not rounding is the correct way. Data should be transported and stored in full precision and only rounded when displaying.

@1technophile
Copy link
Owner

Thanks

@1technophile 1technophile merged commit 4a6838a into 1technophile:development Jul 31, 2023
71 checks passed
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.

INA226 incorrect JSON output
2 participants