-
Notifications
You must be signed in to change notification settings - Fork 514
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
Generate system event when low battery condition is detected #1154
Conversation
I will test this when I get a chance. Thanks! |
I added some code to Test output:
You can see that the Low Batt alarm on the Fuel Gauge is being reset properly with SoC goes above the default 10% as well as triggering each time the SoC goes below 10%. I also added some code to to
I think we should add this complete code example to the
|
API tests also should be updated: https://github.com/spark/firmware/blob/develop/user/tests/wiring/api/system.cpp#L151 |
Right, I think we need to add some simple checks to ensure that |
One way might be to require the user to reset the Alert flag with |
Let's proceed with the discussion of this PR. Quick summary:
@technobly Could you summarize your concerns on querying battery status in the firmware here? |
I'm sorry if this belongs elsewhere, but I'm curious; is this feature destined only for Electron firmware, or will it switch on when any MAX1704x fuel gauge is detected; for example, when a Photon is connected to a SparkFun Photon Battery Shield that contains a MAX1704x? It would seem silly to exclude that use-case from this feature (maybe I'm biased - my Photon project uses that shield!) |
@sergeuz here's what I think we could do to protect the system from memory issues, but not overcomplicate things:
I think we should also keep this PR focused on low battery only as much as possible, so 4) would be left to another PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add 1) and 2) from the comments and consider this PR finished.
Added check for battery's charging status to |
static bool wasCharging = false; // Whether the battery was charging last time when this function was called | ||
const uint8_t status = power.getSystemStatus(); | ||
const bool charging = (status >> 4) & 0x03; | ||
if (!charging && wasCharging) { // Check if charging has been stopped |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it's more likely that a low battery event will be generated while the electron is not charging the battery, it seems like this line should be inverted to reset lowBattEventNotified
when the system goes from not charging to charging.
if (charging && !wasCharging) {
. Currently it would require two actions, a transition from !charging to charging, and then back to !charging.
Without this, it will not be possible to generate a low battery event while applying a heavy load under light/normal charging of the battery.
a11615b
to
43e1289
Compare
Updated |
This PR adds
low_battery
system event, which is generated when low battery condition is detected. The event doesn't carry any data.I don't have any battery, nor adjustable power supply unit to test this PR properly, so any help with the testing would be appreciated. Test application can be as simple as the following:
According to current code, default threshold for low battery alarm is set to 10%.
Doneness:
Features
low_battery
system event, which is generated when low battery condition is detected. This is when the battery falls below the SoC threshold (default 10%, max settable 32%). The event can only be generated again if the system goes from a non-charging to charing state after the event is generated. The event doesn't carry any data. [PR #1154]