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.

Sign up for GitHub

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

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

feat(battery): decouple battery charging status from USB connection state #2153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5a82ebe
(re)implement charging state support
zhiayang Feb 23, 2024
5a8d573
update corneish-zen display widget
zhiayang Feb 23, 2024
6a3463e
update nice_view and default battery widgets
zhiayang Feb 23, 2024
8bee741
add zmk_is_externally_powered
zhiayang Feb 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 8 additions & 15 deletions app/boards/arm/corneish_zen/widgets/battery_status.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ static sys_slist_t widgets = SYS_SLIST_STATIC_INIT(&widgets);

struct battery_status_state {
uint8_t level;
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
bool usb_present;
#endif
bool is_charging;
};

LV_IMG_DECLARE(batt_100);
Expand All @@ -45,17 +43,17 @@ static void set_battery_symbol(lv_obj_t *icon, struct battery_status_state state

#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
if (level > 95) {
lv_img_set_src(icon, state.usb_present ? &batt_100_chg : &batt_100);
lv_img_set_src(icon, state.is_charging ? &batt_100_chg : &batt_100);
} else if (level > 74) {
lv_img_set_src(icon, state.usb_present ? &batt_75_chg : &batt_75);
lv_img_set_src(icon, state.is_charging ? &batt_75_chg : &batt_75);
} else if (level > 49) {
lv_img_set_src(icon, state.usb_present ? &batt_50_chg : &batt_50);
lv_img_set_src(icon, state.is_charging ? &batt_50_chg : &batt_50);
} else if (level > 24) {
lv_img_set_src(icon, state.usb_present ? &batt_25_chg : &batt_25);
lv_img_set_src(icon, state.is_charging ? &batt_25_chg : &batt_25);
} else if (level > 5) {
lv_img_set_src(icon, state.usb_present ? &batt_5_chg : &batt_5);
lv_img_set_src(icon, state.is_charging ? &batt_5_chg : &batt_5);
} else {
lv_img_set_src(icon, state.usb_present ? &batt_0_chg : &batt_0);
lv_img_set_src(icon, state.is_charging ? &batt_0_chg : &batt_0);
}
#endif /* IS_ENABLED(CONFIG_USB_DEVICE_STACK) */
}
Expand All @@ -70,19 +68,14 @@ static struct battery_status_state battery_status_get_state(const zmk_event_t *e

return (struct battery_status_state){
.level = (ev != NULL) ? ev->state_of_charge : zmk_battery_state_of_charge(),
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
.usb_present = zmk_usb_is_powered(),
#endif /* IS_ENABLED(CONFIG_USB_DEVICE_STACK) */
.is_charging = (ev != NULL) ? ev->is_charging : zmk_battery_is_charging(),
};
}

ZMK_DISPLAY_WIDGET_LISTENER(widget_battery_status, struct battery_status_state,
battery_status_update_cb, battery_status_get_state)

ZMK_SUBSCRIPTION(widget_battery_status, zmk_battery_state_changed);
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
ZMK_SUBSCRIPTION(widget_battery_status, zmk_usb_conn_state_changed);
#endif /* IS_ENABLED(CONFIG_USB_DEVICE_STACK) */

