-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
feat(battery): decouple battery charging status from USB connection state #2153
base: main
Are you sure you want to change the base?
Conversation
tested on real hardware: default display widget (not niceview) existing behaviour seems to work just fine, charging icon appears (immediately) when USB goes in, disappears (immediately) when USB goes out. |
I think it makes sense for RGB and backlighting to use USB power and not charging for their auto off triggers as otherwise the RGB and backlighting might randomly change state whilst the board is plugged in when the battery completes charging |
#endif | ||
|
||
bool is_usb_power_present(void) { | ||
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK) |
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.
Why is this being removed? It can be used in conjunction with charging detection to detect when it's fully charged. If USB power is present but board is not charging then it can be inferred that the board is fully charged
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.
you're right! didn't think of that. I only removed it because nobody was calling it (and it wasn't declared in a header anywhere LOL). now that you mention it, for the usecase i have in mind (charging but not via usb), i also want this distinction.
so i feel like we would want is_charging
and is_externally_powered
statuses? I'll add the latter into battery.c
and declare it in battery.h
so people can see it.
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.
edit: will probably wait for the zephyr 3.5 update and #2154 to be merged before continuing on this
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.
okay, i ended up removing this for a few reasons:
zmk_is_externally_powered
subsumes this functionality, and covers both the battery-charging case and the usb-plugged-in case- nobody was actually calling this, and it wasn't declared in a header anywhere
for detecting fully-charged, you would be able to do zmk_is_externally_powered() && !zmk_battery_is_charging()
. if we move rgb and backlights to use this function, they won't toggle once the battery stops charging since is_usb_powered
will still be true.
66b596b
to
c9c620d
Compare
weirdge, it closed the pr. should be ready for review now. for backlights and RGB stuff and their auto-off-unless-on-usb functionality, it might require a bit more changes (since they subscribe to the usb_conn event), and i'll put that in a separate PR. for now they still use usb detection. |
54fae8a
to
8bee741
Compare
reference/prior work: #1923
this should be the first phase in a set of PRs to decouple the status of battery charging from the device being connected to USB. the end goal is to allow non-USB-based charging to be reflected in ZMK, eg. via (as in #1923) a GPIO pin connected to a charge IC, or (in future) reading from a fuel gauge IC.
this first PR adds the
zmk_battery_is_charging
function, which for now simply falls back to detecting USB. It also updates places that were using USB as a proxy to use this new function instead.the RGB and underglow config flags currently explicitly name USB (eg .
CONFIG_ZMK_BACKLIGHT_AUTO_OFF_USB
), so I've opted not to change them -- at least for now, and they still use USB detection.The display widgets (corne-ish zen, nice_view, generic zmk display) have been updated to use
zmk_battery_is_charging
as well.