nRF52840 Power Management Stage 1 v2.1 - Boot lockout fix#2088
nRF52840 Power Management Stage 1 v2.1 - Boot lockout fix#2088entr0p1 wants to merge 1 commit intomeshcore-dev:devfrom
Conversation
e3c0529 to
d5f6c76
Compare
|
Good job |
Improvements: - Add configurable battery chemistry - Add per-chemistry LPCOMP wake threshold and boot lockout voltage - Add configurable enabled/disabled state for boot lockout - Reduce Li-ion/Li-Po lockout threshold to 3.0V from 3.3V - Initialise FS earlier in the startup process to allow reading configured settings for boot lock Fixes: - Add additional shutdown reason "None" for instances where the reason isn't set - Boot lockout disabled by default
d5f6c76 to
e32e30b
Compare
|
Tested on a couple of boards now without issues so far. Ready to be reviewed. |
|
Does setting pwrmgt.bootlock to off also prevent the low voltage shutdown? or is it only for boot? |
|
I truly understand what @entr0p1 wanted to do here and I respect it, however: I feel this became tad too over-engineered and we need to regroup and make it really simple: |
|
@recrof appreciate the respectful approach you've taken, I'm not fussed if it has to be pared right back or removed entirely. The fix is here if desired, but if you'd rather a different approach I won't be offended if you want to close it and do that. |
100% agree.
This is the most crucial point. #1413 was not ready for merging and still got merged for v1.12. It broke a lot of setups and now three months and three releases later things are still broken. People need to modify their firmware manually and climb on roofs because of this. Can we please either:
Thanks! |
|
When the final decision has been made and code merged and released it will be important to communicate about this in any case. I personally believe we will be seeing more and more LTO and Sodium-ion based repeaters in the future. |
I've implemented this as a quick fix in #2377. After deciding on and implementing a proper solution this can be reenabled. |
|
I've been thinking about this more and based on what @recrof said I remembered we already have an auto shutdown voltage in the companion firmware (I think its in others now too?). Couldn't we basically drop this entire feature out of the codebase and just build on that to make the voltage configurable via the CLI? No chemistries, wake, or lockouts to worry about and the code basically exists and just needs a pref added. Thoughts? |
|
My thoughts after being on the 2088 adventure: |
Improvements:
Fixes: