add new battery mappings #253
Conversation
|
Hi |
reg -> regulator, basically what did the board have for power regulation |
|
Ok, thanks. Im not sure if that would need to go into the Code, but if someone needs to make a change, the numbers could be reproduce. (For different circuit) |
im a little confused, i did add a few comments that i thought should explain it all (new) values are derived from logging a battery over time as it discharged if that is what you are asking? |
| @@ -54,6 +54,10 @@ | |||
| #define BATTERY_SHIELD_R2 220.0 | |||
| #endif | |||
|
|
|||
| #ifndef BATTERY_REGULATOR | |||
| #define BATTERY_REGULATOR REG_LDO | |||
There was a problem hiding this comment.
To keep backwards compatibility the fallback should be legacy behavior imo.
| #define BATTERY_REGULATOR REG_LDO | |
| #define BATTERY_REGULATOR REG_LEGACY |
There was a problem hiding this comment.
the legacy mapping is not based on anything, though, what other reason would be to not use the new mapping as default/fallback?
There was a problem hiding this comment.
I mean fair, but then why even keep it around in the first place. At the very least it should be official slime option as default then imo. I'll be setting up a test today.
There was a problem hiding this comment.
official slime uses a buck regulator, which will draw roughly constant power, the discharge characteristic is different than most diy boards which have a linear ldo regulator, which will draw roughly constant current
src/batterymonitor.cpp
Outdated
There was a problem hiding this comment.
Will verify the curves with a test, in theory they look correct.
There was a problem hiding this comment.
looks like i messed up the test a bit so charging the slime and will try again tomorrow
There was a problem hiding this comment.
Did you ever get a chance to test this?
There was a problem hiding this comment.
Didn't get to doing this, had some stuff come up when I was doing it, never finished and forgot about it.
It would be helpful if someone was able to test i think I'm not sure what the regulator is on legacy/dev board but it probably isn't too important, if it has a buck regulator then it can be added then |
I have some time again and I can run a test soon to see if the curve looks good. Wanted to test something else along with this either way. @sctanf could you update the branch and then I'll run some tests this weekend orso on the official hardware. |
|
Ok thank you, will test next weekend 😃 |
|
Wanted to run a test, but this firmware currently doesn't work and crashes the slime. Confirmed working on main so I'm guessing the merge is to blame (there were battery changes on main too for the esp32). Can try to debug later or test again when fixed. |
|
it should be updated with new measurements for official trackers, then keep legacy mapping for now because new one was not tested for diy |
|
I think we could map it to the Board. So if the board is SlimeVR then use the mapping from SlimeVR and else the legacy one? |
measure an actual existing battery and use those values for mapping instead of some random internet graph
maybe this is better?