diff --git a/firmware/main/frame_client.c b/firmware/main/frame_client.c index f43b726..e635090 100644 --- a/firmware/main/frame_client.c +++ b/firmware/main/frame_client.c @@ -20,6 +20,7 @@ #include "combo_button.h" #include "ota_update.h" #include "board_antenna.h" +#include "battery.h" #include "frame_client.h" @@ -313,6 +314,35 @@ static bool json_extract_uint(const char *json, const char *key, uint32_t *out) return true; } +/* Same as json_extract_uint(), but signed -- e.g. battery_percent's -1 + * ("no reading") sentinel. strtoul() would silently wrap a leading '-' + * into a huge unsigned value instead of failing, so this needs its own + * strtol()-based parse rather than reusing json_extract_uint(). */ +static bool json_extract_int(const char *json, const char *key, int *out) +{ + char needle[48]; + snprintf(needle, sizeof(needle), "\"%s\"", key); + const char *pos = strstr(json, needle); + if (pos == NULL) { + return false; + } + pos = strchr(pos, ':'); + if (pos == NULL) { + return false; + } + pos++; + while (*pos == ' ') { + pos++; + } + char *end; + long value = strtol(pos, &end, 10); + if (end == pos) { + return false; + } + *out = (int)value; + return true; +} + /* Finds the string value associated with "key" in a small, flat JSON * blob, e.g. "San Francisco, CA" in {"location": "San Francisco, CA"}. * Same rationale as json_extract_uint() -- not a general parser. Returns @@ -427,20 +457,26 @@ static frame_server_config_t fetch_frame_config(const frame_config_t *cfg) /* GETs the server's /frame/photo-info for the manage-button overlay: * location/taken_at text (left empty if the server didn't have them -- - * e.g. no GPS EXIF to geocode, or no capture date) and a share_url built - * from the returned asset_id, same construction pattern as - * run_fetch_cycle()'s management_url. Any failure (unreachable, no - * current photo, etc.) just leaves all outputs empty -- the caller + * e.g. no GPS EXIF to geocode, or no capture date), a share_url built + * from the returned asset_id (same construction pattern as + * run_fetch_cycle()'s management_url), and the last battery percent this + * frame reported (-1 if none yet). The overlay uses that last-known + * value rather than a fresh local reading -- it's needed before this + * photo is composited and pushed to the panel, i.e. before this cycle's + * own reading (taken later, right before it's reported -- see + * frame_client_run()) even exists yet. Any failure (unreachable, no + * current photo, etc.) just leaves all outputs empty/-1 -- the caller * treats that as "skip these optional overlay regions", not a hard * error, since the base "scan to manage" QR should still show. */ static void fetch_photo_info(const frame_config_t *cfg, char *location_line1, size_t location_line1_size, char *location_line2, size_t location_line2_size, char *taken_at, size_t taken_at_size, - char *share_url, size_t share_url_size) + char *share_url, size_t share_url_size, int *battery_percent) { location_line1[0] = '\0'; location_line2[0] = '\0'; taken_at[0] = '\0'; share_url[0] = '\0'; + *battery_percent = -1; char url[256]; build_url(url, sizeof(url), cfg, "frame/photo-info"); @@ -493,6 +529,7 @@ static void fetch_photo_info(const frame_config_t *cfg, char *location_line1, si json_extract_string(body, "location_line1", location_line1, location_line1_size); json_extract_string(body, "location_line2", location_line2, location_line2_size); json_extract_string(body, "taken_at", taken_at, taken_at_size); + json_extract_int(body, "battery_percent", battery_percent); char asset_id[48]; if (json_extract_string(body, "asset_id", asset_id, sizeof(asset_id))) { @@ -757,8 +794,7 @@ static bool wait_for_button_press(uint32_t timeout_ms) * had that data); level 2 adds named-face labels on top. action only * applies at level 1 -- escalating to level 2 redisplays the same * photo, so it never re-advances/-backs. */ -static esp_err_t show_menu_level(const frame_config_t *cfg, fetch_action_t action, int level, - int battery_percent) +static esp_err_t show_menu_level(const frame_config_t *cfg, fetch_action_t action, int level) { char management_url[256]; build_url(management_url, sizeof(management_url), cfg, ""); @@ -774,8 +810,10 @@ static esp_err_t show_menu_level(const frame_config_t *cfg, fetch_action_t actio * overflow, but a truncated/dropped token still means the resulting * request just 401s with no obvious cause). */ char share_url[320]; + int battery_percent; fetch_photo_info(cfg, location_line1, sizeof(location_line1), location_line2, - sizeof(location_line2), taken_at, sizeof(taken_at), share_url, sizeof(share_url)); + sizeof(location_line2), taken_at, sizeof(taken_at), share_url, sizeof(share_url), + &battery_percent); manage_face_label_t face_labels[MANAGE_FACE_LABELS_MAX]; int face_label_count = 0; @@ -816,10 +854,10 @@ static esp_err_t show_menu_level(const frame_config_t *cfg, fetch_action_t actio * that (escalating, or the final revert) are logged but don't count as * an overall failure -- something was already shown successfully, which * was the point of the button. */ -static esp_err_t run_management_menu(const frame_config_t *cfg, fetch_action_t action, int battery_percent) +static esp_err_t run_management_menu(const frame_config_t *cfg, fetch_action_t action) { int level = 1; - esp_err_t err = show_menu_level(cfg, action, level, battery_percent); + esp_err_t err = show_menu_level(cfg, action, level); if (err != ESP_OK) { ESP_LOGW(TAG, "Could not render management overlay (%s), showing photo normally", esp_err_to_name(err)); return fetch_and_display(cfg, action, NULL); @@ -832,7 +870,7 @@ static esp_err_t run_management_menu(const frame_config_t *cfg, fetch_action_t a break; /* timeout at any level, or a press while already maxed out -- exit */ } level++; - esp_err_t level_err = show_menu_level(cfg, FETCH_NORMAL, level, battery_percent); + esp_err_t level_err = show_menu_level(cfg, FETCH_NORMAL, level); if (level_err != ESP_OK) { ESP_LOGW(TAG, "Could not render menu level %d (%s), reverting", level, esp_err_to_name(level_err)); break; @@ -849,13 +887,12 @@ static esp_err_t run_management_menu(const frame_config_t *cfg, fetch_action_t a /* Runs the appropriate fetch for this cycle: a plain fetch, or -- if * show_management_qr -- the escalating manage menu (see * run_management_menu()). */ -static esp_err_t run_fetch_cycle(const frame_config_t *cfg, fetch_action_t action, bool show_management_qr, - int battery_percent) +static esp_err_t run_fetch_cycle(const frame_config_t *cfg, fetch_action_t action, bool show_management_qr) { if (!show_management_qr) { return fetch_and_display(cfg, action, NULL); } - return run_management_menu(cfg, action, battery_percent); + return run_management_menu(cfg, action); } /* Reports the battery percent to the server (POST /frame/battery). @@ -899,8 +936,7 @@ static void report_battery(const frame_config_t *cfg, int percent) esp_http_client_cleanup(client); } -void frame_client_run(const frame_config_t *cfg, fetch_action_t action, bool show_management_qr, - int battery_percent) +void frame_client_run(const frame_config_t *cfg, fetch_action_t action, bool show_management_qr) { esp_err_t epd_err = epd_init(); bool have_display = (epd_err == ESP_OK); @@ -934,7 +970,7 @@ void frame_client_run(const frame_config_t *cfg, fetch_action_t action, bool sho * worth it to stop false-failing on the common case. */ bool image_ok = true; if (have_display) { - esp_err_t fetch_err = run_fetch_cycle(cfg, action, show_management_qr, battery_percent); + esp_err_t fetch_err = run_fetch_cycle(cfg, action, show_management_qr); image_ok = (fetch_err == ESP_OK); if (!image_ok) { /* epd_display_stream() never triggers a physical refresh on a @@ -969,6 +1005,18 @@ void frame_client_run(const frame_config_t *cfg, fetch_action_t action, bool sho * normal boot, not just the one right after an update). */ esp_ota_mark_app_valid_cancel_rollback(); + /* Read now, not at boot: the photo (and, if shown, the manage + * overlay -- which gets its own battery number from the server's + * last-known value, not a local reading, see fetch_photo_info()) + * is already on the panel, so there's no display deadline to beat. + * Reading here instead of right after waking sidesteps taking the + * ADC sample while the rail's still settling from whatever the + * boot/reset just did, with no need to guess a settle delay -- + * the fetch/display work already done this cycle is the delay. + * Still safe re: the battery/button pin sharing (battery.h) -- + * every button check main.c does happens well before this, at + * the very start of boot. */ + int battery_percent = battery_read_percent(); report_battery(cfg, battery_percent); frame_server_config_t server_cfg = fetch_frame_config(cfg); sleep_seconds = server_cfg.reachable ? server_cfg.refresh_interval_s : CONFIG_FRAME_RETRY_INTERVAL_S; diff --git a/firmware/main/frame_client.h b/firmware/main/frame_client.h index 3f84c1e..2d2e4c8 100644 --- a/firmware/main/frame_client.h +++ b/firmware/main/frame_client.h @@ -36,11 +36,13 @@ esp_err_t frame_wifi_connect_sta(const frame_config_t *cfg); * displayed photo gets a small "scan to manage" QR overlay in the * top-right corner linking to the server's config page, held for 30 * seconds (the device stays awake), then reverted back to the plain - * photo before proceeding to the normal sleep-interval logic. + * photo before proceeding to the normal sleep-interval logic. Its + * battery indicator shows the server's last-known reading, not a fresh + * one -- see fetch_photo_info() in frame_client.c. * - * battery_percent (0-100, or -1 for "no reading" -- see - * battery_read_percent()) is shown on the management menu overlay and - * reported to the server after a successful fetch; -1 skips both. + * Reads the battery (see battery_read_percent()) itself, once, after the + * photo is already on the panel, and reports it to the server on a + * successful fetch; a -1 reading ("no reading" -- on mains, disabled, or + * implausible) skips the report. */ -void frame_client_run(const frame_config_t *cfg, fetch_action_t action, bool show_management_qr, - int battery_percent); +void frame_client_run(const frame_config_t *cfg, fetch_action_t action, bool show_management_qr); diff --git a/firmware/main/main.c b/firmware/main/main.c index 42996bd..faf44e2 100644 --- a/firmware/main/main.c +++ b/firmware/main/main.c @@ -10,7 +10,6 @@ #include "next_button.h" #include "back_button.h" #include "combo_button.h" -#include "battery.h" static const char *TAG = "main"; @@ -52,18 +51,12 @@ void app_main(void) * for "not pressed" (false) or "quick press" (true, show the menu). */ bool show_management_qr = combo_button_check(); - /* Must come after the button checks: the battery pin is (by design, - * on the XIAO board) shared with a button, and the ADC read briefly - * takes the pin over -- see battery.h. -1 = no reading (disabled, - * on mains, or implausible). */ - int battery_percent = battery_read_percent(); - frame_config_t cfg; esp_err_t cfg_err = frame_config_load(&cfg); if (cfg_err == ESP_OK) { ESP_LOGI(TAG, "Found stored config for '%s', connecting to home WiFi", cfg.sta_ssid); if (frame_wifi_connect_sta(&cfg) == ESP_OK) { - frame_client_run(&cfg, action, show_management_qr, battery_percent); + frame_client_run(&cfg, action, show_management_qr); return; /* frame_client_run currently never returns */ } ESP_LOGW(TAG, "Could not connect to stored WiFi after %d attempts, falling back to provisioning", diff --git a/server/app/routers/device.py b/server/app/routers/device.py index c4f2a66..2b57d8b 100644 --- a/server/app/routers/device.py +++ b/server/app/routers/device.py @@ -365,6 +365,13 @@ def frame_photo_info(frame: Frame = Depends(require_device), db: Session = Depen "location_line1": location[0] if location else None, "location_line2": location[1] if location and location[1] else None, "taken_at": _format_taken_at(exif), + # Last value this frame itself reported (see /frame/battery) -- + # not a fresh reading. Good enough for a glance on the manage + # overlay, and lets the device skip a synchronous ADC read (which + # would otherwise need to happen before the overlay is composited, + # i.e. before the photo it's part of is even pushed to the panel) + # just to render this. + "battery_percent": frame.battery_percent, }