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

Improve ADC accuracy #75

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

olliewalsh
Copy link
Contributor

Use the charge current ADC when undocked to determine ADC offset, tweak the resistors values, and disable SMPS powersaving during ADC.

With this I get v_battery pretty close to the measured value

@olliewalsh
Copy link
Contributor Author

Note: I'm using a picow, interested in feedback from someone using a pico as the 3.3v supply is slightly different

Comment on lines 59 to 60
#define VIN_R1 9980.0f
#define VIN_R2 998.0f
Copy link
Owner

Choose a reason for hiding this comment

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

Where do these come from? I assume these will be slightly different for every board (10k 1%, 1k 1%), so we should have them at 10k and 1k

@ClemensElflein
Copy link
Owner

Thank you for the PR! Looks good!

  • Should we use the "raw" voltage for battery overvoltage protection instead of the corrected one? I'm thinking about what happens if the offset is "wrong" (e.g. is at 3V) and breaks overvoltage shutdown. For the overvoltage shutdown the noise and offset don't really matter I think

  • Do you know what the average error is you get? I assume it's <1V. Should we lower the 3V check?

@@ -601,16 +605,32 @@ void loop() {
if (now - last_status_update_millis > STATUS_CYCLETIME) {
updateNeopixel();

// Disable power saving during ADC
digitalWrite(PIN_SMPS_POWERSAVE, HIGH);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In spite of what the datasheets says this is actually worse on my mower and seems to be a general issue with all picos from https://pico-adc.markomo.me/

Use the charge voltage ADC when mowing to determine ADC offset.
@olliewalsh
Copy link
Contributor Author

Thank you for the PR! Looks good!

  • Should we use the "raw" voltage for battery overvoltage protection instead of the corrected one? I'm thinking about what happens if the offset is "wrong" (e.g. is at 3V) and breaks overvoltage shutdown. For the overvoltage shutdown the noise and offset don't really matter I think
  • Do you know what the average error is you get? I assume it's <1V. Should we lower the 3V check?

Replaced the logic to check the high-level mode && GPS to determine that it's mowing, ensures it doesn't try to adjust the offset while docking.
Switched to v_charge instead of charge_current adc as current is affected by regen AFAICT while v_charge is not (behind a diode).
Limited offset to +-3% to ensure it's sane

@pelle-c
Copy link
Contributor

pelle-c commented Sep 1, 2024

I haven't tested this PR in my mower, but I did some measurements to see if it could work since I thought it was a great idea to have self calibration.

My 0.13.2 mb is somewhat modified since I had issues getting stable readings - I now have a capacitor over 3.3V and an "external" voltage reference to ADC_VREF.
When my mower is not connected to the charger, the reported charge voltage is ~0.45V. If I short the related pin to GND, I get ~0.13V.
If I understand this PR correctly, the charge voltage is measured and "averaged" while mowing. This value is then used to offset the measured voltages. In my case, that would mean that the voltage was offset with -0.45 instead of -0.13, which would lead to readings that are ~0.3V off from the real value.

From reading PRs here, and postings on discord, I've got the impression that you have some variations in the ADC errors. Like for me on this one, does the same apply for all boards or is just on 0.13.2, or only mine?
Guess it's hard to tell without gathering info from users.

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.

3 participants