From 988759f7387276b9184f16f64585e9c123f60b84 Mon Sep 17 00:00:00 2001 From: Matt Edholm Date: Thu, 7 May 2026 15:34:20 -0400 Subject: [PATCH] feat(firmware): honor server X-Interval-Ms instead of capping at 60s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dev-only cap that forced every-1-min polling regardless of the app's schedule is removed. The device now sleeps for whatever X-Interval-Ms the server hands back (driven by rotationIntervalMinutes / wakeTimes), clamped to [30s, 25h] as a safety net against malformed values. Renamed FETCH_INTERVAL_MS to FETCH_INTERVAL_MS_FALLBACK — it's now *only* used when the header is absent (rare; rolling deploy / hand- crafted response). Added SLEEP_CLAMP_MIN/MAX for the bounds. Tests FW-09 and FW-10 flipped to lock the new behavior; added FW-10b covering sub-MIN clamping (battery protection if server sends 1000ms). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/config.h | 32 +++++++++-------- src/operation.h | 24 +++++++------ test/mocks/config.h | 4 ++- test/test_normal_operation/test_main.cpp | 45 ++++++++++++++---------- 4 files changed, 61 insertions(+), 44 deletions(-) diff --git a/src/config.h b/src/config.h index f98fdc3..7562a99 100644 --- a/src/config.h +++ b/src/config.h @@ -122,19 +122,23 @@ #define APP_BASE_URL "https://pictureframe.edholm.me" #define AP_IP "192.168.4.1" #define WIFI_TIMEOUT_MS 30000 -#ifndef FETCH_INTERVAL_MS -// TODO(post-dev): drop the 60s cap. Today this value is used in -// operation.h as BOTH the no-header fallback AND the upper bound that -// clamps the server-provided X-Interval-Ms. While we're iterating on the -// firmware we want the frame to poll every minute so changes land fast, -// but in production we want to honor whatever the app sends (e.g., -// rotationIntervalMinutes=60 → 1 hour, or wakeHour set → ~24 h sleep). -// When the firmware stabilizes, split this into two constants: -// - FETCH_INTERVAL_MS_FALLBACK (used when no X-Interval-Ms header) -// - SLEEP_CLAMP_MIN_MS / SLEEP_CLAMP_MAX_MS (sanity bounds, not the -// primary schedule) -// and let server values flow through. See operation.h:138 for the cap -// site, and tests FW-09/FW-10 for the assertions that will need updating. -#define FETCH_INTERVAL_MS 60000 // 1 min deep sleep between polls (DEV value) +// Server's X-Interval-Ms is the primary schedule — driven by the user's +// rotationIntervalMinutes / wakeTimes settings. The constants below are +// only safety nets: +// - FALLBACK is used when the server omits the header (shouldn't happen +// in normal operation, but guards against a rolling deploy or hand- +// crafted response). +// - CLAMP_MIN/MAX bound the server value to sane physical limits — no +// runaway polling on a malformed 0/negative, no week-long naps on a +// misconfigured 999 days. CLAMP_MAX is just past 24 h to give DST +// transitions and edge-case wake-time math a little slack. +#ifndef FETCH_INTERVAL_MS_FALLBACK +#define FETCH_INTERVAL_MS_FALLBACK 60000ULL // 1 min, used only when no X-Interval-Ms header +#endif +#ifndef SLEEP_CLAMP_MIN_MS +#define SLEEP_CLAMP_MIN_MS 30000ULL // 30 s — protect against runaway polls +#endif +#ifndef SLEEP_CLAMP_MAX_MS +#define SLEEP_CLAMP_MAX_MS (25ULL * 60ULL * 60ULL * 1000ULL) // 25 h — past 24h with DST slack #endif #define IMAGE_PATH "/img.bin" diff --git a/src/operation.h b/src/operation.h index effafb0..b4fd74e 100644 --- a/src/operation.h +++ b/src/operation.h @@ -131,20 +131,22 @@ void normal_operation_impl(const String& mac, HTTP& http, const String& url, Pre http.collectHeaders(collectHeaders, 3); int code = http.GET(); - // TODO(post-dev): trust the server's X-Interval-Ms instead of capping - // it at FETCH_INTERVAL_MS. The cap is here so the dev unit polls - // every minute regardless of what the app's rotationIntervalMinutes / - // wakeHour settings say — fast iteration. Once the firmware is stable - // and we want real battery life on V2, the line below should become - // simply `sleepMs = v;` plus a sanity clamp (e.g. min 30 s, max 25 h). - // Tests FW-09 and FW-10 in test_normal_operation/test_main.cpp lock - // the current behavior — update them when removing the cap. See the - // matching note in config.h on FETCH_INTERVAL_MS. - uint64_t sleepMs = FETCH_INTERVAL_MS; + // Honor the server's X-Interval-Ms — that's the user's configured + // rotationIntervalMinutes / wakeTimes schedule, computed in + // DeviceImageController::computeIntervalMs. Clamp to sane physical + // limits so a malformed 0/garbage value doesn't burn the battery + // (CLAMP_MIN) and a misconfigured "every 999 days" doesn't strand the + // device for a week (CLAMP_MAX). When no header is present (server + // bug, mid-deploy), fall back to FETCH_INTERVAL_MS_FALLBACK. + uint64_t sleepMs = FETCH_INTERVAL_MS_FALLBACK; String intervalHdr = http.header("X-Interval-Ms"); if (intervalHdr.length() > 0) { uint64_t v = strtoull(intervalHdr.c_str(), nullptr, 10); - if (v > 0) sleepMs = std::min(v, (uint64_t)FETCH_INTERVAL_MS); + if (v > 0) { + sleepMs = v; + if (sleepMs < SLEEP_CLAMP_MIN_MS) sleepMs = SLEEP_CLAMP_MIN_MS; + if (sleepMs > SLEEP_CLAMP_MAX_MS) sleepMs = SLEEP_CLAMP_MAX_MS; + } } bool displayInitialized = false; diff --git a/test/mocks/config.h b/test/mocks/config.h index fd67dff..3897ebc 100644 --- a/test/mocks/config.h +++ b/test/mocks/config.h @@ -8,7 +8,9 @@ #define NVS_KEY_IMG_ID "img_id" #define NVS_KEY_DRAW_NEEDED "draw" #define IMAGE_PATH "/img.bin" -#define FETCH_INTERVAL_MS 60000ULL +#define FETCH_INTERVAL_MS_FALLBACK 60000ULL +#define SLEEP_CLAMP_MIN_MS 30000ULL +#define SLEEP_CLAMP_MAX_MS (25ULL * 60ULL * 60ULL * 1000ULL) #define WIFI_TIMEOUT_MS 30000 #define RESET_HOLD_MS 5000 #define AP_IP "192.168.4.1" diff --git a/test/test_normal_operation/test_main.cpp b/test/test_normal_operation/test_main.cpp index 44e511c..7224457 100644 --- a/test/test_normal_operation/test_main.cpp +++ b/test/test_normal_operation/test_main.cpp @@ -83,7 +83,7 @@ void tearDown() {} // FW-01: 200 response — file written, epd_draw called, NVS saved, deep sleep started void test_fw01_200_response_happy_path() { - // Use an interval < FETCH_INTERVAL_MS so server value is honored + // 30 s — at SLEEP_CLAMP_MIN_MS, well under MAX, so honored as-is g_http_response_headers["X-Image-Id"] = "42"; g_http_response_headers["X-Interval-Ms"] = "30000"; g_http_body = "BINDATA"; @@ -108,7 +108,7 @@ void test_fw02_headers_read_before_end_regression() { // FW-03: 304 — no epd draw, no init, deep sleep started void test_fw03_304_no_redraw() { g_http_get_code = 304; - // Use an interval < FETCH_INTERVAL_MS so server value is honored + // 30 s — at SLEEP_CLAMP_MIN_MS, well under MAX, so honored as-is g_http_response_headers["X-Interval-Ms"] = "30000"; normal_operation_impl(String("mac"), http, String("url"), prefs); @@ -263,33 +263,41 @@ void test_fw08_no_current_image_id_when_default() { TEST_ASSERT_TRUE(g_http_request_headers.find("X-Current-Image-Id") == g_http_request_headers.end()); } -// FW-09: server interval < FETCH_INTERVAL_MS → server value used +// FW-09: server interval within clamp range → exact server value used. +// 5 min sits well between SLEEP_CLAMP_MIN_MS and SLEEP_CLAMP_MAX_MS. void test_fw09_server_interval_honored() { - g_http_response_headers["X-Interval-Ms"] = "30000"; + g_http_response_headers["X-Interval-Ms"] = "300000"; // 5 min g_http_response_headers["X-Image-Id"] = "1"; normal_operation_impl(String("mac"), http, String("url"), prefs); - TEST_ASSERT_EQUAL_UINT64(30000ULL * 1000ULL, g_sleep_us); + TEST_ASSERT_EQUAL_UINT64(300000ULL * 1000ULL, g_sleep_us); } -// FW-10: server interval > FETCH_INTERVAL_MS → capped at ceiling. -// TODO(post-dev): when the cap in operation.h is removed (so the device -// honors the app's rotationIntervalMinutes / wakeHour settings), this -// test should flip to assert sleepMs == 999999999 (or whatever the -// server-side max is, e.g. 25 h clamp). See operation.h:~140 and the -// matching TODO in config.h on FETCH_INTERVAL_MS. -void test_fw10_server_interval_capped() { +// FW-10: server interval > SLEEP_CLAMP_MAX_MS → clamped at MAX. +// Protects against a misconfigured "every 999 days" stranding the device. +void test_fw10_server_interval_clamped_to_max() { g_http_response_headers["X-Interval-Ms"] = "999999999"; g_http_response_headers["X-Image-Id"] = "1"; normal_operation_impl(String("mac"), http, String("url"), prefs); - TEST_ASSERT_EQUAL_UINT64(FETCH_INTERVAL_MS * 1000ULL, g_sleep_us); + TEST_ASSERT_EQUAL_UINT64(SLEEP_CLAMP_MAX_MS * 1000ULL, g_sleep_us); } -// FW-11: no X-Interval-Ms → default ceiling used -void test_fw11_default_interval_when_absent() { +// FW-10b: server interval < SLEEP_CLAMP_MIN_MS → clamped UP to MIN. +// Protects the battery against a runaway poll if the server sends 1000 ms +// (or anything malformed-but-positive that survives strtoull). +void test_fw10b_server_interval_clamped_to_min() { + g_http_response_headers["X-Interval-Ms"] = "1000"; + g_http_response_headers["X-Image-Id"] = "1"; + normal_operation_impl(String("mac"), http, String("url"), prefs); + TEST_ASSERT_EQUAL_UINT64(SLEEP_CLAMP_MIN_MS * 1000ULL, g_sleep_us); +} + +// FW-11: no X-Interval-Ms → fallback used. Should not happen in normal +// operation but guards against rolling deploys / hand-crafted responses. +void test_fw11_fallback_used_when_header_absent() { g_http_response_headers["X-Image-Id"] = "1"; // no X-Interval-Ms set normal_operation_impl(String("mac"), http, String("url"), prefs); - TEST_ASSERT_EQUAL_UINT64(FETCH_INTERVAL_MS * 1000ULL, g_sleep_us); + TEST_ASSERT_EQUAL_UINT64(FETCH_INTERVAL_MS_FALLBACK * 1000ULL, g_sleep_us); } // FW-14: 304 — epd_sleep NOT called (display already in hardware deep sleep) @@ -392,8 +400,9 @@ int main(int argc, char** argv) { RUN_TEST(test_fw07_current_image_id_sent_when_saved); RUN_TEST(test_fw08_no_current_image_id_when_default); RUN_TEST(test_fw09_server_interval_honored); - RUN_TEST(test_fw10_server_interval_capped); - RUN_TEST(test_fw11_default_interval_when_absent); + RUN_TEST(test_fw10_server_interval_clamped_to_max); + RUN_TEST(test_fw10b_server_interval_clamped_to_min); + RUN_TEST(test_fw11_fallback_used_when_header_absent); RUN_TEST(test_fw12_ap_ssid_from_mac_aabbcc); RUN_TEST(test_fw13_ap_ssid_from_real_mac); RUN_TEST(test_fw14_304_skips_epd_sleep);