Read battery once, after the picture is pushed, not at boot

battery_read_percent() was called once at the very start of boot, before
WiFi even connects, and that value was reused both for the manage-menu
overlay and the server report. Taken right after a reset (e.g. the OTA
reboot that immediately precedes it), the rail may still be settling --
plausible source of noisy jumps in reported battery level.

Now there's a single read, in frame_client_run() right before
report_battery(), after the photo (and manage overlay, if shown) is
already on the panel -- the fetch/display work already done this cycle
is the settle time, no delay to guess. The manage overlay no longer
needs an early local reading at all: it shows the server's last-known
value instead, added to the /frame/photo-info response it already
fetches.

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;

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);

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",

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,
     }