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 battery handling #107

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pelle-c
Copy link
Contributor

@pelle-c pelle-c commented Aug 25, 2024

The values given by the ADC are not very accurate. The introduced errors pose a risk for over-voltage on the battery, which in worst case can lead to an explosion.

This commit aims to mitigate the above by doing the following:

  • Adding correction values in order for the reported values to match the real ones
  • Adding IIR filter (from Jahnkeanater) to get more smooth readings
  • Adding hysteresis for when to restart charging, to avoid charge/discharge toggling

The values given by the ADC are not very accurate. The introduced errors pose a
risk for over-voltage on the battery, which in worst case can lead to an explosion.

This commit aims to mitigate the above by doing the following:

* Adding correction values in order for the reported values to match the real ones
* Adding IIR filter (from Jahnkeanater) to get more smooth readings
* Adding hysteresis for when to restart charging, to avoid charge/discharge toggling
@pelle-c
Copy link
Contributor Author

pelle-c commented Aug 25, 2024

From other PRs I gather that ADC accuracy is somewhat a hot topic. When I began to look into this, I had +/- 2V fluctuations on voltage readings (mitigated by a capacitor on 3V3). But also after that, the readings could be up to 1V off. Which matters when it comes to charging batteries, avoiding overvoltage.
Is it me having bad ADCs in the pico, or are others also experiencing similar? Or in other words, is this PR meaningful at all?

@karlranseierausrom
Copy link

karlranseierausrom commented Aug 25, 2024

I have not changed the hardware and observe high fluctuations of up to 3 V. This results in random mowing aborts, when the voltage seemd to drop for a very short time below the "battery low level". Therefore I think this PR is definently meaningful. For sure a hardware noise reduction would be better.
Even the rain detector flashes (seen in the gui) often due to noise.
I looked at the values just in the gui. In my opinion it is not a typical noise, but more a repeating wave on the signal. I had no time to look at the values with an oscilloscope / spectrum analyser yet. If I should to that, would be nice if someone could tell me at which points I should measure.

@pelle-c
Copy link
Contributor Author

pelle-c commented Aug 25, 2024

I posted a dump from my scope, measuring the 3V3, in openmower-hardware channel yesterday. 5V was ok.

Would be interesting to know if the correction values suite everyone or if they have to be tailored. Keeping in mind that perfection isn't required, but I guess at least a precision like +/- 0.1V would be nice.

@Jahnkeanater
Copy link

I get about 0.2 volts of noise in the GUI. My biggest issue is the noise tripping the over current protection every single time my mower docks. I constantly have to flash the pico firmware to clear the OCP and get the mower to charge. Once it actually starts charging then it's typically 0.8-1.0 amps. I don't see the point of having 7 digits when we realistically only have about 1 decimal point of usable resolution. I'm curious if adding a capacitor to the ADC would help or if the pico just has really noisy ADCs.

@Jahnkeanater
Copy link

Jahnkeanater commented Aug 28, 2024

I think I remember reading that the pico has an internal switching power supply that makes the ADC really noisy. "Driving high the SMPS mode pin (GPIO23), to force the power supply into PWM mode, can greatly reduce the inherent ripple of the SMPS at light load, and therefore the ripple on the ADC supply."

GP23 on the pico is pad TP4 on the bottom of the board. Also maybe adding a capacitor to the ADC Vref pin might help.

@Jahnkeanater
Copy link

Jahnkeanater commented Aug 28, 2024

I think just setting GPIO23 high in software will put the power supply in PWM mode.

#75 Never mind looks like PWM mode is slightly worse.

Also it seems like olliewalsh was working on a way for the mower to self calibrate when mowing. That seems better than hard coding a correction factor.

@pelle-c
Copy link
Contributor Author

pelle-c commented Aug 28, 2024

I think this needs a bit more investigation.
First, I believe that there is a hardware issue that doesn't apply to everyone, if it's a batch of bad picos or mb 0.13.2, I don't know. But I do know that my 3V3 was really ugly before I put a capacitor on it.
Secondly, agree that that the self calibrating PR (olliewalsh) looks nice (could be bootstrapped with a "good value" as well). Not sure that it's enough though, I had to not only add offset but also multiply to get decent values.
For me, the SMPS didn't make any noticeable difference.
Will do some more testing when I find a moment.

@Jahnkeanater
Copy link

My main board is 0.9.1. What voltage regulator are you using? I replaced the cheap Chinese one with the SMD components.
PXL_20220617_052830938~2

@pelle-c
Copy link
Contributor Author

pelle-c commented Aug 31, 2024

I haven't modified anything on my 0.13.2 mb (apart from adding a capacitor on 3.3V). Believe the pico is generating the 3.3V.
Just tested to put a 2.5V refernce voltage on ADC_VREF, and suddenly the readings were good without corrections. The offset is about 0.1V and it fluctuates very little, maybe +/-0.01V.
Will do some more testing, but this does look promising at least on my mb.

Default factors are now 1.0, and offsets are 0. Meaning no correction.
@pelle-c
Copy link
Contributor Author

pelle-c commented Sep 21, 2024

With the hardware mods below and the filter from Jahnkeanater, the values are good. I trimmed the pot to match a voltmeter when the battery was full and the error was about 0.1V when empty. So, this PR is not needed if the above is in place.
om_v0_13_2_mods

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