Fix status screen logic: first-connection screen, false-negative skip

Two bugs found testing against a real (partially-configured) server: - The reachability check hit HEAD / with -- our server only registers GET on that route, so it always got a 405. Harmless for the check itself (any completed HTTP response counts as "reachable"), but noisy and semantically wrong. Points at GET /health instead, which exists for exactly this. - fetch_and_display() failing before any pixel data was sent (e.g. a 400/404 on /frame/image) was treated the same as a mid-stream failure, which skips drawing a status screen to avoid compounding flashing on top of an already-refreshed panel. But a pre-stream failure never touches the panel at all, so skipping the status screen there just left the old provisioning QR code on screen with no indication anything had gone wrong. fetch_and_display() now reports whether streaming ever started so the caller can tell the two cases apart. - The status screen now always shows on the very first successful connection after (re)provisioning, regardless of outcome, via a new "connected_once" NVS flag that frame_config_save() resets on every fresh provisioning event. Later wakes skip it on success (straight to the photo) but still show it on any failure, matching the intent from the original status-screen feature.
2026-07-18 15:06:50 -04:00
parent 7008233320
commit 878909b302
3 changed files with 92 additions and 18 deletions
@@ -111,17 +111,16 @@ esp_err_t frame_wifi_connect_sta(const frame_config_t *cfg)
    return result;
 }

-/* Probes cfg->toolsserver with a plain HTTP HEAD -- any HTTP response
- * (even a 404, since there's no server yet to define real routes) means
- * the socket-level connection succeeded, which is all this is checking. */
+/* Probes the server's /health endpoint -- any completed HTTP response
+ * means the socket-level connection succeeded, which is all this checks. */
 static bool check_server_reachable(const char *toolsserver)
 {
    char url[160];
-    snprintf(url, sizeof(url), "http://%s/", toolsserver);
+    snprintf(url, sizeof(url), "http://%s/health", toolsserver);

    esp_http_client_config_t config = {
        .url = url,
-        .method = HTTP_METHOD_HEAD,
+        .method = HTTP_METHOD_GET,
        .timeout_ms = CONFIG_FRAME_SERVER_CHECK_TIMEOUT_MS,
    };
    esp_http_client_handle_t client = esp_http_client_init(&config);
@@ -148,9 +147,14 @@ static size_t http_read_fn(uint8_t *chunk, size_t chunk_size, void *ctx_)
    return n > 0 ? (size_t)n : 0;
 }

-/* GETs /frame/image and streams the response directly into the panel. */
-static esp_err_t fetch_and_display(const frame_config_t *cfg)
+/* GETs /frame/image and streams the response directly into the panel.
+ * *out_started_stream tells the caller whether the panel was actually
+ * touched -- false means the failure happened before any pixel data was
+ * sent (bad status, connection error), so nothing was drawn this cycle. */
+static esp_err_t fetch_and_display(const frame_config_t *cfg, bool *out_started_stream)
 {
+    *out_started_stream = false;
+
    char url[160];
    snprintf(url, sizeof(url), "http://%s/frame/image", cfg->toolsserver);

@@ -178,6 +182,7 @@ static esp_err_t fetch_and_display(const frame_config_t *cfg)
    }
    ESP_LOGI(TAG, "Fetching frame (%d bytes) from '%s'", content_length, url);

+    *out_started_stream = true;
    http_read_ctx_t ctx = { .client = client };
    err = epd_display_stream(http_read_fn, &ctx);

@@ -195,25 +200,50 @@ void frame_client_run(const frame_config_t *cfg)
        ESP_LOGW(TAG, "EPD init failed (%s), continuing without display", esp_err_to_name(epd_err));
    }