int zmk_widget_battery_status_init(struct zmk_widget_battery_status *widget, lv_obj_t *parent) {
widget->obj = lv_img_create(parent);
Expand Down
16 changes: 5 additions & 11 deletions app/boards/shields/nice_view/widgets/peripheral_status.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,7 @@ static void draw_top(lv_obj_t *widget, lv_color_t cbuf[], const struct status_st

static void set_battery_status(struct zmk_widget_status *widget,
struct battery_status_state state) {
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
widget->state.charging = state.usb_present;
#endif /* IS_ENABLED(CONFIG_USB_DEVICE_STACK) */

widget->state.charging = state.is_charging;
widget->state.battery = state.level;

draw_top(widget->obj, widget->cbuf, &widget->state);
Expand All @@ -71,21 +68,18 @@ static void battery_status_update_cb(struct battery_status_state state) {
}

static struct battery_status_state battery_status_get_state(const zmk_event_t *eh) {
const struct zmk_battery_state_changed *ev = as_zmk_battery_state_changed(eh);

return (struct battery_status_state){
.level = zmk_battery_state_of_charge(),
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
.usb_present = zmk_usb_is_powered(),
#endif /* IS_ENABLED(CONFIG_USB_DEVICE_STACK) */
.level = (ev != NULL) ? ev->state_of_charge : zmk_battery_state_of_charge(),
.is_charging = (ev != NULL) ? ev->is_charging : zmk_battery_is_charging(),
};
}

ZMK_DISPLAY_WIDGET_LISTENER(widget_battery_status, struct battery_status_state,
battery_status_update_cb, battery_status_get_state)

ZMK_SUBSCRIPTION(widget_battery_status, zmk_battery_state_changed);
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
ZMK_SUBSCRIPTION(widget_battery_status, zmk_usb_conn_state_changed);
#endif /* IS_ENABLED(CONFIG_USB_DEVICE_STACK) */

static struct peripheral_status_state get_state(const zmk_event_t *_eh) {
return (struct peripheral_status_state){.connected = zmk_split_bt_peripheral_is_connected()};
Expand Down
12 changes: 2 additions & 10 deletions app/boards/shields/nice_view/widgets/status.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,7 @@ static void draw_bottom(lv_obj_t *widget, lv_color_t cbuf[], const struct status

static void set_battery_status(struct zmk_widget_status *widget,
struct battery_status_state state) {
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
widget->state.charging = state.usb_present;
#endif /* IS_ENABLED(CONFIG_USB_DEVICE_STACK) */

widget->state.charging = state.is_charging;
widget->state.battery = state.level;

draw_top(widget->obj, widget->cbuf, &widget->state);
Expand All @@ -214,19 +211,14 @@ static struct battery_status_state battery_status_get_state(const zmk_event_t *e

return (struct battery_status_state){
.level = (ev != NULL) ? ev->state_of_charge : zmk_battery_state_of_charge(),
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
.usb_present = zmk_usb_is_powered(),
#endif /* IS_ENABLED(CONFIG_USB_DEVICE_STACK) */
.is_charging = (ev != NULL) ? ev->is_charging : zmk_battery_is_charging(),
};
}

ZMK_DISPLAY_WIDGET_LISTENER(widget_battery_status, struct battery_status_state,
battery_status_update_cb, battery_status_get_state)

ZMK_SUBSCRIPTION(widget_battery_status, zmk_battery_state_changed);
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
ZMK_SUBSCRIPTION(widget_battery_status, zmk_usb_conn_state_changed);
#endif /* IS_ENABLED(CONFIG_USB_DEVICE_STACK) */

static void set_output_status(struct zmk_widget_status *widget,
const struct output_status_state *state) {
Expand Down
4 changes: 1 addition & 3 deletions app/boards/shields/nice_view/widgets/util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ struct status_state {

struct battery_status_state {
uint8_t level;
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
bool usb_present;
#endif
bool is_charging;
};

void rotate_canvas(lv_obj_t *canvas, lv_color_t cbuf[]);
Expand Down
2 changes: 2 additions & 0 deletions app/include/zmk/battery.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,5 @@
#pragma once

uint8_t zmk_battery_state_of_charge(void);
bool zmk_battery_is_charging(void);
bool zmk_is_externally_powered(void);
4 changes: 3 additions & 1 deletion app/include/zmk/events/battery_state_changed.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
struct zmk_battery_state_changed {
// TODO: Other battery channels
uint8_t state_of_charge;
bool is_charging;
};

ZMK_EVENT_DECLARE(zmk_battery_state_changed);
Expand All @@ -20,6 +21,7 @@ struct zmk_peripheral_battery_state_changed {
uint8_t source;
// TODO: Other battery channels
uint8_t state_of_charge;
bool is_charging;
};

ZMK_EVENT_DECLARE(zmk_peripheral_battery_state_changed);
ZMK_EVENT_DECLARE(zmk_peripheral_battery_state_changed);
12 changes: 2 additions & 10 deletions app/src/activity.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);
#include <zmk/events/sensor_event.h>

#include <zmk/pm.h>

#include <zmk/battery.h>
#include <zmk/activity.h>

#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
Expand All @@ -30,14 +30,6 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);
#include <zephyr/input/input.h>
#endif

bool is_usb_power_present(void) {
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
Copy link
Contributor

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

Copy link
Contributor Author

@zhiayang zhiayang Feb 9, 2024 •
edited
Loading

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.

Copy link
Contributor Author

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

Copy link
Contributor Author

@zhiayang zhiayang Feb 23, 2024 •
edited
Loading

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:

  1. zmk_is_externally_powered subsumes this functionality, and covers both the battery-charging case and the usb-plugged-in case
  2. 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.

return zmk_usb_is_powered();
#else
return false;
#endif /* IS_ENABLED(CONFIG_USB_DEVICE_STACK) */
}

static enum zmk_activity_state activity_state;

static uint32_t activity_last_uptime;
Expand Down Expand Up @@ -75,7 +67,7 @@ void activity_work_handler(struct k_work *work) {
int32_t current = k_uptime_get();
int32_t inactive_time = current - activity_last_uptime;
#if IS_ENABLED(CONFIG_ZMK_SLEEP)
if (inactive_time > MAX_SLEEP_MS && !is_usb_power_present()) {
if (inactive_time > MAX_SLEEP_MS && !zmk_is_externally_powered()) {
// Put devices in suspend power mode before sleeping
set_state(ZMK_ACTIVITY_SLEEP);

Expand Down
40 changes: 37 additions & 3 deletions app/src/battery.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,23 @@ LOG_MODULE_DECLARE(zmk, CONFIG_ZMK_LOG_LEVEL);
#include <zmk/battery.h>
#include <zmk/events/battery_state_changed.h>
#include <zmk/events/activity_state_changed.h>
#include <zmk/events/usb_conn_state_changed.h>
#include <zmk/activity.h>
#include <zmk/workqueue.h>
#include <zmk/usb.h>

static uint8_t last_state_of_charge = 0;
static bool last_battery_is_charging = false;

uint8_t zmk_battery_state_of_charge(void) { return last_state_of_charge; }
bool zmk_battery_is_charging(void) { return last_battery_is_charging; }
bool zmk_is_externally_powered(void) {
bool ret = zmk_battery_is_charging();
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
ret |= zmk_usb_is_powered();
#endif
return ret;
}

#if DT_HAS_CHOSEN(zmk_battery)
static const struct device *const battery = DEVICE_DT_GET(DT_CHOSEN(zmk_battery));
Expand Down Expand Up @@ -91,8 +102,19 @@ static int zmk_battery_update(const struct device *battery) {
#error "Not a supported reporting fetch mode"
#endif

if (last_state_of_charge != state_of_charge.val1) {
// TODO: for now, battery charging is determined solely by USB being plugged in
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
const bool batt_is_charging = zmk_usb_is_powered();
#else
const bool batt_is_charging = false;
#endif

if (last_state_of_charge != state_of_charge.val1 ||
last_battery_is_charging != batt_is_charging) {

last_state_of_charge = state_of_charge.val1;
last_battery_is_charging = batt_is_charging;

#if IS_ENABLED(CONFIG_BT_BAS)
LOG_DBG("Setting BAS GATT battery level to %d.", last_state_of_charge);

Expand All @@ -103,8 +125,10 @@ static int zmk_battery_update(const struct device *battery) {
return rc;
}
#endif
rc = raise_zmk_battery_state_changed(
(struct zmk_battery_state_changed){.state_of_charge = last_state_of_charge});
rc = raise_zmk_battery_state_changed((struct zmk_battery_state_changed){
.state_of_charge = last_state_of_charge,
.is_charging = batt_is_charging,
});
}

return rc;
Expand Down Expand Up @@ -167,11 +191,21 @@ static int battery_event_listener(const zmk_event_t *eh) {
break;
}
}
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
else if (as_zmk_usb_conn_state_changed(eh)) {
// update the battery on the workqueue if usb connection changed.
k_work_submit_to_queue(zmk_workqueue_lowprio_work_q(), &battery_work);
}
#endif
return -ENOTSUP;
}

ZMK_LISTENER(battery, battery_event_listener);

ZMK_SUBSCRIPTION(battery, zmk_activity_state_changed);

#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
ZMK_SUBSCRIPTION(battery, zmk_usb_conn_state_changed);
#endif

SYS_INIT(zmk_battery_init, APPLICATION, CONFIG_APPLICATION_INIT_PRIORITY);
16 changes: 3 additions & 13 deletions app/src/display/widgets/battery_status.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,16 @@ static sys_slist_t widgets = SYS_SLIST_STATIC_INIT(&widgets);

struct battery_status_state {
uint8_t level;
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
bool usb_present;
#endif
bool is_charging;
};

static void set_battery_symbol(lv_obj_t *label, struct battery_status_state state) {
char text[9] = {};

uint8_t level = state.level;

#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
if (state.usb_present) {
if (state.is_charging) {
strcpy(text, LV_SYMBOL_CHARGE " ");
}
#endif /* IS_ENABLED(CONFIG_USB_DEVICE_STACK) */

#if IS_ENABLED(CONFIG_ZMK_WIDGET_BATTERY_STATUS_SHOW_PERCENTAGE)
char perc[5] = {};
Expand Down Expand Up @@ -67,19 +62,14 @@ static struct battery_status_state battery_status_get_state(const zmk_event_t *e

return (struct battery_status_state){
.level = (ev != NULL) ? ev->state_of_charge : zmk_battery_state_of_charge(),
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
.usb_present = zmk_usb_is_powered(),
#endif /* IS_ENABLED(CONFIG_USB_DEVICE_STACK) */
.is_charging = (ev != NULL) ? ev->is_charging : zmk_battery_is_charging(),
};
}

ZMK_DISPLAY_WIDGET_LISTENER(widget_battery_status, struct battery_status_state,
battery_status_update_cb, battery_status_get_state)

ZMK_SUBSCRIPTION(widget_battery_status, zmk_battery_state_changed);
#if IS_ENABLED(CONFIG_USB_DEVICE_STACK)
ZMK_SUBSCRIPTION(widget_battery_status, zmk_usb_conn_state_changed);
#endif /* IS_ENABLED(CONFIG_USB_DEVICE_STACK) */

int zmk_widget_battery_status_init(struct zmk_widget_battery_status *widget, lv_obj_t *parent) {
widget->obj = lv_label_create(parent);
Expand Down
Loading