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

Track battery % using consumed mAh instead of voltage. #60

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

Conversation

JordanVlieg
Copy link
Contributor

@JordanVlieg JordanVlieg commented Sep 5, 2022

This PR introduces the concept of battery % (SOC) tracking using mAh instead of a voltage table.

The new system tracks SOC using both voltage and mAh in parallel, and only uses mAh if the following conditions are true

  • The user has set a battery capacity > 0 in the settings
  • The SOC as determined by voltage is > 15% (this prevents sudden nosedives at low voltage if the mAh is misconfigured or bugged).

It converts from mAh % remaining to watt hour % remaining using a set formula derived from the chemical properties of LiPo batteries. Namely that the voltage goes from 4.2V->2.8V. I know the maintainers are not a fan of "magic formulas", but this is a very simple square polynomial that is not an approximation, but rather an exact derivation. The only assumption made is that voltage decay is linear (which is obviously not quite true). However since everyone uses different cells, linear decay is the best choice.

Also adds a new (hidden) diagnostics page that displays more internal variables. Useful for developer debugging and visibility.

Known issues:

  • I expect weird SOC jumps near the 15% cutover. Need to add a fuzzy cutover

saveSettings();
});

relay->setBMSSerialOverride(0xFFABCDEF);

relay->setChargeStateMah(Settings->mah_state);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

potentially we may want to remove this setting altogether and replace it with an estimate that comes from the voltage on every startup. This will help prevent tracking drift that may occur if users dont charge to full often enough. However I'm not sure we have voltages available this early, so this should work for now.

Choose a reason for hiding this comment

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

I think this should be fine. If someone hasn't charged their wheel for a bit they can just put it to charge to reset the state no? If you get it from voltage every time there might be some discrepancy built in for those who don't leave it off for too long.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, probably not a huge issue, but its nice to have. I've found that charging on a speed charger causes about a 10-15% drift between what OWIE thinks was regained vs what was actually regained, so this drift can happen fast if you don't fill up completely often.

Choose a reason for hiding this comment

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

That drift has made me wonder if the 0.055 scaling factor used for current is actually correct. Maybe it being slightly off adds up to a noticeable error over time. How did that value get determined?


int BmsRelay::socFromMahUsed() {
float pct = 100 - consumedMahPct();
return round(0.8 * pct + 0.00202 * pct * pct); // Magic formula to go from Ah % to Wh %
Copy link
Owner

Choose a reason for hiding this comment

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

Appreciate the effort but I don't think this is worth it. SOC based on Ah and Wh converge close to 100% and 0% SOC and only diverge by ~3% at the highest, see https://docs.google.com/spreadsheets/d/1WluzeQnXibmWL1CxRknn-5j9r9QVfrluIOjotYvx3uU/edit#gid=0, mAhPercent and mWAhPercent columns.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree its not required, however I personally tried both and the tracking definitely feels smoother and more natural with this in place. Multiplication operations are cheap, I don't think there is harm in having this here. Whats the actual "cost" in keeping this here?

Copy link
Owner

@lolwheel lolwheel left a comment

Choose a reason for hiding this comment

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

A high level comment: Looks like you're asking users to enter their estimate of the battery capacity. We can instead measure this value ourselves.

Do you mind if I build on top of this PR what I initially intended to do?

@JordanVlieg
Copy link
Contributor Author

@lolwheel sorry for the delay in response. Absolutely feel free to build on top of it. Though I would encourage the merging and release of a OWIE 2.0 with this as it is, since this changes nothing for users who don't enter a value, but those who want to beta test can access this feature before we measure on their behalf.

We can then follow up with 2.1 which determines capacity on your behalf.

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.

4 participants