+    /* Always show the status screen on the first successful connection
+     * after (re)provisioning, regardless of outcome -- confirms the
+     * connection worked. Skipped on later wakes to save a refresh, except
+     * when something's actually wrong (handled below). */
+    bool first_connection = !frame_config_has_connected_once();
+    if (first_connection) {
+        frame_config_mark_connected_once();
+        if (have_display) {
+            status_screen_show(cfg->sta_ssid, STATUS_OK, cfg->toolsserver, STATUS_PENDING);
+        }
+    }
+
    uint32_t sleep_seconds = CONFIG_FRAME_SLEEP_INTERVAL_S;
    bool server_reachable = check_server_reachable(cfg->toolsserver);

    if (!server_reachable) {
-        /* Nothing's been drawn yet this cycle, so this is the only
-         * refresh -- cheap diagnostics without compounding flashing. */
+        /* Nothing's been drawn yet this cycle (aside from the optional
+         * PENDING screen above), so this is cheap diagnostics without
+         * compounding flashing. */
        ESP_LOGW(TAG, "Tools server not reachable, retrying sooner");
        sleep_seconds = CONFIG_FRAME_RETRY_INTERVAL_S;
        if (have_display) {
            status_screen_show(cfg->sta_ssid, STATUS_OK, cfg->toolsserver, STATUS_FAILED);
        }
-    } else if (have_display) {
-        esp_err_t fetch_err = fetch_and_display(cfg);
-        if (fetch_err != ESP_OK) {
-            /* The panel may have already partially refreshed with a
-             * truncated frame by this point -- log and retry sooner
-             * rather than drawing a second status screen on top of it. */
-            ESP_LOGW(TAG, "Fetch/display failed (%s), retrying sooner", esp_err_to_name(fetch_err));
-            sleep_seconds = CONFIG_FRAME_RETRY_INTERVAL_S;
+    } else {
+        if (first_connection && have_display) {
+            status_screen_show(cfg->sta_ssid, STATUS_OK, cfg->toolsserver, STATUS_OK);
+        }
+        if (have_display) {
+            bool stream_started = false;
+            esp_err_t fetch_err = fetch_and_display(cfg, &stream_started);
+            if (fetch_err != ESP_OK) {
+                ESP_LOGW(TAG, "Fetch/display failed (%s), retrying sooner", esp_err_to_name(fetch_err));
+                sleep_seconds = CONFIG_FRAME_RETRY_INTERVAL_S;
+                if (!stream_started) {
+                    /* The panel was never touched this cycle (e.g. a bad
+                     * HTTP status before any pixel data was sent) -- show
+                     * the failure instead of leaving whatever was on
+                     * screen before. A mid-stream failure, by contrast,
+                     * already refreshed the panel with something, so
+                     * skip compounding it with a second refresh here. */
+                    status_screen_show(cfg->sta_ssid, STATUS_OK, cfg->toolsserver, STATUS_FAILED);
+                }
+            }
        }
    }

@@ -91,6 +91,11 @@ esp_err_t frame_config_save(const frame_config_t *cfg)
    if (err == ESP_OK) {
        err = nvs_set_str(handle, "toolsserver", cfg->toolsserver);
    }
+    if (err == ESP_OK) {
+        /* Fresh (re)provisioning -- the next successful connection should
+         * show the status screen again. */
+        err = nvs_set_u8(handle, "connected_once", 0);
+    }
    if (err == ESP_OK) {
        err = nvs_commit(handle);
    }
@@ -99,6 +104,32 @@ esp_err_t frame_config_save(const frame_config_t *cfg)
    return err;
 }

+bool frame_config_has_connected_once(void)
+{
+    nvs_handle_t handle;
+    esp_err_t err = nvs_open(NVS_NAMESPACE, NVS_READONLY, &handle);
+    if (err != ESP_OK) {
+        return false;
+    }
+
+    uint8_t value = 0;
+    err = nvs_get_u8(handle, "connected_once", &value);
+    nvs_close(handle);
+
+    return err == ESP_OK && value != 0;
+}
+
+void frame_config_mark_connected_once(void)
+{
+    nvs_handle_t handle;
+    if (nvs_open(NVS_NAMESPACE, NVS_READWRITE, &handle) != ESP_OK) {
+        return;
+    }
+    nvs_set_u8(handle, "connected_once", 1);
+    nvs_commit(handle);
+    nvs_close(handle);
+}
+
 static void generate_ap_password(char *out, size_t out_size)
 {
    size_t len = MIN(FRAME_AP_PASSWORD_LEN, out_size - 1);
@@ -1,5 +1,6 @@
 #pragma once

+#include <stdbool.h>
 #include <stddef.h>
 #include "esp_err.h"

@@ -20,9 +21,21 @@ typedef struct {
 */
 esp_err_t frame_config_load(frame_config_t *out);

-/** Saves the home-network config to NVS. */
+/** Saves the home-network config to NVS. Resets the "connected once"
+ * flag below, since this is a fresh (re)provisioning event. */
 esp_err_t frame_config_save(const frame_config_t *cfg);

+/**
+ * Whether the device has already shown the post-connect status screen at
+ * least once since the current WiFi config was saved. Used so the status
+ * screen always shows on the first connection after (re)provisioning, but
+ * is skipped on later successful wakes to save an extra refresh.
+ */
+bool frame_config_has_connected_once(void);
+
+/** Marks the status screen as having been shown for the current WiFi config. */
+void frame_config_mark_connected_once(void);
+
 /**
 * Returns this device's provisioning AP identity: a fixed SSID (from
 * Kconfig) and a password that's generated once on first use and persisted