From 94d74e425c9a70962cf6a6f37debc2bc214caa93 Mon Sep 17 00:00:00 2001 From: Peter Woolery Date: Thu, 16 Apr 2026 15:55:35 -0700 Subject: [PATCH] refactor(cv): read thresholds from runtime tuning + load from NVS on boot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cv_process and helpers (frame_diff, extract_blob, find_centroids) now read diff_thresh, min_blob_px, max_move, max_missed, and line_offset from state.tuning instead of file-scope static const constants. The four thresholds are promoted to file-local constexpr defaults in cv.cpp (CV_DEFAULT_*) and are no longer part of the public cv.h API — external code can't depend on them. cv_process signature drops the line_pct parameter; callers use state.tuning.line_offset instead. This eliminates the drift hazard of having two sources of truth (DeviceConfig.line_offset vs CVTuning.line_offset); the former is deleted. main.cpp now calls config_load_tuning(g_cv.tuning) after cv_init on boot so previously persisted tuning survives reboot; logs whether tuning came from NVS or defaults. The legacy NVS key "line_offset" is intentionally left alone — harmless and flash_device.py may still write it during provisioning. Migration is out of scope. Tests: 12/12 passing (11 existing + 1 new test_cv_process_respects_runtime_min_blob proving the runtime-read path). Flash: 1,414,069 bytes (89.9%). Co-Authored-By: Claude Opus 4.7 (1M context) --- firmware/lib/cv/cv.cpp | 55 ++++++++++++++--------- firmware/lib/cv/cv.h | 7 +-- firmware/src/config.cpp | 1 - firmware/src/config.h | 1 - firmware/src/main.cpp | 7 ++- firmware/test/test_cv/test_cv.cpp | 73 +++++++++++++++++++++---------- 6 files changed, 93 insertions(+), 51 deletions(-) diff --git a/firmware/lib/cv/cv.cpp b/firmware/lib/cv/cv.cpp index 695107d..44e39bc 100644 --- a/firmware/lib/cv/cv.cpp +++ b/firmware/lib/cv/cv.cpp @@ -5,6 +5,14 @@ #include #include +// File-local defaults. Runtime values live in CVState::tuning and can be +// overridden via config_load_tuning() on boot or server push at runtime. +static constexpr uint8_t CV_DEFAULT_DIFF_THRESH = 30; +static constexpr int CV_DEFAULT_MIN_BLOB_PX = 64; +static constexpr float CV_DEFAULT_MAX_MOVE = 15.0f; +static constexpr int CV_DEFAULT_MAX_MISSED = 10; +static constexpr uint8_t CV_DEFAULT_LINE_OFFSET = 50; + void cv_init(CVState& state) { // Initialize members directly — avoid CVState{} temporary which puts 9KB on stack memset(state.background, 0, sizeof(state.background)); @@ -15,11 +23,11 @@ void cv_init(CVState& state) { state.tracks.clear(); state.entries = 0; state.exits = 0; - state.tuning.diff_thresh = CV_DIFF_THRESH; - state.tuning.min_blob_px = CV_MIN_BLOB_PX; - state.tuning.max_move = CV_MAX_MOVE; - state.tuning.max_missed = CV_MAX_MISSED; - state.tuning.line_offset = 50; + state.tuning.diff_thresh = CV_DEFAULT_DIFF_THRESH; + state.tuning.min_blob_px = CV_DEFAULT_MIN_BLOB_PX; + state.tuning.max_move = CV_DEFAULT_MAX_MOVE; + state.tuning.max_missed = CV_DEFAULT_MAX_MISSED; + state.tuning.line_offset = CV_DEFAULT_LINE_OFFSET; state.tuning.cfg_version = 0; } @@ -32,8 +40,9 @@ struct Point { int x, y; }; // Note: queue may grow to CV_PIXELS entries (~72KB) on large blobs. // Requires PSRAM (enabled via -DBOARD_HAS_PSRAM in platformio.ini). -// BFS flood fill. Marks visited pixels (sets fg to 0). Returns {-1,-1} if blob < CV_MIN_BLOB_PX. -static std::pair extract_blob(uint8_t* fg, int start_x, int start_y) { +// BFS flood fill. Marks visited pixels (sets fg to 0). Returns {-1,-1} if blob < min_blob_px. +static std::pair extract_blob(uint8_t* fg, int start_x, int start_y, + int min_blob_px) { std::vector queue; queue.reserve(512); queue.push_back({start_x, start_y}); @@ -58,11 +67,12 @@ static std::pair extract_blob(uint8_t* fg, int start_x, int start_y } } - if (count < CV_MIN_BLOB_PX) return {-1.0f, -1.0f}; + if (count < min_blob_px) return {-1.0f, -1.0f}; return {sum_x / count, sum_y / count}; } -static std::vector> find_centroids(const uint8_t* fg) { +static std::vector> find_centroids(const uint8_t* fg, + int min_blob_px) { std::vector> result; static uint8_t fg_copy[CV_PIXELS]; // static to avoid 9KB stack allocation memcpy(fg_copy, fg, CV_PIXELS); @@ -70,7 +80,7 @@ static std::vector> find_centroids(const uint8_t* fg) { for (int y = 0; y < CV_H; y++) { for (int x = 0; x < CV_W; x++) { if (!fg_copy[y * CV_W + x]) continue; - auto c = extract_blob(fg_copy, x, y); + auto c = extract_blob(fg_copy, x, y, min_blob_px); if (c.first >= 0) result.push_back(c); } } @@ -78,15 +88,15 @@ static std::vector> find_centroids(const uint8_t* fg) { } static void frame_diff(const uint8_t* frame, const uint8_t* bg, - uint8_t* fg, int pixels) { + uint8_t* fg, int pixels, uint8_t diff_thresh) { for (int i = 0; i < pixels; i++) { int diff = (int)frame[i] - (int)bg[i]; if (diff < 0) diff = -diff; - fg[i] = (diff > CV_DIFF_THRESH) ? 1 : 0; + fg[i] = (diff > diff_thresh) ? 1 : 0; } } -CVResult cv_process(CVState& state, const uint8_t* frame, uint8_t line_pct) { +CVResult cv_process(CVState& state, const uint8_t* frame) { CVResult result = {0, 0}; state.frame_index++; @@ -96,13 +106,18 @@ CVResult cv_process(CVState& state, const uint8_t* frame, uint8_t line_pct) { return result; } + const uint8_t diff_thresh = state.tuning.diff_thresh; + const int min_blob_px = state.tuning.min_blob_px; + const float max_move = state.tuning.max_move; + const int max_missed = state.tuning.max_missed; + static uint8_t fg[CV_PIXELS]; // static: avoids 9KB on task stack - frame_diff(frame, state.background, fg, CV_PIXELS); + frame_diff(frame, state.background, fg, CV_PIXELS, diff_thresh); int fg_count = 0; for (int i = 0; i < CV_PIXELS; i++) fg_count += fg[i]; - bool motion = fg_count > CV_MIN_BLOB_PX; + bool motion = fg_count > min_blob_px; if (!motion) { if (state.frame_index - state.last_motion_frame > 10) { memcpy(state.background, frame, CV_PIXELS); @@ -110,19 +125,19 @@ CVResult cv_process(CVState& state, const uint8_t* frame, uint8_t line_pct) { for (auto& t : state.tracks) t.missed++; state.tracks.erase( std::remove_if(state.tracks.begin(), state.tracks.end(), - [](const CVTrack& t){ return t.missed > CV_MAX_MISSED; }), + [max_missed](const CVTrack& t){ return t.missed > max_missed; }), state.tracks.end()); return result; } state.last_motion_frame = state.frame_index; - auto centroids = find_centroids(fg); + auto centroids = find_centroids(fg, min_blob_px); std::vector centroid_matched(centroids.size(), false); for (auto& track : state.tracks) { - float best_dist = CV_MAX_MOVE * CV_MAX_MOVE; + float best_dist = max_move * max_move; int best_idx = -1; for (int i = 0; i < (int)centroids.size(); i++) { @@ -145,10 +160,10 @@ CVResult cv_process(CVState& state, const uint8_t* frame, uint8_t line_pct) { state.tracks.erase( std::remove_if(state.tracks.begin(), state.tracks.end(), - [](const CVTrack& t){ return t.missed > CV_MAX_MISSED; }), + [max_missed](const CVTrack& t){ return t.missed > max_missed; }), state.tracks.end()); - float line_y = (line_pct / 100.0f) * CV_H; + float line_y = (state.tuning.line_offset / 100.0f) * CV_H; for (int i = 0; i < (int)centroids.size(); i++) { if (centroid_matched[i]) continue; CVTrack t; diff --git a/firmware/lib/cv/cv.h b/firmware/lib/cv/cv.h index fca5d62..8f308e0 100644 --- a/firmware/lib/cv/cv.h +++ b/firmware/lib/cv/cv.h @@ -7,11 +7,6 @@ static const int CV_W = 96; static const int CV_H = 96; static const int CV_PIXELS = CV_W * CV_H; -static const uint8_t CV_DIFF_THRESH = 30; -static const int CV_MIN_BLOB_PX = 64; -static const float CV_MAX_MOVE = 15.0f; -static const int CV_MAX_MISSED = 10; - struct CVTrack { int id; float x, y; @@ -46,5 +41,5 @@ struct CVResult { }; void cv_init(CVState& state); -CVResult cv_process(CVState& state, const uint8_t* frame, uint8_t line_pct); +CVResult cv_process(CVState& state, const uint8_t* frame); void cv_reset_counts(CVState& state); diff --git a/firmware/src/config.cpp b/firmware/src/config.cpp index ea1fefd..e092575 100644 --- a/firmware/src/config.cpp +++ b/firmware/src/config.cpp @@ -13,7 +13,6 @@ bool config_load(DeviceConfig& cfg) { cfg.hmac_secret = prefs.getString("hmac_secret", ""); cfg.wifi_ssid = prefs.getString("wifi_ssid", ""); cfg.wifi_pass = prefs.getString("wifi_pass", ""); - cfg.line_offset = (uint8_t)prefs.getUInt("line_offset", 50); prefs.end(); diff --git a/firmware/src/config.h b/firmware/src/config.h index e10f8d3..2aa2634 100644 --- a/firmware/src/config.h +++ b/firmware/src/config.h @@ -9,7 +9,6 @@ struct DeviceConfig { String hmac_secret; // 32-byte hex string String wifi_ssid; String wifi_pass; - uint8_t line_offset; // 0-100, percent of frame height for virtual line }; // Load all config from NVS. Returns false if device_id/location_id/hmac_secret missing. diff --git a/firmware/src/main.cpp b/firmware/src/main.cpp index c8189cd..f9fc08a 100644 --- a/firmware/src/main.cpp +++ b/firmware/src/main.cpp @@ -45,7 +45,7 @@ static void task_camera(void*) { while (true) { if (camera_capture_96(frame)) { if (xSemaphoreTake(s_cv_mutex, pdMS_TO_TICKS(100)) == pdTRUE) { - CVResult r = cv_process(g_cv, frame, g_cfg.line_offset); + CVResult r = cv_process(g_cv, frame); if (r.entries_delta) Serial.printf("[CV] entry +%d (total %d)\n", r.entries_delta, g_cv.entries); if (r.exits_delta) Serial.printf("[CV] exit +%d (total %d)\n", r.exits_delta, g_cv.exits); xSemaphoreGive(s_cv_mutex); @@ -140,6 +140,11 @@ void setup() { configTime(0, 0, "pool.ntp.org", "time.nist.gov"); cv_init(g_cv); + if (config_load_tuning(g_cv.tuning)) { + Serial.printf("[CFG] tuning loaded from NVS, cfg_version=%u\n", g_cv.tuning.cfg_version); + } else { + Serial.println("[CFG] no persisted tuning, using defaults"); + } if (!camera_init()) { Serial.println("FATAL: camera init failed"); diff --git a/firmware/test/test_cv/test_cv.cpp b/firmware/test/test_cv/test_cv.cpp index 95895f2..13d0bcf 100644 --- a/firmware/test/test_cv/test_cv.cpp +++ b/firmware/test/test_cv/test_cv.cpp @@ -17,10 +17,10 @@ void test_frame_diff_no_change_gives_no_fg() { uint8_t frame[CV_PIXELS]; fill_frame(frame, 128); - CVResult r1 = cv_process(state, frame, 50); + CVResult r1 = cv_process(state, frame); TEST_ASSERT_EQUAL_INT(0, r1.entries_delta); - CVResult r2 = cv_process(state, frame, 50); + CVResult r2 = cv_process(state, frame); TEST_ASSERT_EQUAL_INT(0, r2.entries_delta); TEST_ASSERT_EQUAL_INT(0, r2.exits_delta); } @@ -33,8 +33,8 @@ void test_frame_diff_large_change_detected_no_crash() { fill_frame(bg, 100); fill_frame(fg_frame, 200); - cv_process(state, bg, 50); - CVResult r = cv_process(state, fg_frame, 50); + cv_process(state, bg); + CVResult r = cv_process(state, fg_frame); // Tracking not yet implemented — just verify no crash and result is zero TEST_ASSERT_EQUAL_INT(0, r.entries_delta); @@ -66,7 +66,7 @@ void test_tracking_spawns_track_for_new_blob() { uint8_t bg[CV_PIXELS]; fill_frame(bg, 100); - cv_process(state, bg, 50); // init background + cv_process(state, bg); // init background // Frame with a bright 30x30 blob in top-left quadrant uint8_t blob_frame[CV_PIXELS]; @@ -75,7 +75,7 @@ void test_tracking_spawns_track_for_new_blob() { for (int x = 5; x < 35; x++) blob_frame[y * CV_W + x] = 200; - cv_process(state, blob_frame, 50); + cv_process(state, blob_frame); TEST_ASSERT_EQUAL_INT(1, (int)state.tracks.size()); TEST_ASSERT_FLOAT_WITHIN(5.0f, 20.0f, state.tracks[0].x); @@ -94,20 +94,20 @@ void test_blob_crossing_line_top_to_bottom_is_entry() { CVState state; cv_init(state); - // Line at 50% = y=48; step ≤14px per frame to stay within CV_MAX_MOVE + // Line at 50% = y=48; step ≤14px per frame to stay within max_move (default 15) uint8_t bg[CV_PIXELS]; fill_frame(bg, 100); - cv_process(state, bg, 50); // init background + cv_process(state, bg); // init background // Walk blob from y=20 toward line; crossing occurs at y=48 (above→below) // Stop at crossing frame and assert its result int setup[] = {20, 34}; for (int i = 0; i < 2; i++) { uint8_t f[CV_PIXELS]; make_blob_frame(f, 48, setup[i]); - cv_process(state, f, 50); + cv_process(state, f); } uint8_t fcross[CV_PIXELS]; make_blob_frame(fcross, 48, 48); - CVResult r = cv_process(state, fcross, 50); + CVResult r = cv_process(state, fcross); TEST_ASSERT_EQUAL_INT(1, r.entries_delta); TEST_ASSERT_EQUAL_INT(0, r.exits_delta); @@ -119,17 +119,17 @@ void test_blob_crossing_line_bottom_to_top_is_exit() { cv_init(state); uint8_t bg[CV_PIXELS]; fill_frame(bg, 100); - cv_process(state, bg, 50); + cv_process(state, bg); // Walk blob from y=76 toward line; crossing occurs at y=34 (below→above) // Stop at crossing frame and assert its result int setup[] = {76, 62, 48}; for (int i = 0; i < 3; i++) { uint8_t f[CV_PIXELS]; make_blob_frame(f, 48, setup[i]); - cv_process(state, f, 50); + cv_process(state, f); } uint8_t fcross[CV_PIXELS]; make_blob_frame(fcross, 48, 34); - CVResult r = cv_process(state, fcross, 50); + CVResult r = cv_process(state, fcross); TEST_ASSERT_EQUAL_INT(0, r.entries_delta); TEST_ASSERT_EQUAL_INT(1, r.exits_delta); @@ -147,12 +147,40 @@ void test_cv_init_populates_tuning_defaults() { cv_init(state); - TEST_ASSERT_EQUAL_UINT8(CV_DIFF_THRESH, state.tuning.diff_thresh); - TEST_ASSERT_EQUAL_INT(CV_MIN_BLOB_PX, state.tuning.min_blob_px); - TEST_ASSERT_EQUAL_FLOAT(CV_MAX_MOVE, state.tuning.max_move); - TEST_ASSERT_EQUAL_INT(CV_MAX_MISSED, state.tuning.max_missed); - TEST_ASSERT_EQUAL_UINT8(50, state.tuning.line_offset); - TEST_ASSERT_EQUAL_UINT32(0, state.tuning.cfg_version); + // Values mirror CV_DEFAULT_* constants in cv.cpp (now file-local). + TEST_ASSERT_EQUAL_UINT8(30, state.tuning.diff_thresh); + TEST_ASSERT_EQUAL_INT(64, state.tuning.min_blob_px); + TEST_ASSERT_EQUAL_FLOAT(15.0f, state.tuning.max_move); + TEST_ASSERT_EQUAL_INT(10, state.tuning.max_missed); + TEST_ASSERT_EQUAL_UINT8(50, state.tuning.line_offset); + TEST_ASSERT_EQUAL_UINT32(0, state.tuning.cfg_version); +} + +void test_cv_process_respects_runtime_min_blob() { + // Proves cv_process reads min_blob_px from state.tuning at runtime + // (not from a compile-time constant). With a very high threshold, the + // same blob-producing frame that spawns a track in other tests must NOT + // spawn one here. + CVState state; + cv_init(state); + state.tuning.min_blob_px = 10000; // larger than CV_PIXELS → no blob can qualify + + uint8_t bg[CV_PIXELS]; + fill_frame(bg, 100); + cv_process(state, bg); // init background + + // Same 30x30 blob used by test_tracking_spawns_track_for_new_blob + uint8_t blob_frame[CV_PIXELS]; + fill_frame(blob_frame, 100); + for (int y = 5; y < 35; y++) + for (int x = 5; x < 35; x++) + blob_frame[y * CV_W + x] = 200; + + CVResult r = cv_process(state, blob_frame); + + TEST_ASSERT_EQUAL_INT(0, (int)state.tracks.size()); + TEST_ASSERT_EQUAL_INT(0, r.entries_delta); + TEST_ASSERT_EQUAL_INT(0, r.exits_delta); } void test_no_crossing_same_side_no_count() { @@ -160,13 +188,13 @@ void test_no_crossing_same_side_no_count() { cv_init(state); uint8_t bg[CV_PIXELS]; fill_frame(bg, 100); - cv_process(state, bg, 50); + cv_process(state, bg); uint8_t f1[CV_PIXELS]; make_blob_frame(f1, 48, 20); // above line - cv_process(state, f1, 50); + cv_process(state, f1); uint8_t f2[CV_PIXELS]; make_blob_frame(f2, 48, 30); // still above line, moved closer - CVResult r = cv_process(state, f2, 50); + CVResult r = cv_process(state, f2); TEST_ASSERT_EQUAL_INT(0, r.entries_delta); TEST_ASSERT_EQUAL_INT(0, r.exits_delta); @@ -183,5 +211,6 @@ int main() { RUN_TEST(test_blob_crossing_line_bottom_to_top_is_exit); RUN_TEST(test_no_crossing_same_side_no_count); RUN_TEST(test_cv_init_populates_tuning_defaults); + RUN_TEST(test_cv_process_respects_runtime_min_blob); return UNITY_END(); }