From 32697940a996a742390407142828e6397b80fe24 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Wed, 9 Sep 2026 14:15:54 +0800 Subject: [PATCH 1/5] chore: add bounded Tab5 media stage diagnostics --- .github/workflows/ci.yml | 1 + .../esp_openclaw_room_node.c | 102 +++++- .../esp-openclaw-room-node/tests/README.md | 17 + .../tests/host/esp_log.h | 3 + .../tests/host/esp_rom_sys.h | 5 + .../tests/host/room_host_fakes.c | 46 ++- .../tests/host/room_host_fakes.h | 3 + .../tests/test_room_talk_lifecycle.c | 58 ++++ components/esp-openclaw-talk/README.md | 22 ++ .../esp-openclaw-talk/src/esp_openclaw_talk.c | 123 +++++-- .../esp-openclaw-talk/tests/host/esp_log.h | 3 + .../tests/host/talk_host_runner.c | 17 + .../tests/run_threaded_tests.py | 8 +- .../tests/test_talk_threaded.c | 152 ++++++++- examples/m5stack-tab5-room-node/README.md | 17 + .../tab5_room_board/tab5_room_board.c | 91 ++++- .../tests/fixtures/test_tab5_camera_capture.c | 310 ++++++++++++++++++ scripts/tests/test_tab5_camera_diagnostics.py | 59 ++++ 18 files changed, 965 insertions(+), 72 deletions(-) create mode 100644 scripts/tests/fixtures/test_tab5_camera_capture.c create mode 100644 scripts/tests/test_tab5_camera_diagnostics.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e7821d5..301b822 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -71,6 +71,7 @@ jobs: python3 -m unittest discover -s scripts/tests -p 'test_package_firmware.py' -v python3 -m unittest discover -s scripts/tests -p 'test_idf_tab5_compat.py' -v python3 -m unittest discover -s scripts/tests -p 'test_nvs_diagnostics.py' -v + python3 -m unittest discover -s scripts/tests -p 'test_tab5_camera_diagnostics.py' -v - name: Build uses: espressif/esp-idf-ci-action@v1 diff --git a/components/esp-openclaw-room-node/esp_openclaw_room_node.c b/components/esp-openclaw-room-node/esp_openclaw_room_node.c index 7aa89f8..034af0b 100644 --- a/components/esp-openclaw-room-node/esp_openclaw_room_node.c +++ b/components/esp-openclaw-room-node/esp_openclaw_room_node.c @@ -83,6 +83,26 @@ static char *gateway_http_base; * generation-checked facts under state_lock are the only work authority. */ typedef uint8_t talk_teardown_request_t; +static int talk_stage(uint32_t generation, const char *stage, const char *phase, int result) +{ + ESP_LOGI(TAG, "room_talk_diag generation=%" PRIu32 " stage=%s phase=%s result=%d", + generation, stage, phase, result); + return result; +} + +static void talk_audio_snapshot(uint32_t generation, const char *stage) +{ + room_audio_diagnostics_snapshot_t audio = {0}; + room_diagnostics_audio_get(&audio); + ESP_LOGI(TAG, + "room_talk_audio generation=%" PRIu32 " stage=%s capture_ok=%" PRIu64 + " capture_err=%" PRIu64 " feed_ok=%" PRIu64 " feed_err=%" PRIu64 + " fetch_ok=%" PRIu64 " fetch_err=%" PRIu64 " render_ok=%" PRIu64 " render_err=%" PRIu64, + generation, stage, audio.capture_read_successes, audio.capture_read_errors, + audio.feed_successes, audio.feed_errors, audio.fetch_successes, audio.fetch_errors, + audio.renderer_accepted, audio.renderer_errors); +} + static void refresh_home_facts(void) { xSemaphoreTake(state_lock, portMAX_DELAY); @@ -313,8 +333,9 @@ static bool request_talk_stop_locked(uint32_t generation, const char *message) static void request_talk_teardown(uint32_t generation, const char *message) { xSemaphoreTake(state_lock, portMAX_DELAY); - request_talk_stop_locked(generation, message); + bool accepted = request_talk_stop_locked(generation, message); xSemaphoreGive(state_lock); + talk_stage(generation, "stop_request", accepted ? "accepted" : "ignored", 0); } static void talk_teardown_task(void *arg) @@ -337,6 +358,7 @@ static void talk_teardown_task(void *arg) esp_webrtc_handle_t session = webrtc; esp_openclaw_talk_call_handle_t call = talk_call; const char *message = talk_cancel_message; + uint32_t generation = talk_generation; xSemaphoreGive(state_lock); refresh_home_facts(); /* One worker serializes call/operator UI. A delayed callback never @@ -353,18 +375,36 @@ static void talk_teardown_task(void *arg) /* The owner remains reserved until every local resource and UI update * is finished. Pending RPC refs may survive, but cannot dispatch. */ + talk_stage(generation, "stop_request", "consumed", 0); + talk_audio_snapshot(generation, "teardown"); + talk_stage(generation, "quiesce", "begin", 0); esp_openclaw_talk_call_quiesce(call); + talk_stage(generation, "quiesce", "end", 0); if (talk_timeout_timer != NULL) { xTimerStop(talk_timeout_timer, portMAX_DELAY); xTimerDelete(talk_timeout_timer, portMAX_DELAY); talk_timeout_timer = NULL; } - if (session != NULL) esp_webrtc_close(session); - if (talk_ambient_suspended && room_media_set_ambient_wake(true) != ESP_OK) { - ESP_LOGE(TAG, "failed to restore ambient WakeNet after Talk"); - message = "Talk failed"; + if (session != NULL) { + talk_stage(generation, "sdk_close", "begin", 0); + int result = esp_webrtc_close(session); + talk_stage(generation, "sdk_close", "end", result); + } + if (talk_ambient_suspended) { + talk_stage(generation, "ambient_restore", "begin", 0); + esp_err_t result = room_media_set_ambient_wake(true); + talk_stage(generation, "ambient_restore", "end", result); + if (result != ESP_OK) { + ESP_LOGE(TAG, "failed to restore ambient WakeNet after Talk"); + message = "Talk failed"; + } + } + if (talk_media_owned) { + talk_stage(generation, "media_release", "begin", 0); + room_media_end_talk(!talk_ambient_suspended); + talk_stage(generation, "media_release", "end", 0); } - if (talk_media_owned) room_media_end_talk(!talk_ambient_suspended); + talk_audio_snapshot(generation, "released"); room_ui_set(message != NULL ? ROOM_UI_ERROR : ROOM_UI_IDLE, message); xSemaphoreTake(state_lock, portMAX_DELAY); webrtc = NULL; @@ -415,6 +455,16 @@ static int webrtc_event(esp_webrtc_event_t *event, void *ctx) event->type == ESP_WEBRTC_EVENT_CONNECT_FAILED ? "Talk failed" : NULL); } xSemaphoreGive(state_lock); + const char *stage = event->type == ESP_WEBRTC_EVENT_CONNECTING ? "peer_connecting" : + event->type == ESP_WEBRTC_EVENT_PAIRED ? "peer_paired" : + event->type == ESP_WEBRTC_EVENT_CONNECTED ? "peer_connected" : + event->type == ESP_WEBRTC_EVENT_CONNECT_FAILED ? "peer_failed" : + event->type == ESP_WEBRTC_EVENT_DISCONNECTED ? "peer_disconnected" : NULL; + if (stage != NULL) { + talk_stage(generation, stage, current ? "current" : "stale", + (int)event->type); + if (current) talk_audio_snapshot(generation, stage); + } return 0; } @@ -465,8 +515,12 @@ static void start_talk_once(void) bool admitted = call != NULL && talk_start_in_flight; xSemaphoreGive(state_lock); if (!admitted) return; + talk_stage(generation, "startup", "begin", 0); if (cancelled) goto done; + talk_stage(generation, "media_acquire", "begin", 0); room_media_begin_talk(); + talk_stage(generation, "media_acquire", "end", 0); + talk_audio_snapshot(generation, "acquired"); xSemaphoreTake(state_lock, portMAX_DELAY); talk_media_owned = true; cancelled = talk_cancel_requested; @@ -487,7 +541,16 @@ static void start_talk_once(void) .signaling_impl = esp_openclaw_talk_call_signaling_impl(), }; esp_webrtc_handle_t session = NULL; - if (esp_webrtc_open(&config, &session) != 0) { + /* This SDK tag logs outbound SDP at INFO. Set before any negotiation. */ + esp_log_level_set("webrtc", ESP_LOG_WARN); + if (esp_log_level_get("webrtc") > ESP_LOG_WARN) { + talk_stage(generation, "sdk_log_policy", "end", ESP_FAIL); + request_talk_teardown(generation, "Talk setup failed"); + goto done; + } + talk_stage(generation, "sdk_log_policy", "end", ESP_OK); + talk_stage(generation, "sdk_open", "begin", 0); + if (talk_stage(generation, "sdk_open", "end", esp_webrtc_open(&config, &session)) != 0) { request_talk_teardown(generation, "WebRTC open"); goto done; } @@ -496,10 +559,11 @@ static void start_talk_once(void) talk_dialing = true; xSemaphoreGive(state_lock); esp_webrtc_media_provider_t media = {0}; - if (room_media_get_webrtc_provider(&media) != ESP_OK || - esp_webrtc_set_media_provider(session, &media) != 0 || - esp_webrtc_set_no_auto_capture(session, true) != 0 || - esp_webrtc_set_event_handler(session, webrtc_event, (void *)(uintptr_t)generation) != 0) { + if (talk_stage(generation, "media_provider", "end", room_media_get_webrtc_provider(&media)) != ESP_OK || + talk_stage(generation, "sdk_media_provider", "end", esp_webrtc_set_media_provider(session, &media)) != 0 || + talk_stage(generation, "sdk_capture_policy", "end", esp_webrtc_set_no_auto_capture(session, true)) != 0 || + talk_stage(generation, "sdk_event_handler", "end", + esp_webrtc_set_event_handler(session, webrtc_event, (void *)(uintptr_t)generation)) != 0) { request_talk_teardown(generation, "WebRTC start"); goto done; } @@ -508,8 +572,14 @@ static void start_talk_once(void) if (!cancelled) talk_ambient_suspended = true; xSemaphoreGive(state_lock); if (cancelled) goto done; - if (room_media_set_ambient_wake(false) != ESP_OK || esp_webrtc_start(session) != 0 || - xTimerStart(talk_timeout_timer, 0) != pdPASS) { + talk_stage(generation, "ambient_suspend", "begin", 0); + if (talk_stage(generation, "ambient_suspend", "end", room_media_set_ambient_wake(false)) != ESP_OK) { + request_talk_teardown(generation, "Talk setup failed"); + goto done; + } + talk_stage(generation, "sdk_start", "begin", 0); + if (talk_stage(generation, "sdk_start", "end", esp_webrtc_start(session)) != 0 || + talk_stage(generation, "timer_start", "end", xTimerStart(talk_timeout_timer, 0)) != pdPASS) { request_talk_teardown(generation, "Talk setup failed"); } @@ -517,7 +587,9 @@ static void start_talk_once(void) xSemaphoreTake(state_lock, portMAX_DELAY); talk_start_in_flight = false; if (talk_cancel_requested) wake_talk_teardown(); + cancelled = talk_cancel_requested; xSemaphoreGive(state_lock); + talk_stage(generation, "startup", cancelled ? "canceled" : "end", 0); } static void on_wake(const char *wake_word, void *ctx) @@ -969,8 +1041,10 @@ static esp_err_t handle_talk_stop( return ESP_ERR_INVALID_ARG; } xSemaphoreTake(state_lock, portMAX_DELAY); - bool active = request_talk_stop_locked(talk_generation, NULL); + uint32_t generation = talk_generation; + bool active = request_talk_stop_locked(generation, NULL); xSemaphoreGive(state_lock); + talk_stage(generation, "stop_request", active ? "accepted" : "ignored", 0); *out_payload_json = strdup( active ? "{\"stopped\":true}" : "{\"stopped\":false}"); if (*out_payload_json == NULL) { diff --git a/components/esp-openclaw-room-node/tests/README.md b/components/esp-openclaw-room-node/tests/README.md index f1b9d04..ab9239c 100644 --- a/components/esp-openclaw-room-node/tests/README.md +++ b/components/esp-openclaw-room-node/tests/README.md @@ -1,5 +1,22 @@ # Talk lifetime source proofs +## Media diagnostic regressions + +The Talk pthread suite now captures the actual adapter's fixed-field records, +injects HTTP initialization/transport/status/body failures and callback return +errors, and checks first-failure retention, ignored-return behavior, late +cancellation and secret exclusion. The room lifecycle fixture checks actual +startup failure short circuits, ordered teardown stages, SDK log suppression +before negotiation, and existing audio-counter snapshots outside locks. + +`python3 -m unittest discover -s scripts/tests -p test_tab5_camera_diagnostics.py -v` +compiles the actual Tab5 capture/cleanup functions with narrow synthetic V4L2 +boundaries under ASan/UBSan. It covers immediate errno retention across logging +and failing cleanup, stale errno on validation, partial mappings, initialization +caching, RGB565 variants and bounded dequeue-loop records. This is not V4L2 ABI, +DMA, sensor or physical frame proof. `TAB5_CAMERA_SOURCE` can select a trusted +baseline source file for a red comparison without editing the worktree. + ## Static home UI The UI host runner compiles the real UI controller and board binding with LVGL 9, diff --git a/components/esp-openclaw-room-node/tests/host/esp_log.h b/components/esp-openclaw-room-node/tests/host/esp_log.h index 80e7354..cfb6c1e 100644 --- a/components/esp-openclaw-room-node/tests/host/esp_log.h +++ b/components/esp-openclaw-room-node/tests/host/esp_log.h @@ -1,4 +1,7 @@ #pragma once +typedef enum { ESP_LOG_NONE, ESP_LOG_ERROR, ESP_LOG_WARN, ESP_LOG_INFO } esp_log_level_t; +void esp_log_level_set(const char *tag, esp_log_level_t level); +esp_log_level_t esp_log_level_get(const char *tag); void host_log(const char *tag, const char *format, ...) __attribute__((format(printf, 2, 3))); #define ESP_LOGE(...) host_log(__VA_ARGS__) diff --git a/components/esp-openclaw-room-node/tests/host/esp_rom_sys.h b/components/esp-openclaw-room-node/tests/host/esp_rom_sys.h index e9ff653..6a2cb37 100644 --- a/components/esp-openclaw-room-node/tests/host/esp_rom_sys.h +++ b/components/esp-openclaw-room-node/tests/host/esp_rom_sys.h @@ -1,3 +1,8 @@ #pragma once +#ifdef __APPLE__ +/* Device ELF placement is not meaningful in the Mach-O host fixture. */ +#undef DRAM_STR +#define DRAM_STR(value) (value) +#endif int esp_rom_printf(const char *format, ...) __attribute__((format(printf, 1, 2))); diff --git a/components/esp-openclaw-room-node/tests/host/room_host_fakes.c b/components/esp-openclaw-room-node/tests/host/room_host_fakes.c index cf9b7f1..4f02170 100644 --- a/components/esp-openclaw-room-node/tests/host/room_host_fakes.c +++ b/components/esp-openclaw-room-node/tests/host/room_host_fakes.c @@ -76,12 +76,41 @@ void host_require(bool condition, const char *message) void host_log(const char *tag, const char *format, ...) { + char line[1024]; va_list args; va_start(args, format); - fprintf(stderr, "[%s] ", tag); - vfprintf(stderr, format, args); - fputc('\n', stderr); + int length = vsnprintf(line, sizeof(line), format, args); va_end(args); + host_require(length >= 0 && (size_t)length < sizeof(line), "bounded fixture log"); + bool diagnostic = strncmp(line, "room_talk_", 10) == 0 || strncmp(line, "talk_rtc_", 9) == 0; + if (diagnostic) { + host_require(mutex_depth == 0 && host.critical_depth == 0, "media diagnostic outside locks"); + host_require(strstr(line, "voice-a") == NULL && strstr(line, "synthetic") == NULL && + strstr(line, "gateway.example") == NULL && strstr(line, "agent:fixture") == NULL, + "media diagnostics exclude secret canaries"); + if (host.capture_diagnostics) { + size_t offset = strlen(host.diagnostics); + host_require(offset + (size_t)length + 2 < sizeof(host.diagnostics), "fixture diagnostic capacity"); + memcpy(host.diagnostics + offset, line, (size_t)length); + host.diagnostics[offset + (size_t)length] = '\n'; + host.diagnostics[offset + (size_t)length + 1] = '\0'; + } + } + fprintf(stderr, "[%s] %s\n", tag, line); +} + +void esp_log_level_set(const char *tag, esp_log_level_t level) +{ + host_require(mutex_depth == 0 && host.critical_depth == 0, "log policy outside locks"); + host_require(strcmp(tag, "webrtc") == 0 && level == ESP_LOG_WARN, "only unsafe SDK SDP INFO suppressed"); + if (!host.fail_log_policy) host.sdk_log_suppressed = true; +} + +esp_log_level_t esp_log_level_get(const char *tag) +{ + host_require(mutex_depth == 0 && host.critical_depth == 0, "log policy readback outside locks"); + host_require(strcmp(tag, "webrtc") == 0, "only SDK SDP tag read back"); + return host.sdk_log_suppressed ? ESP_LOG_WARN : ESP_LOG_INFO; } int esp_rom_printf(const char *format, ...) @@ -531,6 +560,7 @@ static int signal_message(esp_peer_signaling_msg_t *message, void *ctx) { (void)message; (void)ctx; host_require(false, "no SDP exchange in this suite"); return -1; } int esp_webrtc_start(esp_webrtc_handle_t session) { + host_require(host.sdk_log_suppressed, "SDK SDP INFO suppressed before negotiation"); if (host.fail_start) return -1; fake_rtc_t *rtc = session; esp_peer_signaling_cfg_t config = { @@ -646,7 +676,15 @@ esp_err_t room_diagnostics_request_open(void) esp_err_t room_diagnostics_request_close(void) { unsupported_boundary(__func__); } void room_diagnostics_audio_get(room_audio_diagnostics_snapshot_t *snapshot) -{ (void)snapshot; unsupported_boundary(__func__); } +{ + host_require(mutex_depth == 0 && host.critical_depth == 0, "audio snapshot outside locks"); + ++host.audio_snapshots; + *snapshot = (room_audio_diagnostics_snapshot_t){ + .capture_read_successes = 11, .capture_read_errors = 2, + .feed_successes = 13, .feed_errors = 3, .fetch_successes = 17, + .fetch_errors = 5, .renderer_accepted = 19, .renderer_errors = 7, + }; +} esp_err_t room_media_request_test_tone(room_media_talk_busy_cb_t busy_cb, void *ctx) { (void)busy_cb; (void)ctx; unsupported_boundary(__func__); } void room_media_get_tone_snapshot(room_media_tone_snapshot_t *snapshot) diff --git a/components/esp-openclaw-room-node/tests/host/room_host_fakes.h b/components/esp-openclaw-room-node/tests/host/room_host_fakes.h index 52b90f0..7dfe102 100644 --- a/components/esp-openclaw-room-node/tests/host/room_host_fakes.h +++ b/components/esp-openclaw-room-node/tests/host/room_host_fakes.h @@ -34,6 +34,9 @@ typedef struct { bool fail_config_submit, fail_create_submit, fail_open, fail_provider, fail_start, fail_timer; const char *node_uri; const char *create_voice; + bool sdk_log_suppressed, fail_log_policy, capture_diagnostics; + unsigned audio_snapshots; + char diagnostics[16384]; } room_host_observations_t; extern room_host_observations_t host; diff --git a/components/esp-openclaw-room-node/tests/test_room_talk_lifecycle.c b/components/esp-openclaw-room-node/tests/test_room_talk_lifecycle.c index cdca6a0..a0f8f08 100644 --- a/components/esp-openclaw-room-node/tests/test_room_talk_lifecycle.c +++ b/components/esp-openclaw-room-node/tests/test_room_talk_lifecycle.c @@ -577,28 +577,39 @@ static void immediate_create_failure(void) } static void open_failure(void) { + host.capture_diagnostics = true; host.fail_open = true; admit(); host_run_task("talk_start"); drain(); CHECK(host.closes == 0 && host.media_ends == 1 && talk_call == NULL, "open failure releases admitted media once"); + CHECK(strstr(host.diagnostics, "stage=sdk_open phase=end result=-1") != NULL, + "actual open failure classified before cleanup"); + CHECK(strstr(host.diagnostics, "stage=sdk_start") == NULL, "failed open never starts the SDK"); } static void provider_failure(void) { + host.capture_diagnostics = true; host.fail_provider = true; admit(); host_run_task("talk_start"); drain(); CHECK(host.closes == 1 && host.media_ends == 1 && host.ambient_restores == 0 && talk_call == NULL, "pre-start failure closes with capture still ambient"); + CHECK(strstr(host.diagnostics, "stage=sdk_media_provider phase=end result=-1") != NULL, + "actual provider failure classified"); + CHECK(strstr(host.diagnostics, "stage=sdk_capture_policy") == NULL, "provider short circuit preserved"); } static void start_failure(void) { + host.capture_diagnostics = true; host.fail_start = true; admit(); host_run_task("talk_start"); drain(); expect_closed(); + CHECK(strstr(host.diagnostics, "stage=sdk_start phase=end result=-1") != NULL, "actual start failure classified"); + CHECK(strstr(host.diagnostics, "stage=timer_start") == NULL, "SDK failure still skips timer start"); } static void timer_failure(void) { @@ -618,7 +629,54 @@ static void timeout_stop(void) expect_closed(); } +static void diagnostic_boundaries(void) +{ + host.capture_diagnostics = true; + start_pending_config(); + host_reply_config(); + host_reply_create(true); + host_emit_peer(webrtc, ESP_WEBRTC_EVENT_CONNECTING); + host_emit_peer(webrtc, ESP_WEBRTC_EVENT_PAIRED); + CHECK(!talk_active, "connecting and paired are not connected"); + host_emit_peer(webrtc, ESP_WEBRTC_EVENT_CONNECTED); + stop(); + drain(); + const char *stages[] = {"stage=peer_connecting", "stage=peer_paired", "stage=peer_connected", + "stage=stop_request phase=accepted", "stage=quiesce phase=begin", "stage=quiesce phase=end", + "stage=sdk_close phase=begin", "stage=sdk_close phase=end", "stage=ambient_restore phase=end", + "stage=media_release phase=end"}; + const char *cursor = host.diagnostics; + for (size_t i = 0; i < sizeof(stages) / sizeof(*stages); ++i) { + const char *next = strstr(cursor, stages[i]); + CHECK(next != NULL, "actual event and teardown boundaries recorded in order"); + if (next != NULL) cursor = next + strlen(stages[i]); + } + CHECK(strstr(host.diagnostics, + "capture_ok=11 capture_err=2 feed_ok=13 feed_err=3 fetch_ok=17 fetch_err=5 render_ok=19 render_err=7") != NULL, + "audio record uses actual existing snapshot fields, not inferred network delivery"); + CHECK(host.audio_snapshots == 6, "audio sampled at acquire, three peer events, teardown and release only"); + expect_closed(); +} + +static void sdk_log_policy_failure(void) +{ + host.capture_diagnostics = true; + host.fail_log_policy = true; + admit(); + host_run_task("talk_start"); + CHECK(host.opens == 0 && host.starts == 0 && host.config_requests == 0, + "failed SDK tag suppression must stop before WebRTC open or negotiation"); + drain(); + CHECK(host.closes == 0 && host.media_ends == 1 && host.ambient && talk_call == NULL, + "privacy failure uses existing local cleanup with capture still ambient"); + CHECK(strstr(host.diagnostics, "stage=sdk_log_policy phase=end result=-1") != NULL, + "privacy refusal has a fixed local failure stage"); + CHECK(strstr(host.diagnostics, "stage=sdk_open") == NULL, "no SDK open marker after privacy refusal"); +} + static const struct { const char *name; void (*run)(void); } cases[] = { + {"sdk-log-policy-failure", sdk_log_policy_failure}, + {"diagnostic-boundaries", diagnostic_boundaries}, {"home-connection-facts", home_connection_facts}, {"late-create-replacement", late_create_replacement}, {"wake-admission-loss", wake_admission_loss}, diff --git a/components/esp-openclaw-talk/README.md b/components/esp-openclaw-talk/README.md index a285bfc..332a316 100644 --- a/components/esp-openclaw-talk/README.md +++ b/components/esp-openclaw-talk/README.md @@ -29,6 +29,28 @@ default. The ESP WebRTC source is pinned as the repository submodule at `third_party/esp-webrtc-solution`; clone this repository with submodules enabled. +## Stage diagnostics + +`talk_rtc_diag` records local stage/phase, numeric result and a +`first_failure` flag claimed once per call. A `begin` without its matching +`end` is unfinished, not a reported failure. RPC submission completion is not +remote completion. `absent` and `canceled` distinguish an unconfigured callback +from one skipped after cancellation. Header, post-field and ICE/signaling +callback errors are observed without changing their existing ignored-return +policy. Descriptor and HTTP records contain only validation flags and HTTP +status; SDP, URLs, identity fields, headers and remote error text are excluded. + +Room examples emit `room_talk_diag` with the existing local generation around +startup, peer events and teardown. Its `result` preserves each stage's native +convention: timer `pdPASS=1` is success and peer enum values are not errors; +nonzero does not generically mean failure. `room_talk_audio` snapshots existing +cumulative capture/AFE/renderer counters at those boundaries only. These +counters do not prove network delivery or audible output. The application sets +the pinned SDK's `webrtc` tag to WARN before negotiation because its INFO +output includes outbound SDP. It reads the tag level back and refuses to open +WebRTC if suppression failed. Do not enable that tag's INFO/DEBUG output in +captures intended for sharing. These diagnostics do not qualify RTC or audio. + ## Prepared-call lifetime Owners that close WebRTC asynchronously should prepare a call before scheduling diff --git a/components/esp-openclaw-talk/src/esp_openclaw_talk.c b/components/esp-openclaw-talk/src/esp_openclaw_talk.c index 70f47d4..7084f5b 100644 --- a/components/esp-openclaw-talk/src/esp_openclaw_talk.c +++ b/components/esp-openclaw-talk/src/esp_openclaw_talk.c @@ -58,14 +58,31 @@ typedef struct esp_openclaw_talk_call { bool stopped; bool close_notified; bool gateway_owned; + bool diagnostic_failure_seen; } talk_signaling_t; typedef struct { char *data; size_t len; bool failed; + const char *failure_stage; + esp_err_t failure_error; } http_response_t; +/* Stage and phase are local literals only. Never pass broker or SDP fields. */ +static void talk_diag(talk_signaling_t *talk, const char *stage, const char *phase, int error) +{ + bool first = false; + if (error != 0) { + portENTER_CRITICAL(&talk->state_lock); + first = !talk->diagnostic_failure_seen; + talk->diagnostic_failure_seen = true; + portEXIT_CRITICAL(&talk->state_lock); + } + ESP_LOGI(TAG, "talk_rtc_diag stage=%s phase=%s error=%d first_failure=%d", + stage, phase, error, first); +} + static char *duplicate_optional(const char *value) { return value != NULL && value[0] != '\0' ? strdup(value) : NULL; @@ -409,9 +426,7 @@ static bool has_gateway_control_descriptor(cJSON *payload) static void signal_failed( talk_signaling_t *talk, - esp_openclaw_talk_setup_result_t result, - const char *message, - const char *code) + esp_openclaw_talk_setup_result_t result) { if (!dispatch_enter(talk)) return; bool notify = false; @@ -422,8 +437,7 @@ static void signal_failed( } portEXIT_CRITICAL(&talk->state_lock); if (notify) { - /* Error codes are bounded; provider/configuration payloads stay private. */ - ESP_LOGE(TAG, "%s%s%.32s", message, code != NULL ? ": " : "", code != NULL ? code : ""); + ESP_LOGE(TAG, "Talk setup failed: result=%d", (int)result); if (talk->setup_failed_cb != NULL) { talk->setup_failed_cb(result, talk->setup_failed_ctx); } @@ -444,6 +458,7 @@ static void handle_talk_create( { (void)node; talk_signaling_t *talk = user_ctx; + talk_diag(talk, "create_callback", "begin", 0); cJSON *payload = result->ok && result->payload_json != NULL ? cJSON_Parse(result->payload_json) : NULL; @@ -470,6 +485,12 @@ static void handle_talk_create( offer_url->valuestring[0] == '/' && cJSON_IsString(client_secret) && session_id != NULL; + ESP_LOGI(TAG, + "talk_rtc_descriptor rpc_ok=%d payload_object=%d gateway_control=%d transport_valid=%d offer_present=%d secret_present=%d session_valid=%d", + result->ok, cJSON_IsObject(payload), gateway_control, + cJSON_IsString(transport) && strcmp(transport->valuestring, "webrtc") == 0, + cJSON_IsString(offer_url), cJSON_IsString(client_secret), session_id != NULL); + talk_diag(talk, "create_descriptor", "end", valid ? 0 : ESP_FAIL); if (valid) { talk->offer_url = resolve_offer_url( talk->gateway_http_base_url, @@ -477,6 +498,7 @@ static void handle_talk_create( talk->client_secret = duplicate_optional(client_secret->valuestring); valid = talk->offer_url != NULL && talk->client_secret != NULL && copy_offer_headers(talk, offer_headers); + talk_diag(talk, "create_material", "end", valid ? 0 : ESP_FAIL); } cJSON_Delete(payload); if (!valid) { @@ -493,9 +515,8 @@ static void handle_talk_create( talk, upgrade_required ? ESP_OPENCLAW_TALK_GATEWAY_UPGRADE_REQUIRED - : ESP_OPENCLAW_TALK_SETUP_FAILED, - upgrade_required ? "Gateway upgrade required" : "Talk setup failed", - result->ok ? NULL : result->error_code); + : ESP_OPENCLAW_TALK_SETUP_FAILED); + talk_diag(talk, "create_callback", "end", ESP_FAIL); release_talk_signaling(talk); return; } @@ -508,6 +529,7 @@ static void handle_talk_create( } portEXIT_CRITICAL(&talk->state_lock); if (stopped) { + talk_diag(talk, "create_callback", "canceled", 0); close_voice_session(talk->operator_node, talk->session_key, talk->voice_session_id); release_talk_signaling(talk); return; @@ -516,19 +538,27 @@ static void handle_talk_create( esp_peer_signaling_ice_info_t ice_info = {.is_initiator = true}; if (dispatch_enter(talk)) { if (talk->signaling.on_ice_info != NULL) { - talk->signaling.on_ice_info(&ice_info, talk->signaling.ctx); - } + talk_diag(talk, "ice_callback", "begin", 0); + int result = talk->signaling.on_ice_info(&ice_info, talk->signaling.ctx); + talk_diag(talk, "ice_callback", "end", result); + } else talk_diag(talk, "ice_callback", "absent", 0); dispatch_exit(talk); - } + } else talk_diag(talk, "ice_callback", "canceled", 0); if (dispatch_enter(talk)) { - if (talk->signaling.on_connected != NULL) talk->signaling.on_connected(talk->signaling.ctx); + if (talk->signaling.on_connected != NULL) { + talk_diag(talk, "signaling_callback", "begin", 0); + int result = talk->signaling.on_connected(talk->signaling.ctx); + talk_diag(talk, "signaling_callback", "end", result); + } else talk_diag(talk, "signaling_callback", "absent", 0); dispatch_exit(talk); - } + } else talk_diag(talk, "signaling_callback", "canceled", 0); + talk_diag(talk, "create_callback", "end", 0); release_talk_signaling(talk); } static esp_err_t request_talk_create(talk_signaling_t *talk) { + talk_diag(talk, "create_rpc", "begin", 0); cJSON *params = cJSON_CreateObject(); cJSON_AddStringToObject(params, "mode", "realtime"); cJSON_AddStringToObject(params, "transport", "webrtc"); @@ -555,12 +585,17 @@ static esp_err_t request_talk_create(talk_signaling_t *talk) !cJSON_AddItemToArray(capabilities, gateway_control)) { cJSON_Delete(gateway_control); cJSON_Delete(params); + talk_diag(talk, "create_rpc", "end", ESP_ERR_NO_MEM); return ESP_ERR_NO_MEM; } char *params_json = cJSON_PrintUnformatted(params); cJSON_Delete(params); - if (params_json == NULL) return ESP_ERR_NO_MEM; + if (params_json == NULL) { + talk_diag(talk, "create_rpc", "end", ESP_ERR_NO_MEM); + return ESP_ERR_NO_MEM; + } esp_err_t err = request_talk_rpc(talk, "talk.client.create", params_json, handle_talk_create); + talk_diag(talk, "create_rpc", "end", err); free(params_json); return err; } @@ -572,30 +607,35 @@ static void handle_talk_config( { (void)node; talk_signaling_t *talk = user_ctx; + int diagnostic_result = 0; portENTER_CRITICAL(&talk->state_lock); bool stopped = talk->stopped; portEXIT_CRITICAL(&talk->state_lock); + talk_diag(talk, "config_callback", stopped ? "canceled" : "begin", 0); if (stopped) goto done; if (!result->ok) { - signal_failed(talk, ESP_OPENCLAW_TALK_SETUP_FAILED, - "Talk configuration lookup failed", result->error_code); + diagnostic_result = ESP_FAIL; + talk_diag(talk, "config_rpc", "end", ESP_FAIL); + signal_failed(talk, ESP_OPENCLAW_TALK_SETUP_FAILED); goto done; } /* Freeze the selected key before create; close must not rediscover * a new owner after configuration changes or cancellation. */ talk->session_key = resolve_default_session_key(result->payload_json); if (talk->session_key == NULL) { - signal_failed(talk, ESP_OPENCLAW_TALK_SETUP_FAILED, - "Talk routing configuration is invalid", NULL); + diagnostic_result = ESP_FAIL; + talk_diag(talk, "config_routing", "end", ESP_FAIL); + signal_failed(talk, ESP_OPENCLAW_TALK_SETUP_FAILED); goto done; } esp_err_t err = request_talk_create(talk); if (err != ESP_OK) { - signal_failed(talk, ESP_OPENCLAW_TALK_SETUP_FAILED, - "Talk create request failed", esp_err_to_name(err)); + diagnostic_result = err; + signal_failed(talk, ESP_OPENCLAW_TALK_SETUP_FAILED); } done: + talk_diag(talk, "config_callback", stopped ? "canceled" : "end", diagnostic_result); release_talk_signaling(talk); } @@ -662,9 +702,11 @@ static int start_prepared(talk_signaling_t *talk, esp_peer_signaling_cfg_t *cfg, portEXIT_CRITICAL(&talk->state_lock); if (!valid) return ESP_PEER_ERR_FAIL; + talk_diag(talk, "signaling_start", "begin", 0); esp_err_t err = talk->session_key != NULL ? request_talk_create(talk) : request_talk_rpc(talk, "talk.config", "{\"includeSecrets\":false}", handle_talk_config); + talk_diag(talk, "signaling_start", "end", err); if (err != ESP_OK) { release_talk_signaling(talk); return ESP_PEER_ERR_FAIL; @@ -747,11 +789,15 @@ static esp_err_t http_event_handler(esp_http_client_event_t *event) } if (response->len + (size_t)event->data_len > MAX_SDP_RESPONSE_BYTES) { response->failed = true; + response->failure_stage = "response_size"; + response->failure_error = ESP_FAIL; return ESP_FAIL; } char *resized = realloc(response->data, response->len + (size_t)event->data_len + 1U); if (resized == NULL) { response->failed = true; + response->failure_stage = "response_alloc"; + response->failure_error = ESP_ERR_NO_MEM; return ESP_ERR_NO_MEM; } response->data = resized; @@ -790,34 +836,53 @@ static int exchange_sdp(talk_signaling_t *talk, const char *sdp) .crt_bundle_attach = esp_crt_bundle_attach, #endif }; + talk_diag(talk, "http_init", "begin", 0); esp_http_client_handle_t client = esp_http_client_init(&config); + talk_diag(talk, "http_init", "end", client != NULL ? 0 : ESP_PEER_ERR_NO_MEM); if (client == NULL) { return ESP_PEER_ERR_NO_MEM; } - esp_http_client_set_header(client, "Content-Type", "application/sdp"); + esp_err_t header_error = esp_http_client_set_header(client, "Content-Type", "application/sdp"); + talk_diag(talk, "http_content_type", "end", header_error); size_t auth_len = strlen("Bearer ") + strlen(talk->client_secret) + 1U; char *authorization = malloc(auth_len); if (authorization == NULL) { + talk_diag(talk, "http_auth_alloc", "end", ESP_PEER_ERR_NO_MEM); esp_http_client_cleanup(client); return ESP_PEER_ERR_NO_MEM; } snprintf(authorization, auth_len, "Bearer %s", talk->client_secret); - esp_http_client_set_header(client, "Authorization", authorization); + header_error = esp_http_client_set_header(client, "Authorization", authorization); + talk_diag(talk, "http_authorization", "end", header_error); + header_error = ESP_OK; for (size_t i = 0; i < talk->offer_header_count; ++i) { - esp_http_client_set_header( + esp_err_t result = esp_http_client_set_header( client, talk->offer_headers[i].name, talk->offer_headers[i].value); + if (header_error == ESP_OK) header_error = result; } - esp_http_client_set_post_field(client, sdp, (int)strlen(sdp)); + talk_diag(talk, "http_offer_headers", "end", header_error); + esp_err_t post_error = esp_http_client_set_post_field(client, sdp, (int)strlen(sdp)); + talk_diag(talk, "http_post", "end", post_error); + talk_diag(talk, "http_perform", "begin", 0); esp_err_t err = esp_http_client_perform(client); + if (response.failed) talk_diag(talk, response.failure_stage, "end", response.failure_error); + talk_diag(talk, "http_perform", "end", err); int status = esp_http_client_get_status_code(client); + ESP_LOGI(TAG, "talk_rtc_http status=%d response_failed=%d response_present=%d", + status, response.failed, response.data != NULL); free(authorization); esp_http_client_cleanup(client); if (err != ESP_OK || response.failed || status < 200 || status >= 300 || response.data == NULL || response.len < 3 || strncmp(response.data, "v=0", 3) != 0) { - ESP_LOGE(TAG, "SDP exchange failed: status=%d err=%s", status, esp_err_to_name(err)); + const char *stage = err != ESP_OK ? "http_perform" : + response.failed ? response.failure_stage : + status < 200 || status >= 300 ? "http_status" : + response.data == NULL ? "response_missing" : + response.len < 3 ? "response_short" : "response_malformed"; + talk_diag(talk, stage, "end", ESP_PEER_ERR_FAIL); free(response.data); return ESP_PEER_ERR_FAIL; } @@ -829,9 +894,13 @@ static int exchange_sdp(talk_signaling_t *talk, const char *sdp) }; int callback_result = ESP_PEER_ERR_NONE; if (dispatch_enter(talk)) { - if (talk->signaling.on_msg != NULL) callback_result = talk->signaling.on_msg(&answer, talk->signaling.ctx); + if (talk->signaling.on_msg != NULL) { + talk_diag(talk, "answer_callback", "begin", 0); + callback_result = talk->signaling.on_msg(&answer, talk->signaling.ctx); + talk_diag(talk, "answer_callback", "end", callback_result); + } else talk_diag(talk, "answer_callback", "absent", 0); dispatch_exit(talk); - } + } else talk_diag(talk, "answer_callback", "canceled", 0); free(response.data); return callback_result; } diff --git a/components/esp-openclaw-talk/tests/host/esp_log.h b/components/esp-openclaw-talk/tests/host/esp_log.h index 9c50e04..a27952a 100644 --- a/components/esp-openclaw-talk/tests/host/esp_log.h +++ b/components/esp-openclaw-talk/tests/host/esp_log.h @@ -2,4 +2,7 @@ void talk_host_log_error(const char *tag, const char *format, ...) __attribute__((format(printf, 2, 3))); +void talk_host_log_info(const char *tag, const char *format, ...) + __attribute__((format(printf, 2, 3))); #define ESP_LOGE(...) talk_host_log_error(__VA_ARGS__) +#define ESP_LOGI(...) talk_host_log_info(__VA_ARGS__) diff --git a/components/esp-openclaw-talk/tests/host/talk_host_runner.c b/components/esp-openclaw-talk/tests/host/talk_host_runner.c index 7e1f9a6..94468cd 100644 --- a/components/esp-openclaw-talk/tests/host/talk_host_runner.c +++ b/components/esp-openclaw-talk/tests/host/talk_host_runner.c @@ -16,8 +16,25 @@ static size_t test_count; _Thread_local unsigned talk_host_critical_depth; unsigned talk_host_error_count; +void talk_host_log_info(const char *tag, const char *format, ...) +{ + (void)tag; + TEST_ASSERT_EQUAL_MESSAGE(0, talk_host_critical_depth, "Diagnostic logging outside locks"); + char line[1024]; + va_list args; + va_start(args, format); + int length = vsnprintf(line, sizeof(line), format, args); + va_end(args); + TEST_ASSERT_TRUE(length >= 0 && (size_t)length < sizeof(line)); + TEST_ASSERT_NULL(strstr(line, "broker-token")); + TEST_ASSERT_NULL(strstr(line, "voice-1")); + TEST_ASSERT_NULL(strstr(line, "gateway.example")); + TEST_ASSERT_NULL(strstr(line, "SECRET_ERROR_CANARY")); +} + void talk_host_log_error(const char *tag, const char *format, ...) { + TEST_ASSERT_EQUAL_MESSAGE(0, talk_host_critical_depth, "Diagnostic logging outside locks"); ++talk_host_error_count; va_list args; va_start(args, format); diff --git a/components/esp-openclaw-talk/tests/run_threaded_tests.py b/components/esp-openclaw-talk/tests/run_threaded_tests.py index 0882417..bdc272b 100644 --- a/components/esp-openclaw-talk/tests/run_threaded_tests.py +++ b/components/esp-openclaw-talk/tests/run_threaded_tests.py @@ -7,7 +7,13 @@ import tempfile CASES = ["ice-drain", "connected-drain", "failure-drain", "answer-drain", "late-http", - "late-create", "late-config", "late-failure", "submission-failure", "canceled-start", "lifecycle-isolation", "admission-race"] + "late-create", "late-config", "late-failure", "submission-failure", "canceled-start", "lifecycle-isolation", "admission-race", + "callback-diagnostics"] +CASES += [f"diagnostic-{stage}" for stage in ( + "http_init", "http_content_type", "http_post", "http_perform", "http_status", + "response_size", "response_alloc", "response_missing", "response_short", + "response_malformed", "answer_callback", +)] def main(): diff --git a/components/esp-openclaw-talk/tests/test_talk_threaded.c b/components/esp-openclaw-talk/tests/test_talk_threaded.c index 1e61f22..e837abc 100644 --- a/components/esp-openclaw-talk/tests/test_talk_threaded.c +++ b/components/esp-openclaw-talk/tests/test_talk_threaded.c @@ -3,10 +3,14 @@ #include #include #include +#include #include #include #include +static void *fixture_realloc(void *pointer, size_t size); +#define realloc fixture_realloc #include "../src/esp_openclaw_talk.c" +#undef realloc _Thread_local unsigned talk_host_critical_depth; @@ -23,6 +27,16 @@ static atomic_bool closed; static bool reject_submission; static bool reply_ok = true; static bool use_config; +static const char *http_case = ""; +static unsigned http_headers, http_posts, http_performs, http_cleanups; +static int answer_result; +static int ice_result, connected_result; +static struct { + char stage[40], phase[16]; + int error, first; +} diagnostic_records[64]; +static size_t diagnostic_count; +static pthread_mutex_t diagnostic_lock = PTHREAD_MUTEX_INITIALIZER; static unsigned config_count, create_count; static char close_key[257], close_voice[129]; static esp_openclaw_node_handle_t close_node; @@ -37,6 +51,21 @@ static const esp_peer_signaling_impl_t *implementation; struct callback_context { unsigned magic; }; static struct callback_context *callback_context; +static void *fixture_realloc(void *pointer, size_t size) +{ + return strcmp(http_case, "response_alloc") == 0 ? NULL : realloc(pointer, size); +} + +static bool diagnostic_has(const char *stage, const char *phase, int error, int first) +{ + for (size_t i = 0; i < diagnostic_count; ++i) { + if (strcmp(diagnostic_records[i].stage, stage) == 0 && + strcmp(diagnostic_records[i].phase, phase) == 0 && + diagnostic_records[i].error == error && diagnostic_records[i].first == first) return true; + } + return false; +} + static void pause_here(enum pause_point point) { if (pause_at != point) return; @@ -71,14 +100,14 @@ static int on_ice(esp_peer_signaling_ice_info_t *info, void *ctx) atomic_fetch_add(&ice_count, 1); pause_here(ICE); touch_context(ctx); - return 0; + return ice_result; } static int on_connected(void *ctx) { atomic_fetch_add(&connected_count, 1); pause_here(CONNECTED); touch_context(ctx); - return 0; + return connected_result; } static int on_close(void *ctx) { @@ -92,7 +121,7 @@ static int on_answer(esp_peer_signaling_msg_t *msg, void *ctx) atomic_fetch_add(&answer_count, 1); pause_here(ANSWER); touch_context(ctx); - return 0; + return answer_result; } static void on_failure(esp_openclaw_talk_setup_result_t result, void *ctx) { @@ -107,8 +136,42 @@ static void on_terminal(void *ctx) atomic_fetch_add(&terminal_count, 1); } +static void capture_diagnostic(const char *format, va_list args) +{ + assert(talk_host_critical_depth == 0); + char line[1024]; + int length = vsnprintf(line, sizeof(line), format, args); + assert(length >= 0 && (size_t)length < sizeof(line)); + const char *canaries[] = {"gateway.example", "synthetic", "voice-a", "agent:fixture", + "SDP_CANARY", "HEADER_CANARY", "SECRET_ERROR_CANARY"}; + for (size_t i = 0; i < sizeof(canaries) / sizeof(*canaries); ++i) assert(strstr(line, canaries[i]) == NULL); + if (strncmp(line, "talk_rtc_diag ", 14) != 0) return; + assert(pthread_mutex_lock(&diagnostic_lock) == 0); + assert(diagnostic_count < sizeof(diagnostic_records) / sizeof(*diagnostic_records)); + size_t i = diagnostic_count++; + int consumed = 0; + assert(sscanf(line, "talk_rtc_diag stage=%39s phase=%15s error=%d first_failure=%d%n", + diagnostic_records[i].stage, diagnostic_records[i].phase, + &diagnostic_records[i].error, &diagnostic_records[i].first, &consumed) == 4); + assert(line[consumed] == '\0'); + assert(pthread_mutex_unlock(&diagnostic_lock) == 0); +} +void talk_host_log_info(const char *tag, const char *format, ...) +{ + (void)tag; + va_list args; + va_start(args, format); + capture_diagnostic(format, args); + va_end(args); +} void talk_host_log_error(const char *tag, const char *format, ...) -{ (void)tag; (void)format; } +{ + (void)tag; + va_list args; + va_start(args, format); + capture_diagnostic(format, args); + va_end(args); +} const char *esp_err_to_name(esp_err_t error) { (void)error; return "synthetic failure"; } @@ -147,7 +210,7 @@ static void *reply_thread(void *arg) pending.cb = NULL; const esp_openclaw_node_gateway_result_t result = { .ok = reply_ok, - .error_code = reply_ok ? NULL : "UNAVAILABLE", + .error_code = reply_ok ? NULL : "SECRET_ERROR_CANARY", .payload_json = use_config ? "{\"config\":{\"talk\":{\"agentId\":\"fixture\"}}}" : "{\"transport\":\"webrtc\",\"offerUrl\":\"/offer\",\"clientSecret\":\"synthetic\"," "\"voiceSessionId\":\"voice-a\",\"clientControl\":{\"owner\":\"gateway\"}}", @@ -185,6 +248,7 @@ static void wait_for_drain(void) static void prepare(bool automatic) { + diagnostic_count = 0; callback_context = malloc(sizeof(*callback_context)); assert(callback_context != NULL); callback_context->magic = 0xcafef00d; @@ -233,7 +297,10 @@ static void race_dispatch(enum pause_point point) assert(pthread_join(dispatch, NULL) == 0); assert(atomic_load(&close_count) == 0); if (point == ICE || point == FAILURE || point == REPLY) assert(atomic_load(&connected_count) == 0); - if (point == HTTP) assert(atomic_load(&answer_count) == 0); + if (point == HTTP) { + assert(atomic_load(&answer_count) == 0); + assert(diagnostic_has("answer_callback", "canceled", 0, 0)); + } if (point == REPLY && reply_ok) { assert(atomic_load(&ice_count) == 0); assert(strcmp(close_voice, "voice-a") == 0); @@ -335,32 +402,93 @@ static void admission_race(void) struct esp_http_client { esp_http_client_config_t config; }; esp_http_client_handle_t esp_http_client_init(const esp_http_client_config_t *config) { + if (strcmp(http_case, "http_init") == 0) return NULL; esp_http_client_handle_t client = malloc(sizeof(*client)); assert(client != NULL); client->config = *config; return client; } esp_err_t esp_http_client_set_header(esp_http_client_handle_t client, const char *key, const char *value) -{ (void)client; (void)key; (void)value; return ESP_OK; } +{ + (void)client; (void)value; + ++http_headers; + return strcmp(http_case, "http_content_type") == 0 && strcmp(key, "Content-Type") == 0 ? ESP_FAIL : ESP_OK; +} esp_err_t esp_http_client_set_post_field(esp_http_client_handle_t client, const char *data, int length) -{ (void)client; (void)data; (void)length; return ESP_OK; } +{ + (void)client; + assert(data != NULL && length > 0); + ++http_posts; + return strcmp(http_case, "http_post") == 0 ? ESP_FAIL : ESP_OK; +} esp_err_t esp_http_client_perform(esp_http_client_handle_t client) { + ++http_performs; pause_here(HTTP); + if (strcmp(http_case, "http_perform") == 0) return ESP_FAIL; + if (strcmp(http_case, "response_missing") == 0) return ESP_OK; + const char *body = strcmp(http_case, "response_malformed") == 0 ? "SDP_CANARY" : + strcmp(http_case, "response_short") == 0 ? "v=" : "v=0"; esp_http_client_event_t event = { - .event_id = HTTP_EVENT_ON_DATA, .user_data = client->config.user_data, .data = "v=0", .data_len = 3, + .event_id = HTTP_EVENT_ON_DATA, .user_data = client->config.user_data, + .data = (char *)body, .data_len = (int)strlen(body), }; + if (strcmp(http_case, "response_size") == 0) event.data_len = MAX_SDP_RESPONSE_BYTES + 1; return client->config.event_handler(&event); } int esp_http_client_get_status_code(esp_http_client_handle_t client) -{ (void)client; return 200; } +{ (void)client; return strcmp(http_case, "http_status") == 0 ? 503 : 200; } esp_err_t esp_http_client_cleanup(esp_http_client_handle_t client) -{ free(client); return ESP_OK; } +{ ++http_cleanups; free(client); return ESP_FAIL; } + +static void http_diagnostic_case(const char *name) +{ + http_case = name; + prepare(false); + assert(start() == ESP_PEER_ERR_NONE); + reply_thread(NULL); + if (strcmp(name, "answer_callback") == 0) answer_result = -7; + esp_peer_signaling_msg_t msg = { + .type = ESP_PEER_SIGNALING_MSG_SDP, .data = (uint8_t *)"v=0\r\nSDP_CANARY", .size = 15, + }; + int result = implementation->send_msg(signaling, &msg); + bool ignored = strcmp(name, "http_content_type") == 0 || strcmp(name, "http_post") == 0; + bool init = strcmp(name, "http_init") == 0; + bool callback = strcmp(name, "answer_callback") == 0; + assert(result == (init ? ESP_PEER_ERR_NO_MEM : callback ? -7 : ignored ? 0 : ESP_PEER_ERR_FAIL)); + int error = init ? ESP_PEER_ERR_NO_MEM : callback ? -7 : + strcmp(name, "response_alloc") == 0 ? ESP_ERR_NO_MEM : + ignored || strcmp(name, "http_perform") == 0 || strcmp(name, "response_size") == 0 ? ESP_FAIL : ESP_PEER_ERR_FAIL; + assert(diagnostic_has(name, "end", error, 1)); + unsigned first_count = 0; + for (size_t i = 0; i < diagnostic_count; ++i) first_count += diagnostic_records[i].first != 0; + assert(first_count == 1); + assert(http_headers == (init ? 0U : 2U)); + assert(http_posts == (init ? 0U : 1U) && http_performs == http_posts && http_cleanups == http_posts); + assert(atomic_load(&answer_count) == (ignored || callback ? 1U : 0U)); + close_thread(NULL); +} + +static void callback_diagnostics(void) +{ + prepare(false); + assert(start() == ESP_PEER_ERR_NONE); + ice_result = -5; + connected_result = -6; + reply_thread(NULL); + assert(atomic_load(&ice_count) == 1 && atomic_load(&connected_count) == 1); + assert(atomic_load(&failure_count) == 0); + assert(diagnostic_has("ice_callback", "end", -5, 1)); + assert(diagnostic_has("signaling_callback", "end", -6, 0)); + close_thread(NULL); +} int main(int argc, char **argv) { assert(argc == 2); - if (strcmp(argv[1], "ice-drain") == 0) race_dispatch(ICE); + if (strcmp(argv[1], "callback-diagnostics") == 0) callback_diagnostics(); + else if (strncmp(argv[1], "diagnostic-", 11) == 0) http_diagnostic_case(argv[1] + 11); + else if (strcmp(argv[1], "ice-drain") == 0) race_dispatch(ICE); else if (strcmp(argv[1], "connected-drain") == 0) race_dispatch(CONNECTED); else if (strcmp(argv[1], "failure-drain") == 0) race_dispatch(FAILURE); else if (strcmp(argv[1], "answer-drain") == 0) race_dispatch(ANSWER); diff --git a/examples/m5stack-tab5-room-node/README.md b/examples/m5stack-tab5-room-node/README.md index 4866653..e3d1df0 100644 --- a/examples/m5stack-tab5-room-node/README.md +++ b/examples/m5stack-tab5-room-node/README.md @@ -265,6 +265,23 @@ mislabeling the front image. openclaw nodes camera snap --node --facing front ``` +### Media stage diagnostics + +The capture owner emits one `camera_capture_diag` failure with a fixed `stage`, +`domain` and numeric `error`. `esp` is the BSP result; `errno` is captured +immediately after the failed syscall; `validation` uses zero rather than stale +errno. Existing numeric format, stride and length rejection details remain. +The single `dqbuf_begin` marker precedes the first blocking dequeue; its +presence without a terminal record does not prove capture success or failure. +There is no per-frame trace, new timeout, capture retry or format fallback. +The camera RPC's existing error code and cleanup behavior are unchanged. + +Talk uses the [component's fixed-field stage diagnostics](../../components/esp-openclaw-talk/README.md#stage-diagnostics), +with room-generation startup/peer/teardown records and existing audio-counter +snapshots. They separate local progress from RTC/media success and contain no +SDP, broker credentials or session identifiers. Native builds and synthetic +fixtures are not physical camera, RTC or audio qualification. + | Surface | Commands and bounds | | --- | --- | | Device | `device.info`, `device.status`, `wifi.status` | diff --git a/examples/m5stack-tab5-room-node/components/tab5_room_board/tab5_room_board.c b/examples/m5stack-tab5-room-node/components/tab5_room_board/tab5_room_board.c index a046ed6..fb43afe 100644 --- a/examples/m5stack-tab5-room-node/components/tab5_room_board/tab5_room_board.c +++ b/examples/m5stack-tab5-room-node/components/tab5_room_board/tab5_room_board.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -716,17 +717,34 @@ static void close_camera_frame(camera_frame_t *camera) static esp_err_t capture_rgb565_frame(int delay_ms, camera_frame_t *camera) { static bool camera_started; + const char *stage = "bsp_start"; + const char *domain = "errno"; + int failure_error = 0; + esp_err_t result = ESP_FAIL; + bool needs_cleanup = false; if (!camera_started) { - ESP_RETURN_ON_ERROR(bsp_camera_start(NULL), TAG, "camera start"); + esp_err_t start_result = bsp_camera_start(NULL); + if (start_result != ESP_OK) { + failure_error = start_result; + domain = "esp"; + result = start_result; + goto fail; + } camera_started = true; } memset(camera, 0, sizeof(*camera)); camera->fd = open(BSP_CAMERA_DEVICE, O_RDONLY); - if (camera->fd < 0) return ESP_FAIL; + if (camera->fd < 0) { + failure_error = errno; + stage = "open"; + goto fail; + } + needs_cleanup = true; struct v4l2_format format = {.type = V4L2_BUF_TYPE_VIDEO_CAPTURE}; if (ioctl(camera->fd, VIDIOC_G_FMT, &format) != 0) { - ESP_LOGE(TAG, "VIDIOC_G_FMT failed"); + failure_error = errno; + stage = "g_fmt_initial"; goto fail; } if (format.fmt.pix.width != CAMERA_SENSOR_WIDTH || @@ -738,6 +756,8 @@ static esp_err_t capture_rgb565_frame(int delay_ms, camera_frame_t *camera) (unsigned long)format.fmt.pix.height, CAMERA_SENSOR_WIDTH, CAMERA_SENSOR_HEIGHT); + stage = "validate_default"; + domain = "validation"; goto fail; } /* maxWidth constrains the post-rotation JPEG, never the SC202CS mode. */ @@ -750,13 +770,15 @@ static esp_err_t capture_rgb565_frame(int delay_ms, camera_frame_t *camera) }, }; if (ioctl(camera->fd, VIDIOC_S_FMT, &requested) != 0) { - ESP_LOGE(TAG, "VIDIOC_S_FMT RGB565 %ux%u failed", CAMERA_SENSOR_WIDTH, CAMERA_SENSOR_HEIGHT); + failure_error = errno; + stage = "s_fmt_rgb565"; goto fail; } memset(&format, 0, sizeof(format)); format.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; if (ioctl(camera->fd, VIDIOC_G_FMT, &format) != 0) { - ESP_LOGE(TAG, "VIDIOC_G_FMT after RGB565 negotiation failed"); + failure_error = errno; + stage = "g_fmt_rgb565"; goto fail; } uint32_t expected_stride = CAMERA_SENSOR_WIDTH * sizeof(uint16_t); @@ -776,6 +798,8 @@ static esp_err_t capture_rgb565_frame(int delay_ms, camera_frame_t *camera) CAMERA_SENSOR_WIDTH, CAMERA_SENSOR_HEIGHT, (unsigned long)expected_stride); + stage = "validate_format"; + domain = "validation"; goto fail; } camera->width = format.fmt.pix.width; @@ -789,42 +813,78 @@ static esp_err_t capture_rgb565_frame(int delay_ms, camera_frame_t *camera) .type = V4L2_BUF_TYPE_VIDEO_CAPTURE, .memory = V4L2_MEMORY_MMAP, }; - if (ioctl(camera->fd, VIDIOC_REQBUFS, &request) != 0 || request.count < 2) goto fail; + if (ioctl(camera->fd, VIDIOC_REQBUFS, &request) != 0) { + failure_error = errno; + stage = "reqbufs"; + goto fail; + } + if (request.count < 2) { + stage = "validate_count"; + domain = "validation"; + goto fail; + } for (uint32_t i = 0; i < 2; ++i) { struct v4l2_buffer buffer = { .type = V4L2_BUF_TYPE_VIDEO_CAPTURE, .memory = V4L2_MEMORY_MMAP, .index = i, }; - if (ioctl(camera->fd, VIDIOC_QUERYBUF, &buffer) != 0) goto fail; + if (ioctl(camera->fd, VIDIOC_QUERYBUF, &buffer) != 0) { + failure_error = errno; + stage = "querybuf"; + goto fail; + } if (buffer.length < (size_t)camera->stride * camera->height) { ESP_LOGE(TAG, "camera MMAP buffer is too short: %lu", (unsigned long)buffer.length); + stage = "validate_buffer_length"; + domain = "validation"; goto fail; } camera->lengths[i] = buffer.length; camera->buffers[i] = mmap(NULL, buffer.length, PROT_READ | PROT_WRITE, MAP_SHARED, camera->fd, buffer.m.offset); if (camera->buffers[i] == MAP_FAILED) { + failure_error = errno; + stage = "mmap"; camera->buffers[i] = NULL; goto fail; } - if (ioctl(camera->fd, VIDIOC_QBUF, &buffer) != 0) goto fail; + if (ioctl(camera->fd, VIDIOC_QBUF, &buffer) != 0) { + failure_error = errno; + stage = "qbuf"; + goto fail; + } } int type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - if (ioctl(camera->fd, VIDIOC_STREAMON, &type) != 0) goto fail; + if (ioctl(camera->fd, VIDIOC_STREAMON, &type) != 0) { + failure_error = errno; + stage = "streamon"; + goto fail; + } + ESP_LOGI(TAG, "camera_capture_diag stage=dqbuf_begin domain=none error=0"); int64_t deadline_us = esp_timer_get_time() + (int64_t)delay_ms * 1000; for (;;) { camera->frame = (struct v4l2_buffer){ .type = V4L2_BUF_TYPE_VIDEO_CAPTURE, .memory = V4L2_MEMORY_MMAP, }; - if (ioctl(camera->fd, VIDIOC_DQBUF, &camera->frame) != 0 || - camera->frame.index >= 2) { + if (ioctl(camera->fd, VIDIOC_DQBUF, &camera->frame) != 0) { + failure_error = errno; + stage = "dqbuf"; + goto fail; + } + if (camera->frame.index >= 2) { + stage = "validate_index"; + domain = "validation"; goto fail; } if (esp_timer_get_time() >= deadline_us) break; /* Keep the two-buffer pipeline moving while delayMs elapses. Sleeping * here lets both buffers fill and stalls the sensor before capture. */ - if (ioctl(camera->fd, VIDIOC_QBUF, &camera->frame) != 0) goto fail; + if (ioctl(camera->fd, VIDIOC_QBUF, &camera->frame) != 0) { + failure_error = errno; + stage = "requeue"; + goto fail; + } } size_t frame_size = (size_t)camera->stride * camera->height; if (camera->frame.bytesused != 0 && camera->frame.bytesused < frame_size) { @@ -833,12 +893,15 @@ static esp_err_t capture_rgb565_frame(int delay_ms, camera_frame_t *camera) "camera frame is too short: %lu bytes (expected at least %lu)", (unsigned long)camera->frame.bytesused, (unsigned long)frame_size); + stage = "validate_frame_length"; + domain = "validation"; goto fail; } return ESP_OK; fail: - close_camera_frame(camera); - return ESP_FAIL; + ESP_LOGE(TAG, "camera_capture_diag stage=%s domain=%s error=%d", stage, domain, failure_error); + if (needs_cleanup) close_camera_frame(camera); + return result; } typedef struct { diff --git a/scripts/tests/fixtures/test_tab5_camera_capture.c b/scripts/tests/fixtures/test_tab5_camera_capture.c new file mode 100644 index 0000000..cb3d8a6 --- /dev/null +++ b/scripts/tests/fixtures/test_tab5_camera_capture.c @@ -0,0 +1,310 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define ESP_OK 0 +#define ESP_FAIL -1 +#define BSP_FAILURE 0x105 +#define TAG "camera-fixture" +#define CAMERA_SENSOR_WIDTH 1280U +#define CAMERA_SENSOR_HEIGHT 720U +#define BSP_CAMERA_DEVICE "synthetic-camera-path-canary" +#define O_RDONLY 0 +#define PROT_READ 1 +#define PROT_WRITE 2 +#define MAP_SHARED 1 +/* ESP-IDF's VFS mmap failure sentinel is NULL, not the POSIX host sentinel. */ +#define MAP_FAILED NULL +#define V4L2_BUF_TYPE_VIDEO_CAPTURE 1 +#define V4L2_MEMORY_MMAP 1 +#define V4L2_PIX_FMT_RGB565 0x50424752U +#define V4L2_PIX_FMT_RGB565X 0x52424752U +#define FRAME_SIZE (CAMERA_SENSOR_WIDTH * CAMERA_SENSOR_HEIGHT * 2U) +enum { VIDIOC_G_FMT, VIDIOC_S_FMT, VIDIOC_REQBUFS, VIDIOC_QUERYBUF, + VIDIOC_QBUF, VIDIOC_STREAMON, VIDIOC_DQBUF, VIDIOC_STREAMOFF }; +typedef int esp_err_t; +struct v4l2_format { + uint32_t type; + union { struct { uint32_t width, height, pixelformat, bytesperline; } pix; } fmt; +}; +struct v4l2_requestbuffers { uint32_t count, type, memory; }; +struct v4l2_buffer { + uint32_t type, memory, index, length, bytesused; + union { uint32_t offset; } m; +}; +typedef struct { char stage[40], domain[16]; int error; } record_t; +static record_t records[2]; +static unsigned record_count, log_count, bsp_calls, gfmt_calls, maps, unmaps; +static unsigned closes, streamoffs, dequeues, initial_queues, requeues; +static int64_t clock_us; +static uint8_t mapped[2][8]; +static bool live_mapping[2]; +static const char *scenario; +static char validation_detail[512]; + +static bool is(const char *name) { return strcmp(scenario, name) == 0; } + +static void capture_log(const char *format, ...) +{ + char line[512]; + va_list args; + va_start(args, format); + int length = vsnprintf(line, sizeof(line), format, args); + va_end(args); + assert(length >= 0 && (size_t)length < sizeof(line)); + assert(strstr(line, BSP_CAMERA_DEVICE) == NULL); + ++log_count; + if (strncmp(line, "camera_capture_diag ", 20) == 0) { + assert(record_count < 2); + record_t *record = &records[record_count++]; + int consumed = 0; + assert(sscanf(line, "camera_capture_diag stage=%39s domain=%15s error=%d%n", + record->stage, record->domain, &record->error, &consumed) == 3); + assert(line[consumed] == '\0'); + } else { + memcpy(validation_detail, line, (size_t)length + 1); + } + errno = EBUSY; +} +#define ESP_LOGE(tag, ...) do { (void)(tag); capture_log(__VA_ARGS__); } while (0) +#define ESP_LOGI(tag, ...) do { (void)(tag); capture_log(__VA_ARGS__); } while (0) +#define ESP_RETURN_ON_ERROR(call, tag, ...) do { \ + esp_err_t error = (call); \ + if (error != ESP_OK) { ESP_LOGE(tag, __VA_ARGS__); return error; } \ +} while (0) + +static esp_err_t bsp_camera_start(const void *config) +{ + assert(config == NULL); + ++bsp_calls; + errno = ESTALE; + return is("bsp") || is("bsp-retry") ? BSP_FAILURE : ESP_OK; +} +static int camera_open(const char *path, int flags) +{ + assert(strcmp(path, BSP_CAMERA_DEVICE) == 0 && flags == O_RDONLY); + errno = EIO; + return is("open") || is("open-cache") ? -1 : 7; +} +static int camera_close(int fd) +{ + assert(fd == 7); + ++closes; + errno = EBADF; + return -1; +} +static void *camera_mmap(void *address, size_t length, int protection, + int flags, int fd, uint32_t offset) +{ + assert(address == NULL && length == FRAME_SIZE && fd == 7 && offset < 2); + assert(protection == (PROT_READ | PROT_WRITE) && flags == MAP_SHARED); + if ((offset == 0 && is("mmap0")) || (offset == 1 && is("mmap1"))) { + errno = EIO; + return MAP_FAILED; + } + assert(!live_mapping[offset]); + live_mapping[offset] = true; + ++maps; + return mapped[offset]; +} +static int camera_munmap(void *address, size_t length) +{ + assert(length == FRAME_SIZE); + unsigned index = address == mapped[0] ? 0 : 1; + assert(address == mapped[index] && live_mapping[index]); + live_mapping[index] = false; + ++unmaps; + errno = EBADF; + return -1; +} +static int camera_ioctl(int fd, int request, void *argument) +{ + assert(fd == 7); + errno = ESTALE; + if (request == VIDIOC_STREAMOFF) { + assert(*(int *)argument == V4L2_BUF_TYPE_VIDEO_CAPTURE); + ++streamoffs; + errno = EBADF; + return -1; + } + if (request == VIDIOC_G_FMT) { + struct v4l2_format *format = argument; + ++gfmt_calls; + if ((gfmt_calls == 1 && is("gfmt")) || (gfmt_calls == 2 && is("gfmt-after"))) goto failure; + format->fmt.pix.width = CAMERA_SENSOR_WIDTH; + format->fmt.pix.height = CAMERA_SENSOR_HEIGHT; + format->fmt.pix.pixelformat = is("rgb565x") ? V4L2_PIX_FMT_RGB565X : V4L2_PIX_FMT_RGB565; + format->fmt.pix.bytesperline = is("explicit-stride") ? CAMERA_SENSOR_WIDTH * 2 : 0; + if (gfmt_calls == 1 && is("default-size")) format->fmt.pix.width = 640; + if (gfmt_calls == 2) { + if (is("negotiated-size")) format->fmt.pix.height = 480; + if (is("negotiated-format")) format->fmt.pix.pixelformat = 0; + if (is("negotiated-stride")) format->fmt.pix.bytesperline = 1; + } + } else if (request == VIDIOC_S_FMT) { + const struct v4l2_format *format = argument; + assert(format->fmt.pix.width == CAMERA_SENSOR_WIDTH); + assert(format->fmt.pix.height == CAMERA_SENSOR_HEIGHT); + assert(format->fmt.pix.pixelformat == V4L2_PIX_FMT_RGB565); + if (is("sfmt")) goto failure; + } else if (request == VIDIOC_REQBUFS) { + struct v4l2_requestbuffers *buffers = argument; + assert(buffers->count == 2 && buffers->memory == V4L2_MEMORY_MMAP); + if (is("reqbufs")) goto failure; + if (is("buffer-count")) buffers->count = 1; + } else if (request == VIDIOC_QUERYBUF) { + struct v4l2_buffer *buffer = argument; + assert(buffer->index < 2); + if ((buffer->index == 0 && is("query0")) || (buffer->index == 1 && is("query1"))) goto failure; + buffer->length = buffer->index == 1 && is("buffer-length") ? 1 : FRAME_SIZE; + buffer->m.offset = buffer->index; + } else if (request == VIDIOC_QBUF) { + const struct v4l2_buffer *buffer = argument; + assert(buffer->index < 2); + if (dequeues == 0) { + ++initial_queues; + if ((buffer->index == 0 && is("qbuf0")) || (buffer->index == 1 && is("qbuf1"))) goto failure; + } else { + ++requeues; + if (is("requeue")) goto failure; + } + } else if (request == VIDIOC_STREAMON) { + assert(*(int *)argument == V4L2_BUF_TYPE_VIDEO_CAPTURE); + assert(initial_queues == 2); + if (is("streamon")) goto failure; + } else if (request == VIDIOC_DQBUF) { + struct v4l2_buffer *buffer = argument; + ++dequeues; + if (is("dqbuf")) goto failure; + buffer->index = is("frame-index") ? 2 : (dequeues - 1) % 2; + buffer->bytesused = is("frame-length") ? 1 : is("zero-bytesused") ? 0 : FRAME_SIZE; + } else { + assert(false); + } + return 0; +failure: + errno = EIO; + return -1; +} +static int64_t esp_timer_get_time(void) +{ + int64_t current = clock_us; + clock_us += 1000; + return current; +} +#define open camera_open +#define close camera_close +#define mmap camera_mmap +#define munmap camera_munmap +#define ioctl camera_ioctl +#include "camera_capture_under_test.inc" + +typedef struct { + const char *scenario, *stage, *domain; + unsigned mapped_count, marker_count; + int result; +} failure_case_t; +static const failure_case_t failures[] = { + {"bsp", "bsp_start", "esp", 0, 0, BSP_FAILURE}, + {"open", "open", "errno", 0, 0, ESP_FAIL}, + {"gfmt", "g_fmt_initial", "errno", 0, 0, ESP_FAIL}, + {"default-size", "validate_default", "validation", 0, 0, ESP_FAIL}, + {"sfmt", "s_fmt_rgb565", "errno", 0, 0, ESP_FAIL}, + {"gfmt-after", "g_fmt_rgb565", "errno", 0, 0, ESP_FAIL}, + {"negotiated-size", "validate_format", "validation", 0, 0, ESP_FAIL}, + {"negotiated-format", "validate_format", "validation", 0, 0, ESP_FAIL}, + {"negotiated-stride", "validate_format", "validation", 0, 0, ESP_FAIL}, + {"reqbufs", "reqbufs", "errno", 0, 0, ESP_FAIL}, + {"buffer-count", "validate_count", "validation", 0, 0, ESP_FAIL}, + {"query0", "querybuf", "errno", 0, 0, ESP_FAIL}, + {"query1", "querybuf", "errno", 1, 0, ESP_FAIL}, + {"buffer-length", "validate_buffer_length", "validation", 1, 0, ESP_FAIL}, + {"mmap0", "mmap", "errno", 0, 0, ESP_FAIL}, + {"mmap1", "mmap", "errno", 1, 0, ESP_FAIL}, + {"qbuf0", "qbuf", "errno", 1, 0, ESP_FAIL}, + {"qbuf1", "qbuf", "errno", 2, 0, ESP_FAIL}, + {"streamon", "streamon", "errno", 2, 0, ESP_FAIL}, + {"dqbuf", "dqbuf", "errno", 2, 1, ESP_FAIL}, + {"frame-index", "validate_index", "validation", 2, 1, ESP_FAIL}, + {"requeue", "requeue", "errno", 2, 1, ESP_FAIL}, + {"frame-length", "validate_frame_length", "validation", 2, 1, ESP_FAIL}, +}; + +static void assert_marker(void) +{ + assert(strcmp(records[0].stage, "dqbuf_begin") == 0); + assert(strcmp(records[0].domain, "none") == 0 && records[0].error == 0); +} + +int main(int argc, char **argv) +{ + const struct rlimit no_core = {0, 0}; + assert(setrlimit(RLIMIT_CORE, &no_core) == 0); + assert(argc == 2); + scenario = argv[1]; + errno = EDOM; + camera_frame_t camera = {.fd = -1}; + int delay = is("delayed") ? 20 : is("requeue") ? 10 : 0; + esp_err_t result = capture_rgb565_frame(delay, &camera); + for (size_t i = 0; i < sizeof(failures) / sizeof(failures[0]); ++i) { + const failure_case_t *test = &failures[i]; + if (!is(test->scenario)) continue; + assert(result == test->result); + bool detail = is("default-size") || is("negotiated-size") || + is("negotiated-format") || is("negotiated-stride") || + is("buffer-length") || is("frame-length"); + assert(record_count == test->marker_count + 1); + assert(log_count == record_count + (detail ? 1U : 0U)); + if (is("default-size")) assert(strstr(validation_detail, "640x720") != NULL); + if (is("negotiated-size")) assert(strstr(validation_detail, "1280x480") != NULL); + if (is("negotiated-format")) assert(strstr(validation_detail, "fourcc=0x00000000") != NULL); + if (is("negotiated-stride")) assert(strstr(validation_detail, "stride=1 ") != NULL); + if (is("buffer-length")) assert(strstr(validation_detail, "short: 1") != NULL); + if (is("frame-length")) assert(strstr(validation_detail, "1 bytes") != NULL); + if (test->marker_count) assert_marker(); + const record_t *failure = &records[test->marker_count]; + assert(strcmp(failure->stage, test->stage) == 0); + assert(strcmp(failure->domain, test->domain) == 0); + assert(failure->error == (strcmp(test->domain, "errno") == 0 ? EIO : + strcmp(test->domain, "esp") == 0 ? BSP_FAILURE : 0)); + assert(maps == test->mapped_count && unmaps == maps); + assert(closes == (is("bsp") || is("open") ? 0U : 1U)); + assert(streamoffs == closes && camera.fd == -1); + return 0; + } + bool cache_case = is("bsp-retry") || is("open-cache") || is("success-cache"); + if (is("bsp-retry") || is("open-cache")) { + assert(result == (is("bsp-retry") ? BSP_FAILURE : ESP_FAIL)); + assert(closes == 0 && maps == 0); + } else { + assert(result == ESP_OK && camera.fd == 7); + assert(camera.width == CAMERA_SENSOR_WIDTH && camera.height == CAMERA_SENSOR_HEIGHT); + assert(camera.stride == CAMERA_SENSOR_WIDTH * 2); + assert(camera.format == (is("rgb565x") ? V4L2_PIX_FMT_RGB565X : V4L2_PIX_FMT_RGB565)); + assert(record_count == 1 && log_count == 1); + assert_marker(); + assert(maps == 2 && unmaps == 0 && closes == 0 && streamoffs == 0); + assert(dequeues == (is("delayed") ? 20U : 1U)); + assert(requeues + 1 == dequeues); + close_camera_frame(&camera); + assert(unmaps == 2 && closes == 1 && streamoffs == 1); + } + if (cache_case) { + unsigned expected_starts = is("bsp-retry") ? 2 : 1; + scenario = "success"; + record_count = log_count = gfmt_calls = maps = unmaps = closes = 0; + streamoffs = dequeues = initial_queues = requeues = 0; + assert(capture_rgb565_frame(0, &camera) == ESP_OK); + assert(bsp_calls == expected_starts && record_count == 1); + assert_marker(); + close_camera_frame(&camera); + assert(unmaps == 2 && closes == 1); + } + return 0; +} diff --git a/scripts/tests/test_tab5_camera_diagnostics.py b/scripts/tests/test_tab5_camera_diagnostics.py new file mode 100644 index 0000000..a465437 --- /dev/null +++ b/scripts/tests/test_tab5_camera_diagnostics.py @@ -0,0 +1,59 @@ +"""Execute the real capture owner against bounded V4L2 and logging stubs.""" + +import os +from pathlib import Path +import subprocess +import tempfile +import unittest + + +ROOT = Path(__file__).resolve().parents[2] +SOURCE = ROOT / "examples/m5stack-tab5-room-node/components/tab5_room_board/tab5_room_board.c" +FIXTURE = Path(__file__).parent / "fixtures/test_tab5_camera_capture.c" + + +class CameraCaptureDiagnosticsTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + source = Path(os.environ.get("TAB5_CAMERA_SOURCE", SOURCE)).read_text() + start = source.index("typedef struct {\n int fd;") + end = source.index("\ntypedef struct {", start + 1) + temporary = tempfile.TemporaryDirectory(prefix="tab5-camera-capture-") + cls.addClassCleanup(temporary.cleanup) + directory = Path(temporary.name) + (directory / "camera_capture_under_test.inc").write_text(source[start:end]) + cls.binary = directory / "camera-capture" + subprocess.run([ + os.environ.get("CC", "cc"), "-std=c11", "-Wall", "-Wextra", "-Werror", + "-fsanitize=address,undefined", "-fno-sanitize-recover=all", + "-I", str(directory), str(FIXTURE), "-o", str(cls.binary), + ], check=True) + + def run_case(self, scenario): + result = subprocess.run([str(self.binary), scenario], capture_output=True, text=True) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_failure_stage_errno_domain_and_partial_cleanup(self): + for scenario in ( + "bsp", "open", "gfmt", "default-size", "sfmt", "gfmt-after", + "negotiated-size", "negotiated-format", "negotiated-stride", + "reqbufs", "buffer-count", "query0", "query1", "buffer-length", + "mmap0", "mmap1", "qbuf0", "qbuf1", "streamon", "dqbuf", + "frame-index", "requeue", "frame-length", + ): + with self.subTest(scenario=scenario): + self.run_case(scenario) + + def test_success_formats_stride_and_bounded_loop_marker(self): + for scenario in ("success", "rgb565x", "explicit-stride", "zero-bytesused", "delayed"): + with self.subTest(scenario=scenario): + self.run_case(scenario) + + def test_existing_initialization_cache_policy(self): + for scenario in ("bsp-retry", "open-cache", "success-cache"): + with self.subTest(scenario=scenario): + self.run_case(scenario) + + +if __name__ == "__main__": + unittest.main() From 0116ddcd482125ef74d8be15d469cd8707d78c4c Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Wed, 9 Sep 2026 23:30:27 +0800 Subject: [PATCH 2/5] fix(tab5): align CSI input with ISP output --- .github/workflows/ci.yml | 11 +- examples/m5stack-tab5-room-node/README.md | 47 +++ .../esp-video/tab5-csi-post-isp-format.patch | 18 + scripts/package_firmware.py | 20 +- scripts/tab5_camera_compat.py | 193 +++++++++++ .../tests/fixtures/test_tab5_camera_format.c | 139 ++++++++ scripts/tests/test_package_firmware.py | 76 +++- scripts/tests/test_tab5_camera_compat.py | 327 ++++++++++++++++++ 8 files changed, 827 insertions(+), 4 deletions(-) create mode 100644 patches/esp-video/tab5-csi-post-isp-format.patch create mode 100644 scripts/tab5_camera_compat.py create mode 100644 scripts/tests/fixtures/test_tab5_camera_format.c create mode 100644 scripts/tests/test_tab5_camera_compat.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 301b822..3ae843f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -82,20 +82,29 @@ jobs: command: | nvs_diagnostics="" && sdio_diagnostics="" && + camera_compat="" && if [ "${{ matrix.name }}" = "m5stack-tab5-room-node" ]; then nvs_diagnostics="--nvs-diagnostics" && sdio_diagnostics="--sdio-diagnostics --sdio-psram-rx" && + camera_compat="--camera-compat" && python3 ../../scripts/idf_tab5_compat.py --idf-path "$IDF_PATH" --nvs-diagnostics fi && idf.py reconfigure && if [ "${{ matrix.name }}" = "m5stack-tab5-room-node" ]; then python3 ../../scripts/tab5_sdio_diagnostics.py \ --project-path . --component-path managed_components/espressif__esp_hosted --build-path build --psram-rx && + python3 ../../scripts/tab5_camera_compat.py --idf-path "$IDF_PATH" \ + --project-path . --component-path managed_components/espressif__esp_video --build-path build && python3 -m unittest discover -s ../../scripts/tests -p test_tab5_sdio_rx_psram.py -v && python3 -m unittest discover -s ../../scripts/tests -p test_tab5_sdio_diagnostics.py -v && + python3 -m unittest discover -s ../../scripts/tests -p test_tab5_camera_compat.py -v && python3 -m unittest discover -s ../../scripts/tests -p test_package_firmware.py -v fi && ninja -C build -j2 && + if [ "${{ matrix.name }}" = "m5stack-tab5-room-node" ]; then + python3 ../../scripts/tab5_camera_compat.py --idf-path "$IDF_PATH" \ + --project-path . --component-path managed_components/espressif__esp_video --build-path build --verify-only + fi && if [ "${{ matrix.name }}" = "component-test-app-build" ]; then python3 ../../../esp-openclaw-talk/tests/run_host_tests.py --sanitize && python3 ../../../esp-openclaw-talk/tests/run_threaded_tests.py && @@ -115,7 +124,7 @@ jobs: --run-id "${{ github.run_id }}" \ --run-attempt "${{ github.run_attempt }}" \ --idf-image "espressif/idf:release-v5.5" \ - $nvs_diagnostics $sdio_diagnostics + $nvs_diagnostics $sdio_diagnostics $camera_compat fi - name: Upload firmware diff --git a/examples/m5stack-tab5-room-node/README.md b/examples/m5stack-tab5-room-node/README.md index e3d1df0..c00e229 100644 --- a/examples/m5stack-tab5-room-node/README.md +++ b/examples/m5stack-tab5-room-node/README.md @@ -53,6 +53,53 @@ The separate ELF/map artifact copies that same firmware manifest unchanged. Retire the patch, helper and packaging exception together only after an upstream SDK fix is selected and qualified on affected hardware. +### Camera CSI compatibility + +Tab5 CI additionally applies `patches/esp-video/tab5-csi-post-isp-format.patch` +to the published `esp_video` 2.4.1 component at revision +`67a555a517b7aa4753836432453659abfaca393a`. That component's IDF<6 mapping asks +CSI to convert the sensor's RAW8 format to RGB565. The exact SDK above has +backported the post-ISP CSI contract: on P4 revisions below 3, CSI input and +output must match. The captured revision-1.3 failure was at `VIDIOC_STREAMON`, +errno 3, which this video component maps from `ESP_ERR_NOT_SUPPORTED`. +See [esp-video-components issue 98](https://github.com/espressif/esp-video-components/issues/98) +for a related report; its different board/sensor is not Tab5 qualification. + +The patch keeps ISP conversion RAW8 to RGB565, then passes RGB565 to RGB565 +through CSI. This preserves 16-bit output and DMA sizing; it does not pass +RAW8 through while claiming RGB565 buffers. Raw bypass, unsupported-format +checks, SDK guards, allocation, sensor settings, and chip-revision selections +are unchanged. + +Use only the isolated SDK and diagnostic profile described on this page. +After SDK patching and `idf.py reconfigure`, apply the camera patch alongside +the SDIO patches below, then build with normal Ninja: + +```sh +python3 ../../scripts/tab5_camera_compat.py --idf-path "$IDF_PATH" \ + --project-path . --component-path managed_components/espressif__esp_video --build-path build +ninja -C build -j2 +python3 ../../scripts/tab5_camera_compat.py --idf-path "$IDF_PATH" \ + --project-path . --component-path managed_components/espressif__esp_video --build-path build --verify-only +``` + +Application is gated by the exact SDK commit and CSI source hash, unchanged +early-P4 profile, registry lock/manifest, original whole-component/source hashes, +and tracked patch hash. Another SDK is refused without modifying it; no version +macro is spoofed. After Ninja, verification checks the configured mapper path, +current RISC-V object, and final patched component/source hashes. Component-manager +integrity markers are never rewritten; stop if reconfiguration rejects a modified +component rather than bypassing its checks. + +Packaging requires `--camera-compat` in addition to this branch's SDK/SDIO flags. +Its additive `camera_compatibility_patch` record retains SDK and SDIO provenance +unchanged, and the separate symbols artifact receives the same manifest. Host +tests execute the complete upstream mapper, including original-failure/patched-pass, +raw bypass, sibling outputs and rejection cases; they do not execute CSI DMA or +capture a frame. A successful native build is not physical camera qualification. +Retire this patch and its explicit packaging mode together after selecting and +qualifying an upstream component that matches the actual SDK contract. + ### NVS diagnostic build This diagnostic branch additionally applies diff --git a/patches/esp-video/tab5-csi-post-isp-format.patch b/patches/esp-video/tab5-csi-post-isp-format.patch new file mode 100644 index 0000000..c7901db --- /dev/null +++ b/patches/esp-video/tab5-csi-post-isp-format.patch @@ -0,0 +1,18 @@ +diff --git a/src/device/esp_video_csi_format.c b/src/device/esp_video_csi_format.c +--- a/src/device/esp_video_csi_format.c ++++ b/src/device/esp_video_csi_format.c +@@ -506,13 +506,8 @@ esp_err_t esp_video_csi_check_format(esp_cam_sensor_output_format_t sensor_fmt, uint + /* ISP output to requested format */ + in_out_format->isp_output_fmt = v4l2_to_isp_color(requested_fmt); + +- /* ISP output to CSI output */ +-#if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(6, 0, 0) +- /* Old IDF version: use sensor format as CSI input */ +- in_out_format->csi_input_fmt = v4l2_to_csi_color(sensor_v4l2_fmt); +-#else ++ /* The pinned Tab5 SDK backports the post-ISP CSI input contract. */ + in_out_format->csi_input_fmt = v4l2_to_csi_color(requested_fmt); +-#endif + in_out_format->csi_output_fmt = v4l2_to_csi_color(requested_fmt); + + ESP_LOGD(TAG, "Format supported: sensor->ISP(process)->CSI(passthrough)"); diff --git a/scripts/package_firmware.py b/scripts/package_firmware.py index 2480d06..ea4f083 100644 --- a/scripts/package_firmware.py +++ b/scripts/package_firmware.py @@ -17,6 +17,7 @@ from idf_tab5_compat import verify_sdk_patch from tab5_sdio_diagnostics import COMPONENT as SDIO_COMPONENT, verify_component_patch +from tab5_camera_compat import COMPONENT as CAMERA_COMPONENT, verify_component_patch as verify_camera_patch EXAMPLES = { @@ -164,7 +165,8 @@ def tab5_provenance(project, build): def package_firmware(project, build, output, target, provenance, dependencies, - nvs_diagnostics=False, sdio_diagnostics=False, sdio_psram_rx=False): + nvs_diagnostics=False, sdio_diagnostics=False, sdio_psram_rx=False, + camera_compat=False): project, build, output = project.resolve(), build.resolve(), output.resolve() repo = Path(git(project, "rev-parse", "--show-toplevel")).resolve() if project.parent != repo / "examples" or EXAMPLES.get(project.name) != target: @@ -217,6 +219,16 @@ def package_firmware(project, build, output, target, provenance, dependencies, project, project / "managed_components/espressif__esp_hosted", build, dependencies, patched=sdio_diagnostics, psram_rx=sdio_psram_rx, ) + camera_patch = None + if camera_compat and (project.name != "m5stack-tab5-room-node" or not nvs_diagnostics): + raise ValueError("Camera compatibility requires the explicitly patched Tab5 SDK") + if camera_compat or ( + project.name == "m5stack-tab5-room-node" + and CAMERA_COMPONENT in dependencies["dependencies"]): + camera_patch = verify_camera_patch( + project, project / "managed_components/espressif__esp_video", + build, idf, dependencies, patched=camera_compat, compiled=camera_compat, + ) normalize = [(build, ""), (project, ""), (repo, ""), (idf, "")] lock_file = checked_file(project / "dependencies.lock", roots) manifest = { @@ -254,6 +266,8 @@ def package_firmware(project, build, output, target, provenance, dependencies, manifest["component_compatibility_patch"] = sdio_patch elif sdio_diagnostics: manifest["component_diagnostic_patch"] = sdio_patch + if camera_compat: + manifest["camera_compatibility_patch"] = camera_patch if project.name == "m5stack-tab5-room-node": manifest["tab5_bsp"] = tab5_provenance(project, build) extra = flash["extra_esptool_args"] @@ -363,12 +377,14 @@ def main(): parser.add_argument("--nvs-diagnostics", action="store_true") parser.add_argument("--sdio-diagnostics", action="store_true") parser.add_argument("--sdio-psram-rx", action="store_true") + parser.add_argument("--camera-compat", action="store_true") args = parser.parse_args() provenance = vars(args).copy() provenance.pop("target") provenance.pop("nvs_diagnostics") provenance.pop("sdio_diagnostics") provenance.pop("sdio_psram_rx") + provenance.pop("camera_compat") project = Path.cwd() # ruamel.yaml is already part of ESP-IDF's component-manager environment. try: @@ -380,7 +396,7 @@ def main(): output = package_firmware( project, project / "build", project / "build/firmware", args.target, provenance, dependencies, args.nvs_diagnostics, - args.sdio_diagnostics, args.sdio_psram_rx, + args.sdio_diagnostics, args.sdio_psram_rx, args.camera_compat, ) except ValueError as error: parser.exit(1, f"Firmware packaging failed: {error}\n") diff --git a/scripts/tab5_camera_compat.py b/scripts/tab5_camera_compat.py new file mode 100644 index 0000000..4ce5292 --- /dev/null +++ b/scripts/tab5_camera_compat.py @@ -0,0 +1,193 @@ +#!/usr/bin/env python3 +"""Apply or verify the pinned Tab5 post-ISP CSI format compatibility patch.""" + +import argparse +import hashlib +import json +from pathlib import Path +import shlex +import subprocess + +from idf_tab5_compat import BASE_COMMIT, git, verify_sdk_patch + + +COMPONENT = "espressif/esp_video" +VERSION = "2.4.1" +REVISION = "67a555a517b7aa4753836432453659abfaca393a" +COMPONENT_HASH = "ef614cc04e69e117fd08272a8b347ae5972eff2d0c8659af6444f8484931c7c1" +PATCHED_COMPONENT_HASH = "106b93e92355f7577ee03f2ea3208d43b8fad15780c438b51633e20c81d10ccf" +MANIFEST_SHA256 = "7ebdab70659a0b198ab04bf1baaacf0b97a6b3e90200f26190c48f51e0e90f62" +SOURCE_PATH = "src/device/esp_video_csi_format.c" +ORIGINAL_SHA256 = "1564837860a5af41f0e3191508e41b5c1dcbec87518b1c4b4d1a549a66a3aa3b" +PATCHED_SHA256 = "99e47cbdbb6fc8d218ea17ac8ac4fd02c7b03aad89e06c48bbc1e5df4d4c3dfb" +PATCH_RELATIVE = "patches/esp-video/tab5-csi-post-isp-format.patch" +PATCH_PATH = Path(__file__).resolve().parents[1] / PATCH_RELATIVE +PATCH_SHA256 = "725f6309fe77fd532a69330c7f3c9632b45675fe05a2489f76895d154098cd36" +SDK_SOURCE = "components/esp_driver_cam/csi/src/esp_cam_ctlr_csi.c" +SDK_SOURCE_SHA256 = "2ec4c6e9fa2f6b83760e04d8dce269b02f5eafaae3feed7d761de5699fac152b" +PROFILE = { + "CONFIG_IDF_TARGET": '"esp32p4"', + "CONFIG_ESP32P4_SELECTS_REV_LESS_V3": "y", + "CONFIG_ESP32P4_REV_MIN_FULL": "0", + "CONFIG_ESP32P4_REV_MAX_FULL": "199", +} + + +def digest(data): + return hashlib.sha256(data).hexdigest() + + +def inspect_component(project, component, build, idf, dependencies, patched): + project = Path(project).resolve(strict=True) + component = Path(component).absolute() + build = Path(build).resolve(strict=True) + idf = Path(idf).resolve(strict=True) + expected = project / "managed_components/espressif__esp_video" + if (project.name != "m5stack-tab5-room-node" or project.parent.name != "examples" + or component != expected or component.resolve(strict=True) != expected + or not build.is_relative_to(project)): + raise ValueError("Select the configured Tab5 project and its managed esp_video") + if any(path.is_symlink() for path in component.rglob("*")): + raise ValueError("Managed component must not contain symbolic links") + if (not isinstance(dependencies, dict) + or not isinstance(dependencies.get("dependencies"), dict)): + raise ValueError("Dependency lock must contain a dependency mapping") + dependency = dependencies["dependencies"].get(COMPONENT, {}) + if not isinstance(dependency, dict) or not isinstance(dependency.get("source"), dict): + raise ValueError("Resolved camera component must have a registry source") + source = dependency["source"] + if (dependencies.get("target") != "esp32p4" + or dependency.get("version") != VERSION + or dependency.get("component_hash") != COMPONENT_HASH + or source.get("type") != "service" + or source.get("registry_url") != "https://components.espressif.com/"): + raise ValueError("Resolved esp_video identity differs from the approved 2.4.1 component") + if digest((component / "idf_component.yml").read_bytes()) != MANIFEST_SHA256: + raise ValueError("Camera component manifest or registry revision differs") + if PATCH_PATH.is_symlink() or digest(PATCH_PATH.read_bytes()) != PATCH_SHA256: + raise ValueError("Tracked camera patch has unexpected contents") + description = json.loads((build / "project_description.json").read_text()) + if (Path(description["idf_path"]).resolve() != idf + or Path(description["project_path"]).resolve() != project + or Path(description["build_dir"]).resolve() != build + or description["target"] != "esp32p4"): + raise ValueError("Camera build does not select the explicit SDK and project") + config_file = Path(description["config_file"]).resolve(strict=True) + if not config_file.is_relative_to(project): + raise ValueError("Camera configuration is outside the selected project") + config = dict(line.split("=", 1) for line in config_file.read_text().splitlines() + if line.startswith("CONFIG_") and "=" in line) + if patched and any(config.get(key) != value for key, value in PROFILE.items()): + raise ValueError("Camera compatibility requires the unchanged early-P4 Tab5 profile") + if patched: + # The version number alone does not identify this backported CSI contract. + verify_sdk_patch(idf, nvs_diagnostics=True) + sdk_source = idf / SDK_SOURCE + if (sdk_source.resolve(strict=True) != sdk_source + or digest(sdk_source.read_bytes()) != SDK_SOURCE_SHA256 + or digest(git(idf, "show", f"HEAD:{SDK_SOURCE}")) != SDK_SOURCE_SHA256): + raise ValueError("SDK CSI source does not match the approved backported contract") + commands = json.loads((build / "compile_commands.json").read_text()) + selected = [entry for entry in commands if Path(entry["file"]).name == Path(SOURCE_PATH).name] + if len(selected) != 1: + raise ValueError("Build must compile exactly one camera format mapper") + command = selected[0] + directory = Path(command["directory"]) + if (directory / command["file"]).resolve(strict=True) != component / SOURCE_PATH: + raise ValueError("Build selects a different camera format mapper") + arguments = command.get("arguments") + if arguments is None: + arguments = shlex.split(command["command"]) + if (arguments.count("-c") != 1 or arguments.index("-c") + 1 == len(arguments) + or (directory / arguments[arguments.index("-c") + 1]).resolve(strict=True) + != component / SOURCE_PATH): + raise ValueError("Camera compiler invocation selects a different source") + if arguments.count("-o") != 1 or arguments.index("-o") + 1 == len(arguments): + raise ValueError("Camera compile command must identify its output object") + output = (directory / arguments[arguments.index("-o") + 1]).resolve() + if not output.is_relative_to(build): + raise ValueError("Camera object must be inside the selected build") + return component, output + + +def validate_component_hash(component, expected): + from idf_component_tools.errors import ProcessingError + from idf_component_tools.hash_tools.errors import ValidatingHashError + from idf_component_tools.hash_tools.validate import ( + validate_hash_eq_hashdir, + validate_hash_eq_hashfile, + ) + try: + validate_hash_eq_hashfile(component, COMPONENT_HASH) + validate_hash_eq_hashdir(component, expected) + except (ProcessingError, ValidatingHashError) as error: + raise ValueError("Camera component bytes do not match the selected profile") from error + + +def verify_component_patch(project, component, build, idf, dependencies, + patched=True, compiled=True): + component, output = inspect_component(project, component, build, idf, dependencies, patched) + source = component / SOURCE_PATH + source_hash = digest(source.read_bytes()) + if source_hash != (PATCHED_SHA256 if patched else ORIGINAL_SHA256): + raise ValueError("Camera source does not match the explicitly selected profile") + validate_component_hash(component, PATCHED_COMPONENT_HASH if patched else COMPONENT_HASH) + result = {"component": COMPONENT, "version": VERSION, "modified": patched} + if patched: + result.update( + profile="tab5-early-p4-post-isp-csi", sdk_base_commit=BASE_COMMIT, + sdk_contract_source=SDK_SOURCE, sdk_contract_sha256=SDK_SOURCE_SHA256, + registry_revision=REVISION, registry_path="esp_video", + original_component_hash=COMPONENT_HASH, + patched_component_hash=PATCHED_COMPONENT_HASH, + source=SOURCE_PATH, original_source_sha256=ORIGINAL_SHA256, + patched_source_sha256=source_hash, + patch=PATCH_RELATIVE, patch_sha256=PATCH_SHA256, + ) + if compiled: + with output.open("rb") as stream: + header = stream.read(20) + if (header[:6] != b"\x7fELF\x01\x01" or header[16:20] != b"\x01\x00\xf3\x00" + or output.stat().st_mtime_ns < source.stat().st_mtime_ns): + raise ValueError("Camera mapper needs a current ELF32 RISC-V relocatable object") + result["compiled_object"] = { + "path": output.relative_to(Path(build).resolve()).as_posix(), + "sha256": digest(output.read_bytes()), + } + return result + + +def apply_component_patch(project, component, build, idf, dependencies): + component, _ = inspect_component(project, component, build, idf, dependencies, patched=True) + if digest((component / SOURCE_PATH).read_bytes()) != PATCHED_SHA256: + verify_component_patch(project, component, build, idf, dependencies, + patched=False, compiled=False) + subprocess.run(["git", "apply", "--check", str(PATCH_PATH)], cwd=component, check=True) + subprocess.run(["git", "apply", str(PATCH_PATH)], cwd=component, check=True) + return verify_component_patch(project, component, build, idf, dependencies, compiled=False) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + for name in ("project-path", "component-path", "build-path", "idf-path"): + parser.add_argument("--" + name, type=Path, required=True) + parser.add_argument("--verify-only", action="store_true") + args = parser.parse_args() + try: + from ruamel.yaml import YAML + from ruamel.yaml.error import YAMLError + except ImportError: + parser.exit(1, "Run this helper in the configured ESP-IDF Python environment.\n") + try: + dependencies = YAML(typ="safe").load((args.project_path / "dependencies.lock").read_text()) + operation = verify_component_patch if args.verify_only else apply_component_patch + result = operation(args.project_path, args.component_path, args.build_path, + args.idf_path, dependencies) + except (ValueError, TypeError, KeyError, OSError, ImportError, + YAMLError, subprocess.CalledProcessError) as error: + parser.exit(1, f"Camera compatibility patch refused: {type(error).__name__}. Check the pinned profile and build inputs.\n") + print(json.dumps(result, indent=2)) + + +if __name__ == "__main__": + main() diff --git a/scripts/tests/fixtures/test_tab5_camera_format.c b/scripts/tests/fixtures/test_tab5_camera_format.c new file mode 100644 index 0000000..2ad1583 --- /dev/null +++ b/scripts/tests/fixtures/test_tab5_camera_format.c @@ -0,0 +1,139 @@ +#include +#include +#include +#include + +/* Host-only type/log boundaries for the complete, hash-checked upstream mapper. */ +typedef int esp_err_t; +typedef int cam_ctlr_color_t; +typedef int isp_color_t; +typedef int esp_cam_sensor_output_format_t; +enum { ESP_OK, ESP_ERR_INVALID_ARG, ESP_ERR_NOT_SUPPORTED }; +enum { + CAM_CTLR_COLOR_RAW8, CAM_CTLR_COLOR_RAW10, CAM_CTLR_COLOR_RAW12, + CAM_CTLR_COLOR_RGB565, CAM_CTLR_COLOR_RGB888, CAM_CTLR_COLOR_YUV420, + CAM_CTLR_COLOR_YUV422, CAM_CTLR_COLOR_YUV422_UYVY, CAM_CTLR_COLOR_GRAY8 +}; +enum { + ISP_COLOR_RAW8, ISP_COLOR_RAW10, ISP_COLOR_RAW12, ISP_COLOR_RGB565, + ISP_COLOR_RGB888, ISP_COLOR_YUV420, ISP_COLOR_YUV422 +}; +enum { + ESP_CAM_SENSOR_PIXFORMAT_RGB565_LE, ESP_CAM_SENSOR_PIXFORMAT_RGB565_BE, + ESP_CAM_SENSOR_PIXFORMAT_YUV422_UYVY, ESP_CAM_SENSOR_PIXFORMAT_YUV422_YUYV, + ESP_CAM_SENSOR_PIXFORMAT_YUV420, ESP_CAM_SENSOR_PIXFORMAT_RGB888, + ESP_CAM_SENSOR_PIXFORMAT_RGB444, ESP_CAM_SENSOR_PIXFORMAT_RGB555, + ESP_CAM_SENSOR_PIXFORMAT_BGR888, ESP_CAM_SENSOR_PIXFORMAT_RAW8, + ESP_CAM_SENSOR_PIXFORMAT_RAW10, ESP_CAM_SENSOR_PIXFORMAT_RAW12, + ESP_CAM_SENSOR_PIXFORMAT_GRAYSCALE +}; +enum { + V4L2_PIX_FMT_SBGGR8 = 100, V4L2_PIX_FMT_SBGGR10, V4L2_PIX_FMT_SBGGR12, + V4L2_PIX_FMT_RGB565, V4L2_PIX_FMT_RGB565X, V4L2_PIX_FMT_RGB24, + V4L2_PIX_FMT_YUV420, V4L2_PIX_FMT_UYVY, V4L2_PIX_FMT_VYUY, + V4L2_PIX_FMT_YUYV, V4L2_PIX_FMT_YVYU, V4L2_PIX_FMT_GREY +}; +typedef struct { + bool isp_bypass_required; + isp_color_t isp_input_fmt; + isp_color_t isp_output_fmt; + uint8_t isp_bpp; + cam_ctlr_color_t csi_input_fmt; + cam_ctlr_color_t csi_output_fmt; +} esp_video_csi_isp_in_out_format_t; + +#define ARRAY_SIZE(array) (sizeof(array) / sizeof((array)[0])) +#define ESP_IDF_VERSION_VAL(major, minor, patch) (((major) << 16) | ((minor) << 8) | (patch)) +#define ESP_VIDEO_CSI_DEVICE_CONV_FORMAT 0 +#define CONFIG_ESP_VIDEO_ENABLE_ISP_VIDEO_DEVICE 1 +#define ESP_LOGD(tag, ...) ((void)(tag)) +#define ESP_LOGI(tag, ...) ((void)(tag)) +#define ESP_LOGE(tag, ...) ((void)(tag)) +#define ESP_RETURN_ON_FALSE(condition, result, ...) do { if (!(condition)) return (result); } while (0) + +static bool esp_video_isp_video_device_is_raw_bypass(void) +{ + return true; +} + +#include "camera_mapper_under_test.inc" + +#define CHECK(condition, message) do { \ + if (!(condition)) { fprintf(stderr, "%s\n", message); return 1; } \ +} while (0) + +static int processing(void) +{ + const struct { + uint32_t output; + cam_ctlr_color_t csi; + isp_color_t isp; + uint8_t bpp; + } cases[] = { + {V4L2_PIX_FMT_RGB565, CAM_CTLR_COLOR_RGB565, ISP_COLOR_RGB565, 16}, + {V4L2_PIX_FMT_RGB24, CAM_CTLR_COLOR_RGB888, ISP_COLOR_RGB888, 24}, + {V4L2_PIX_FMT_YUV420, CAM_CTLR_COLOR_YUV420, ISP_COLOR_YUV420, 12}, + }; + for (size_t i = 0; i < ARRAY_SIZE(cases); ++i) { + for (int raw10 = 0; raw10 <= 1; ++raw10) { + esp_video_csi_isp_in_out_format_t format = {0}; + int sensor = raw10 ? ESP_CAM_SENSOR_PIXFORMAT_RAW10 : ESP_CAM_SENSOR_PIXFORMAT_RAW8; + CHECK(esp_video_csi_check_format(sensor, cases[i].output, &format) == ESP_OK, + "supported processing format rejected"); + CHECK(!format.isp_bypass_required, "ISP processing was bypassed"); + CHECK(format.isp_input_fmt == (raw10 ? ISP_COLOR_RAW10 : ISP_COLOR_RAW8), + "ISP sensor input changed"); + CHECK(format.isp_output_fmt == cases[i].isp, "ISP output changed"); + CHECK(isp_color_to_bpp(format.isp_output_fmt) == cases[i].bpp, "output bit depth changed"); + CHECK(format.csi_output_fmt == cases[i].csi, "CSI output changed"); + CHECK(format.csi_input_fmt == cases[i].csi, "post-ISP CSI input mismatch"); + CHECK(format.csi_input_fmt == format.csi_output_fmt, "early-P4 CSI conversion requested"); + } + } + return 0; +} + +static int bypass(void) +{ + const int sensors[] = {ESP_CAM_SENSOR_PIXFORMAT_RAW8, ESP_CAM_SENSOR_PIXFORMAT_RAW10}; + const uint32_t pixels[] = {V4L2_PIX_FMT_SBGGR8, V4L2_PIX_FMT_SBGGR10}; + const int colors[] = {CAM_CTLR_COLOR_RAW8, CAM_CTLR_COLOR_RAW10}; + for (size_t i = 0; i < ARRAY_SIZE(sensors); ++i) { + for (int use_default = 0; use_default <= 1; ++use_default) { + esp_video_csi_isp_in_out_format_t format = {0}; + CHECK(esp_video_csi_check_format(sensors[i], use_default ? 0 : pixels[i], &format) == ESP_OK, + "raw bypass rejected"); + CHECK(format.isp_bypass_required, "raw bypass changed"); + CHECK(format.csi_input_fmt == colors[i] && format.csi_output_fmt == colors[i], + "raw CSI passthrough changed"); + CHECK(format.isp_input_fmt == format.isp_output_fmt, "raw ISP passthrough changed"); + CHECK(format.isp_bpp == (i ? 10 : 8), "raw bypass bit depth changed"); + } + } + return 0; +} + +static int unsupported(void) +{ + esp_video_csi_isp_in_out_format_t format = {0}; + CHECK(esp_video_csi_check_format(ESP_CAM_SENSOR_PIXFORMAT_RAW8, UINT32_MAX, &format) + == ESP_ERR_NOT_SUPPORTED, "unknown output accepted"); + CHECK(esp_video_csi_check_format(ESP_CAM_SENSOR_PIXFORMAT_RAW12, V4L2_PIX_FMT_RGB565, &format) + == ESP_ERR_NOT_SUPPORTED, "unsupported raw12 conversion accepted"); + CHECK(esp_video_csi_check_format(ESP_CAM_SENSOR_PIXFORMAT_RAW8, V4L2_PIX_FMT_YUYV, &format) + == ESP_ERR_NOT_SUPPORTED, "unsupported old-chip output accepted"); + CHECK(esp_video_csi_check_format(-1, V4L2_PIX_FMT_RGB565, &format) + == ESP_ERR_NOT_SUPPORTED, "unknown sensor accepted"); + CHECK(esp_video_csi_check_format(ESP_CAM_SENSOR_PIXFORMAT_RAW8, V4L2_PIX_FMT_RGB565, NULL) + == ESP_ERR_INVALID_ARG, "null output accepted"); + return 0; +} + +int main(int argc, char **argv) +{ + if (argc != 2) return 2; + if (strcmp(argv[1], "processing") == 0) return processing(); + if (strcmp(argv[1], "bypass") == 0) return bypass(); + if (strcmp(argv[1], "unsupported") == 0) return unsupported(); + return 2; +} diff --git a/scripts/tests/test_package_firmware.py b/scripts/tests/test_package_firmware.py index 501783b..dde8fbb 100644 --- a/scripts/tests/test_package_firmware.py +++ b/scripts/tests/test_package_firmware.py @@ -21,6 +21,8 @@ import idf_tab5_compat as compat from test_tab5_sdio_diagnostics import HAS_MANAGER, component_fixture import tab5_sdio_diagnostics as sdio +from test_tab5_camera_compat import camera_fixture, seed_camera_sdk, write_object +import tab5_camera_compat as camera SPEC = importlib.util.spec_from_file_location("package_firmware", SCRIPT) firmware = importlib.util.module_from_spec(SPEC) @@ -135,11 +137,13 @@ def write_metadata(self): (self.build / "project_description.json").write_text(json.dumps(self.description)) (self.build / "flasher_args.json").write_text(json.dumps(self.flash)) - def package(self, nvs_diagnostics=False, sdio_diagnostics=False, sdio_psram_rx=False): + def package(self, nvs_diagnostics=False, sdio_diagnostics=False, sdio_psram_rx=False, + camera_compat=False): self.write_metadata() return firmware.package_firmware( self.project, self.build, self.output, self.description["target"], self.provenance, self.dependencies, nvs_diagnostics, sdio_diagnostics, sdio_psram_rx, + camera_compat, ) def test_relocated_bundle_preserves_every_image_and_original_inputs(self): @@ -487,6 +491,76 @@ def test_psram_rx_bundle_requires_opt_in_capabilities_and_exact_patch_chain(self self.assertEqual(expected["patched_source_sha256"], firmware.sha256(component / sdio.SOURCE_PATH)) + @unittest.skipUnless(HAS_MANAGER, "actual component hashing runs in the IDF environment") + def test_camera_metadata_is_additive_and_revalidates_final_component_and_object(self): + self.configure_tab5() + self.config += "CONFIG_SPIRAM=y\nCONFIG_SOC_SDMMC_PSRAM_DMA_CAPABLE=y\n" + self.config += "".join(f"{k}={v}\n" for k, v in camera.PROFILE.items()) + base = seed_camera_sdk(self.idf) + self.write_metadata() + with fixture_profile(base), component_fixture( + self.project, self.build, self.dependencies) as sdio_component, camera_fixture( + self.project, self.build, self.idf, self.dependencies) as camera_component: + expected_sdk = compat.apply_sdk_patch(self.idf, nvs_diagnostics=True) + expected_sdio = sdio.apply_component_patch( + self.project, sdio_component, self.build, self.dependencies, psram_rx=True) + camera.apply_component_patch( + self.project, camera_component, self.build, self.idf, self.dependencies) + with self.assertRaises(FileNotFoundError): + self.package(True, True, True, True) + self.assertFalse(self.output.exists()) + source = camera_component / camera.SOURCE_PATH + write_object(self.build, source) + with self.assertRaises(ValueError): + self.package(True, True, True) + other = camera_component / "other.c" + original = other.read_bytes() + other.write_bytes(original + b"/* after-build tamper */\n") + with self.assertRaises(ValueError): + self.package(True, True, True, True) + self.assertFalse(self.output.exists()) + other.write_bytes(original) + self.package(True, True, True, True) + manifest = json.loads((self.output / "manifest.json").read_text()) + self.assertEqual(manifest["component_compatibility_patch"], expected_sdio) + for key, value in expected_sdk.items(): + self.assertEqual(manifest["idf"][key], value) + self.assertEqual(manifest["camera_compatibility_patch"], camera.verify_component_patch( + self.project, camera_component, self.build, self.idf, self.dependencies)) + self.assertEqual(manifest["camera_compatibility_patch"]["patched_source_sha256"], + firmware.sha256(source)) + + def test_camera_profile_does_not_enable_itself_on_other_builds(self): + with self.assertRaises(ValueError): + self.package(camera_compat=True) + self.assertFalse(self.output.exists()) + self.configure_tab5() + with self.assertRaises(ValueError): + self.package(camera_compat=True) + self.assertFalse(self.output.exists()) + + @unittest.skipUnless(HAS_MANAGER, "actual component hashing runs in the IDF environment") + def test_unpatched_rev3_packages_but_camera_patch_profile_rejects_it(self): + self.configure_tab5() + self.config += ( + "CONFIG_ESP32P4_REV_MIN_FULL=300\nCONFIG_ESP32P4_REV_MAX_FULL=399\n" + "CONFIG_ESP32P4_SELECTS_REV_LESS_V3=n\n" + ) + self.description.update(min_rev="300", max_rev="399") + base = seed_camera_sdk(self.idf) + self.write_metadata() + with fixture_profile(base), camera_fixture( + self.project, self.build, self.idf, self.dependencies) as component: + compat.apply_sdk_patch(self.idf, nvs_diagnostics=True) + before = {p: p.read_bytes() for p in component.rglob("*") if p.is_file()} + with self.assertRaisesRegex(ValueError, "unchanged early-P4 Tab5 profile"): + self.package(nvs_diagnostics=True, camera_compat=True) + self.assertFalse(self.output.exists()) + self.package(nvs_diagnostics=True) + manifest = json.loads((self.output / "manifest.json").read_text()) + self.assertNotIn("camera_compatibility_patch", manifest) + self.assertEqual({p: p.read_bytes() for p in before}, before) + if __name__ == "__main__": unittest.main() diff --git a/scripts/tests/test_tab5_camera_compat.py b/scripts/tests/test_tab5_camera_compat.py new file mode 100644 index 0000000..4eeee11 --- /dev/null +++ b/scripts/tests/test_tab5_camera_compat.py @@ -0,0 +1,327 @@ +"""Exercise exact component guards and the real upstream camera format mapper.""" + +from contextlib import contextmanager +import copy +import difflib +import json +import os +from pathlib import Path +import shlex +import subprocess +import sys +import tempfile +import unittest +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from test_idf_tab5_compat import fixture_profile, seed_sdk +from test_tab5_sdio_diagnostics import HAS_MANAGER, directory_hash, REGISTRY_SOURCE +import idf_tab5_compat as sdk +import tab5_camera_compat as camera + + +ORIGINAL = b"/* Synthetic camera component, not a pipeline simulation. */\nint original;\n" +PATCHED = ORIGINAL.replace(b"original", b"patched") +SDK_SOURCE = b"/* Synthetic pinned SDK CSI contract fixture. */\n" +OBJECT_PATH = "esp-idf/esp_video/CMakeFiles/esp_video.dir/src/device/esp_video_csi_format.c.obj" + + +def seed_camera_sdk(idf): + seed_sdk(idf) + source = idf / camera.SDK_SOURCE + source.parent.mkdir(parents=True, exist_ok=True) + source.write_bytes(SDK_SOURCE) + sdk.git(idf, "add", ".") + sdk.git(idf, "-c", "user.name=Fixture", "-c", "user.email=fixture@example.com", + "-c", "commit.gpgsign=false", "commit", "-qm", "CSI contract fixture") + return sdk.git(idf, "rev-parse", "HEAD").decode().strip() + + +def write_object(build, source): + output = build / OBJECT_PATH + output.parent.mkdir(parents=True, exist_ok=True) + output.write_bytes(b"\x7fELF\x01\x01" + b"\x00" * 10 + b"\x01\x00\xf3\x00fixture-object") + stamp = max(output.stat().st_mtime_ns, source.stat().st_mtime_ns + 1) + os.utime(output, ns=(stamp, stamp)) + return output + + +@contextmanager +def camera_fixture(project, build, idf, dependencies): + component = project / "managed_components/espressif__esp_video" + source = component / camera.SOURCE_PATH + source.parent.mkdir(parents=True) + source.write_bytes(ORIGINAL) + (component / "other.c").write_text("/* Unrelated component source. */\n") + manifest = component / "idf_component.yml" + manifest.write_text(json.dumps({ + "version": camera.VERSION, + "repository": "git://github.com/espressif/esp-video-components.git", + "repository_info": {"commit_sha": camera.REVISION, "path": "esp_video"}, + "dependencies": {"idf": ">=5.4"}, + })) + original_hash = directory_hash(component) + (component / ".component_hash").write_text(original_hash) + source.write_bytes(PATCHED) + patched_hash = directory_hash(component) + source.write_bytes(ORIGINAL) + patch_file = build / "camera-fixture.patch" + patch_file.write_text("".join(difflib.unified_diff( + ORIGINAL.decode().splitlines(keepends=True), PATCHED.decode().splitlines(keepends=True), + fromfile="a/" + camera.SOURCE_PATH, tofile="b/" + camera.SOURCE_PATH, + ))) + commands_file = build / "compile_commands.json" + commands = json.loads(commands_file.read_text()) if commands_file.exists() else [] + commands.append({ + "directory": str(build), "file": str(source), + "arguments": ["riscv32-esp-elf-gcc", "-o", OBJECT_PATH, "-c", str(source)], + }) + commands_file.write_text(json.dumps(commands)) + dependencies["target"] = "esp32p4" + dependencies.setdefault("dependencies", {})[camera.COMPONENT] = { + "version": camera.VERSION, "component_hash": original_hash, + "source": copy.deepcopy(REGISTRY_SOURCE), + } + with patch.multiple( + camera, COMPONENT_HASH=original_hash, PATCHED_COMPONENT_HASH=patched_hash, + MANIFEST_SHA256=camera.digest(manifest.read_bytes()), + ORIGINAL_SHA256=camera.digest(ORIGINAL), PATCHED_SHA256=camera.digest(PATCHED), + PATCH_PATH=patch_file, PATCH_SHA256=camera.digest(patch_file.read_bytes()), + SDK_SOURCE_SHA256=camera.digest(SDK_SOURCE), + BASE_COMMIT=sdk.git(idf, "rev-parse", "HEAD").decode().strip(), + ): + yield component + + +@unittest.skipUnless(HAS_MANAGER, "requires the real IDF component-manager environment") +class CameraPatchTests(unittest.TestCase): + def setUp(self): + temporary = tempfile.TemporaryDirectory(prefix="camera-compat-test-") + self.addCleanup(temporary.cleanup) + self.root = Path(temporary.name).resolve() + self.project = self.root / "examples/m5stack-tab5-room-node" + self.build = self.project / "build" + self.build.mkdir(parents=True) + self.idf = self.root / "idf" + self.idf.mkdir() + self.enterContext(patch.dict(os.environ, { + "GIT_CONFIG_GLOBAL": os.devnull, "GIT_CONFIG_NOSYSTEM": "1", + })) + for root in (self.root, self.idf): + sdk.git(root, "init", "-q") + self.base = seed_camera_sdk(self.idf) + self.enterContext(fixture_profile(self.base)) + sdk.apply_sdk_patch(self.idf, nvs_diagnostics=True) + self.dependencies = {} + self.component = self.enterContext(camera_fixture( + self.project, self.build, self.idf, self.dependencies)) + self.source = self.component / camera.SOURCE_PATH + self.config = self.project / "sdkconfig" + self.config.write_text("".join(f"{k}={v}\n" for k, v in camera.PROFILE.items())) + (self.build / "project_description.json").write_text(json.dumps({ + "idf_path": str(self.idf), "project_path": str(self.project), + "build_dir": str(self.build), "target": "esp32p4", "config_file": str(self.config), + })) + + def apply(self): + return camera.apply_component_patch( + self.project, self.component, self.build, self.idf, self.dependencies) + + def verify(self, **options): + return camera.verify_component_patch( + self.project, self.component, self.build, self.idf, self.dependencies, **options) + + def test_exact_application_idempotence_integrity_markers_and_metadata(self): + before = {p: p.read_bytes() for p in self.component.rglob("*") if p.is_file()} + sdk_before = sdk.git(self.idf, "diff") + expected = self.apply() + self.assertEqual(self.apply(), expected) + self.assertEqual(self.source.read_bytes(), PATCHED) + for path, data in before.items(): + if path != self.source: + self.assertEqual(path.read_bytes(), data) + self.assertEqual(sdk.git(self.idf, "diff"), sdk_before) + output = write_object(self.build, self.source) + verified = self.verify() + self.assertEqual(verified["sdk_base_commit"], self.base) + self.assertEqual(verified["patched_component_hash"], directory_hash(self.component)) + self.assertEqual(verified["patched_source_sha256"], camera.digest(PATCHED)) + self.assertEqual(verified["compiled_object"]["sha256"], camera.digest(output.read_bytes())) + + def test_verification_never_mutates_and_requires_native_current_object(self): + with self.assertRaises(ValueError): + self.verify() + self.assertEqual(self.source.read_bytes(), ORIGINAL) + self.assertFalse(self.verify(patched=False, compiled=False)["modified"]) + self.apply() + with self.assertRaises(FileNotFoundError): + self.verify() + output = write_object(self.build, self.source) + output.write_bytes(b"not-an-ELF") + with self.assertRaises(ValueError): + self.verify() + write_object(self.build, self.source) + os.utime(output, ns=(1, 1)) + with self.assertRaises(ValueError): + self.verify() + write_object(self.build, self.source) + with self.assertRaises(ValueError): + self.verify(patched=False) + + def test_wrong_sdk_or_tampered_contract_refuses_before_mutation(self): + for module, key in ((sdk, "BASE_COMMIT"), (camera, "SDK_SOURCE_SHA256")): + with self.subTest(key=key), patch.object(module, key, "0" * 64): + with self.assertRaises(ValueError): + self.apply() + self.assertEqual(self.source.read_bytes(), ORIGINAL) + (self.idf / camera.SDK_SOURCE).write_bytes(SDK_SOURCE + b"/* tampered */\n") + with self.assertRaises(ValueError): + self.apply() + self.assertEqual(self.source.read_bytes(), ORIGINAL) + + def test_wrong_profile_lock_and_registry_key_fail_closed(self): + original = copy.deepcopy(self.dependencies) + for key, value in (("version", "2.4.0"), ("component_hash", "0" * 64), + ("source", {"type": "service", "service_url": REGISTRY_SOURCE["registry_url"]})): + with self.subTest(key=key): + self.dependencies = copy.deepcopy(original) + self.dependencies["dependencies"][camera.COMPONENT][key] = value + with self.assertRaises(ValueError): + self.apply() + self.dependencies = original + before = self.config.read_text() + for key in camera.PROFILE: + self.config.write_text(before.replace(f"{key}={camera.PROFILE[key]}", f"{key}=unexpected")) + with self.subTest(key=key), self.assertRaises(ValueError): + self.apply() + self.assertEqual(self.source.read_bytes(), ORIGINAL) + + def test_patch_manifest_and_component_tampering_are_not_idempotence(self): + for name in ("PATCH_SHA256", "MANIFEST_SHA256"): + with self.subTest(name=name), patch.object(camera, name, "0" * 64): + with self.assertRaises(ValueError): + self.apply() + self.assertEqual(self.source.read_bytes(), ORIGINAL) + self.apply() + write_object(self.build, self.source) + other = self.component / "other.c" + other.write_text("/* Changed after the build. */\n") + with self.assertRaises(ValueError): + self.verify() + with self.assertRaises(ValueError): + self.apply() + (self.component / ".component_hash").write_text(camera.PATCHED_COMPONENT_HASH) + with self.assertRaises(ValueError): + self.verify() + + def test_native_compile_path_cannot_select_another_mapper(self): + path = self.build / "compile_commands.json" + original = json.loads(path.read_text()) + wrong = self.build / Path(camera.SOURCE_PATH).name + wrong.write_bytes(ORIGINAL) + wrong_arguments = original[0]["arguments"][:-1] + [str(wrong)] + for commands in ([], original * 2, [{**original[0], "file": str(wrong)}], + [{**original[0], "arguments": wrong_arguments}]): + path.write_text(json.dumps(commands)) + with self.subTest(count=len(commands)), self.assertRaises(ValueError): + self.apply() + self.assertEqual(self.source.read_bytes(), ORIGINAL) + + def test_native_command_string_resolves_the_same_source_and_output(self): + path = self.build / "compile_commands.json" + commands = json.loads(path.read_text()) + commands[0]["command"] = shlex.join(commands[0].pop("arguments")) + path.write_text(json.dumps(commands)) + self.apply() + write_object(self.build, self.source) + self.assertEqual(self.verify()["compiled_object"]["path"], OBJECT_PATH) + + def test_symlink_and_post_build_source_changes_are_rejected(self): + link = self.component / "alias.c" + link.symlink_to(self.source) + with self.assertRaises(ValueError): + self.apply() + link.unlink() + self.apply() + write_object(self.build, self.source) + self.source.write_bytes(PATCHED + b"/* unexpected */\n") + with self.assertRaises(ValueError): + self.verify() + + +class ActualMapperTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + component = Path(os.environ.get( + "ESP_VIDEO_COMPONENT_DIR", "managed_components/espressif__esp_video")) + source = component / camera.SOURCE_PATH + if not source.is_file(): + raise unittest.SkipTest("requires the configured esp_video 2.4.1 source") + data = source.read_bytes() + source_hash = camera.digest(data) + if source_hash not in (camera.ORIGINAL_SHA256, camera.PATCHED_SHA256): + raise AssertionError("mapper fixture requires the pinned original or patched source") + temporary = tempfile.TemporaryDirectory(prefix="camera-mapper-test-") + cls.addClassCleanup(temporary.cleanup) + cls.root = Path(temporary.name) + original = cls.root / "original" / camera.SOURCE_PATH + original.parent.mkdir(parents=True) + original.write_bytes(data) + if source_hash == camera.PATCHED_SHA256: + subprocess.run(["git", "apply", "--reverse", str(camera.PATCH_PATH)], + cwd=cls.root / "original", check=True) + if camera.digest(original.read_bytes()) != camera.ORIGINAL_SHA256: + raise AssertionError("original mapper reconstruction differs") + repaired = cls.root / "patched" / camera.SOURCE_PATH + repaired.parent.mkdir(parents=True) + repaired.write_bytes(original.read_bytes()) + subprocess.run(["git", "apply", str(camera.PATCH_PATH)], cwd=cls.root / "patched", check=True) + if camera.digest(repaired.read_bytes()) != camera.PATCHED_SHA256: + raise AssertionError("patched mapper differs") + cls.binaries = {} + for name, path in (("original", original), ("patched", repaired)): + # Keep the complete upstream implementation; replace only its SDK includes. + include = cls.root / name / "camera_mapper_under_test.inc" + include.write_text("".join( + line for line in path.read_text().splitlines(keepends=True) + if not line.startswith("#include ") + )) + for version in ("0x050505", "0x060000"): + output = cls.root / f"{name}-{version}" + subprocess.run([ + os.environ.get("CC", "cc"), "-std=c11", "-Wall", "-Wextra", "-Werror", + "-Wno-sign-compare", f"-DESP_IDF_VERSION={version}", + "-I", str(include.parent), + str(Path(__file__).parent / "fixtures/test_tab5_camera_format.c"), + "-o", str(output), + ], check=True) + cls.binaries[name, version] = output + + def run_case(self, name, case, version="0x050505"): + return subprocess.run([str(self.binaries[name, version]), case], + capture_output=True, text=True) + + def test_processing_is_red_on_original_and_green_on_repaired_mapper(self): + original = self.run_case("original", "processing") + self.assertEqual(original.returncode, 1, original.stdout + original.stderr) + self.assertIn("post-ISP CSI input mismatch", original.stderr) + repaired = self.run_case("patched", "processing") + self.assertEqual(repaired.returncode, 0, repaired.stdout + repaired.stderr) + + def test_raw_bypass_and_unsupported_formats_are_unchanged(self): + for name in ("original", "patched"): + for case in ("bypass", "unsupported"): + with self.subTest(name=name, case=case): + result = self.run_case(name, case) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_existing_six_point_zero_mapping_is_unchanged(self): + for name in ("original", "patched"): + for case in ("processing", "bypass", "unsupported"): + with self.subTest(name=name, case=case): + result = self.run_case(name, case, "0x060000") + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + +if __name__ == "__main__": + unittest.main() From b3a7ea49c7c8bd67ce80fef7f825d3c5bf2ad93d Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 10 Sep 2026 00:52:50 +0800 Subject: [PATCH 3/5] fix(ci): identify Tab5 camera guard refusals --- scripts/tab5_camera_compat.py | 32 ++++++++++++- scripts/tests/test_tab5_camera_compat.py | 60 +++++++++++++++++++++++- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/scripts/tab5_camera_compat.py b/scripts/tab5_camera_compat.py index 4ce5292..32c8dd4 100644 --- a/scripts/tab5_camera_compat.py +++ b/scripts/tab5_camera_compat.py @@ -32,6 +32,35 @@ "CONFIG_ESP32P4_REV_MAX_FULL": "199", } +# Only these exact static refusals may cross the CLI boundary. +REFUSAL_CODES = { + "Select the configured Tab5 project and its managed esp_video": "project_path", + "Managed component must not contain symbolic links": "component_symlink", + "Dependency lock must contain a dependency mapping": "dependency_mapping", + "Resolved camera component must have a registry source": "registry_source", + "Resolved esp_video identity differs from the approved 2.4.1 component": "component_identity", + "Camera component manifest or registry revision differs": "component_manifest", + "Tracked camera patch has unexpected contents": "tracked_patch", + "Camera build does not select the explicit SDK and project": "build_identity", + "Camera configuration is outside the selected project": "config_path", + "Camera compatibility requires the unchanged early-P4 Tab5 profile": "early_p4_profile", + "SDK CSI source does not match the approved backported contract": "sdk_csi_contract", + "Build must compile exactly one camera format mapper": "mapper_count", + "Build selects a different camera format mapper": "mapper_path", + "Camera compiler invocation selects a different source": "compiler_source", + "Camera compile command must identify its output object": "compiler_output", + "Camera object must be inside the selected build": "object_path", + "Camera component bytes do not match the selected profile": "component_integrity", + "Camera source does not match the explicitly selected profile": "source_profile", + "Camera mapper needs a current ELF32 RISC-V relocatable object": "compiled_object", + "Select the SDK repository root explicitly": "sdk_root", + "SDK is not at the approved compatibility-patch base": "sdk_base", + "Tracked SDK patch has unexpected contents": "sdk_patch_bytes", + "SDK base source does not match the approved whole-file hash": "sdk_base_source", + "SDK source must be a regular file at the approved path": "sdk_source_path", + "SDK is not modified by exactly the selected Tab5 patches": "sdk_patch_state", +} + def digest(data): return hashlib.sha256(data).hexdigest() @@ -185,7 +214,8 @@ def main(): args.idf_path, dependencies) except (ValueError, TypeError, KeyError, OSError, ImportError, YAMLError, subprocess.CalledProcessError) as error: - parser.exit(1, f"Camera compatibility patch refused: {type(error).__name__}. Check the pinned profile and build inputs.\n") + code = REFUSAL_CODES.get(str(error), "unclassified") if type(error) is ValueError else "unclassified" + parser.exit(1, f"Camera compatibility patch refused: guard={code}. Check the pinned profile and build inputs.\n") print(json.dumps(result, indent=2)) diff --git a/scripts/tests/test_tab5_camera_compat.py b/scripts/tests/test_tab5_camera_compat.py index 4eeee11..a730d8a 100644 --- a/scripts/tests/test_tab5_camera_compat.py +++ b/scripts/tests/test_tab5_camera_compat.py @@ -1,8 +1,9 @@ """Exercise exact component guards and the real upstream camera format mapper.""" -from contextlib import contextmanager +from contextlib import contextmanager, redirect_stderr, redirect_stdout import copy import difflib +import io import json import os from pathlib import Path @@ -131,6 +132,63 @@ def verify(self, **options): return camera.verify_component_patch( self.project, self.component, self.build, self.idf, self.dependencies, **options) + def assert_cli_refusal(self, code, canary): + (self.project / "dependencies.lock").write_text(json.dumps(self.dependencies)) + before = {p: p.read_bytes() for p in self.component.rglob("*") if p.is_file()} + sdk_before = sdk.git(self.idf, "diff") + sdk_status = sdk.git(self.idf, "status", "--porcelain=v1") + stdout, stderr = io.StringIO(), io.StringIO() + arguments = [ + "tab5_camera_compat.py", "--project-path", str(self.project), + "--component-path", str(self.component), "--build-path", str(self.build), + "--idf-path", str(self.idf), + ] + with patch.object(sys, "argv", arguments), redirect_stdout(stdout), redirect_stderr(stderr): + with self.assertRaises(SystemExit) as result: + camera.main() + self.assertEqual(result.exception.code, 1) + self.assertEqual(stdout.getvalue(), "") + self.assertIn(f"guard={code}", stderr.getvalue()) + for value in (canary, str(self.root), "Traceback"): + self.assertNotIn(value, stdout.getvalue() + stderr.getvalue()) + self.assertEqual( + {p: p.read_bytes() for p in self.component.rglob("*") if p.is_file()}, before) + self.assertEqual(sdk.git(self.idf, "diff"), sdk_before) + self.assertEqual(sdk.git(self.idf, "status", "--porcelain=v1"), sdk_status) + + def test_cli_classifies_real_camera_and_sdk_guards_before_apply(self): + canary = "synthetic-private-value-must-not-print" + self.dependencies["dependencies"][camera.COMPONENT]["version"] = canary + self.assert_cli_refusal("component_identity", canary) + self.dependencies["dependencies"][camera.COMPONENT]["version"] = camera.VERSION + config = self.config.read_text() + self.config.write_text(config.replace('CONFIG_IDF_TARGET="esp32p4"', + f'CONFIG_IDF_TARGET="{canary}"')) + self.assert_cli_refusal("early_p4_profile", canary) + self.config.write_text(config) + for module, name, code in ( + (camera, "MANIFEST_SHA256", "component_manifest"), + (camera, "SDK_SOURCE_SHA256", "sdk_csi_contract"), + (sdk, "BASE_COMMIT", "sdk_base"), + (sdk, "PATCH_SHA256", "sdk_patch_bytes"), + (sdk, "ORIGINAL_SHA256", "sdk_base_source"), + (sdk, "PATCHED_SHA256", "sdk_patch_state"), + ): + with self.subTest(code=code), patch.object(module, name, canary): + self.assert_cli_refusal(code, canary) + + def test_cli_unknown_errors_have_a_fixed_nonleaking_fallback(self): + canary = "synthetic-private-value-must-not-print" + for error in ( + ValueError(canary), + ValueError("Build must compile exactly one camera format mapper " + canary), + OSError(5, canary, str(self.root / canary)), + subprocess.CalledProcessError(1, [canary], output=canary, stderr=canary), + ): + with self.subTest(error=type(error).__name__), patch.object( + camera, "apply_component_patch", side_effect=error): + self.assert_cli_refusal("unclassified", canary) + def test_exact_application_idempotence_integrity_markers_and_metadata(self): before = {p: p.read_bytes() for p in self.component.rglob("*") if p.is_file()} sdk_before = sdk.git(self.idf, "diff") From 5e66310797e87d35684d8e61b9dba324c5b9cb66 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 10 Sep 2026 01:21:57 +0800 Subject: [PATCH 4/5] fix(ci): apply camera patch from repository root --- scripts/tab5_camera_compat.py | 25 +++++++++++- scripts/tests/test_tab5_camera_compat.py | 51 ++++++++++++++++++++++-- 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/scripts/tab5_camera_compat.py b/scripts/tab5_camera_compat.py index 32c8dd4..697d8b9 100644 --- a/scripts/tab5_camera_compat.py +++ b/scripts/tab5_camera_compat.py @@ -4,6 +4,7 @@ import argparse import hashlib import json +import os from pathlib import Path import shlex import subprocess @@ -59,6 +60,8 @@ "SDK base source does not match the approved whole-file hash": "sdk_base_source", "SDK source must be a regular file at the approved path": "sdk_source_path", "SDK is not modified by exactly the selected Tab5 patches": "sdk_patch_state", + "Cannot determine the camera component Git worktree": "component_worktree", + "Camera component is outside its Git worktree": "component_worktree_path", } @@ -191,8 +194,26 @@ def apply_component_patch(project, component, build, idf, dependencies): if digest((component / SOURCE_PATH).read_bytes()) != PATCHED_SHA256: verify_component_patch(project, component, build, idf, dependencies, patched=False, compiled=False) - subprocess.run(["git", "apply", "--check", str(PATCH_PATH)], cwd=component, check=True) - subprocess.run(["git", "apply", str(PATCH_PATH)], cwd=component, check=True) + discovery = subprocess.run( + ["git", "rev-parse", "--show-toplevel"], cwd=component, + capture_output=True, text=True, env={**os.environ, "LC_ALL": "C"}, + ) + root, directory = component, [] + if discovery.returncode == 0: + root = Path(discovery.stdout.strip()).resolve(strict=True) + if not component.is_relative_to(root): + raise ValueError("Camera component is outside its Git worktree") + directory = ["--directory=" + component.relative_to(root).as_posix()] + elif (discovery.returncode != 128 + or discovery.stderr.strip() + != "fatal: not a git repository (or any of the parent directories): .git" + or any((parent / ".git").exists() or (parent / ".git").is_symlink() + for parent in (component, *component.parents))): + raise ValueError("Cannot determine the camera component Git worktree") + # Git-style patch headers are filtered by the current repository prefix. + subprocess.run(["git", "apply", "--check", *directory, str(PATCH_PATH)], + cwd=root, check=True) + subprocess.run(["git", "apply", *directory, str(PATCH_PATH)], cwd=root, check=True) return verify_component_patch(project, component, build, idf, dependencies, compiled=False) diff --git a/scripts/tests/test_tab5_camera_compat.py b/scripts/tests/test_tab5_camera_compat.py index a730d8a..645fd77 100644 --- a/scripts/tests/test_tab5_camera_compat.py +++ b/scripts/tests/test_tab5_camera_compat.py @@ -67,10 +67,12 @@ def camera_fixture(project, build, idf, dependencies): patched_hash = directory_hash(component) source.write_bytes(ORIGINAL) patch_file = build / "camera-fixture.patch" - patch_file.write_text("".join(difflib.unified_diff( - ORIGINAL.decode().splitlines(keepends=True), PATCHED.decode().splitlines(keepends=True), - fromfile="a/" + camera.SOURCE_PATH, tofile="b/" + camera.SOURCE_PATH, - ))) + patch_file.write_text( + f"diff --git a/{camera.SOURCE_PATH} b/{camera.SOURCE_PATH}\n" + + "".join(difflib.unified_diff( + ORIGINAL.decode().splitlines(keepends=True), PATCHED.decode().splitlines(keepends=True), + fromfile="a/" + camera.SOURCE_PATH, tofile="b/" + camera.SOURCE_PATH, + ))) commands_file = build / "compile_commands.json" commands = json.loads(commands_file.read_text()) if commands_file.exists() else [] commands.append({ @@ -190,6 +192,13 @@ def test_cli_unknown_errors_have_a_fixed_nonleaking_fallback(self): self.assert_cli_refusal("unclassified", canary) def test_exact_application_idempotence_integrity_markers_and_metadata(self): + tracked = self.root / "tracked.txt" + tracked.write_text("Original caller-owned file.\n") + sdk.git(self.root, "add", "tracked.txt") + sdk.git(self.root, "-c", "user.name=Fixture", "-c", "user.email=fixture@example.com", + "-c", "commit.gpgsign=false", "commit", "-qm", "Caller-owned file") + tracked.write_text("Preserve this caller edit.\n") + root_before = sdk.git(self.root, "diff", "--binary") before = {p: p.read_bytes() for p in self.component.rglob("*") if p.is_file()} sdk_before = sdk.git(self.idf, "diff") expected = self.apply() @@ -199,6 +208,7 @@ def test_exact_application_idempotence_integrity_markers_and_metadata(self): if path != self.source: self.assertEqual(path.read_bytes(), data) self.assertEqual(sdk.git(self.idf, "diff"), sdk_before) + self.assertEqual(sdk.git(self.root, "diff", "--binary"), root_before) output = write_object(self.build, self.source) verified = self.verify() self.assertEqual(verified["sdk_base_commit"], self.base) @@ -206,6 +216,39 @@ def test_exact_application_idempotence_integrity_markers_and_metadata(self): self.assertEqual(verified["patched_source_sha256"], camera.digest(PATCHED)) self.assertEqual(verified["compiled_object"]["sha256"], camera.digest(output.read_bytes())) + def test_standalone_application_preserves_bytes_and_idempotence(self): + (self.root / ".git").rename(self.root / "saved-git-metadata") + before = {p: p.read_bytes() for p in self.component.rglob("*") if p.is_file()} + sdk_before = sdk.git(self.idf, "diff") + result = self.apply() + self.assertEqual(self.apply(), result) + self.assertEqual(self.source.read_bytes(), PATCHED) + for path, data in before.items(): + if path != self.source: + self.assertEqual(path.read_bytes(), data) + self.assertEqual(sdk.git(self.idf, "diff"), sdk_before) + + def test_cli_rejects_invalid_owner_config_instead_of_standalone_fallback(self): + canary = "synthetic-private-value-must-not-print" + (self.root / ".git/config").write_text(f"[invalid\n{canary}\n") + self.assert_cli_refusal("component_worktree", canary) + + def test_cli_rejects_bare_repository_instead_of_standalone_fallback(self): + (self.root / ".git").rename(self.root / "saved-git-metadata") + sdk.git(self.root, "init", "--bare", "-q") + self.assert_cli_refusal("component_worktree", "synthetic-private-value-must-not-print") + + def test_cli_rejects_broken_git_marker_instead_of_standalone_fallback(self): + (self.root / ".git").rename(self.root / "saved-git-metadata") + (self.root / ".git").mkdir() + self.assert_cli_refusal("component_worktree", "synthetic-private-value-must-not-print") + + def test_cli_requires_component_containment_in_discovered_worktree(self): + outside = self.root / "other-worktree" + outside.mkdir() + sdk.git(self.root, "config", "core.worktree", str(outside)) + self.assert_cli_refusal("component_worktree_path", "synthetic-private-value-must-not-print") + def test_verification_never_mutates_and_requires_native_current_object(self): with self.assertRaises(ValueError): self.verify() From b05b470fabf54fbc9aac687488eafb76e5fc2814 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Thu, 10 Sep 2026 03:23:44 +0800 Subject: [PATCH 5/5] fix(tab5): match camera geometry to PPA scale --- examples/m5stack-tab5-room-node/README.md | 8 + .../tab5_room_board/tab5_room_board.c | 26 +-- .../fixtures/test_tab5_camera_transform.c | 191 ++++++++++++++++++ scripts/tests/test_tab5_camera_diagnostics.py | 41 +++- 4 files changed, 250 insertions(+), 16 deletions(-) create mode 100644 scripts/tests/fixtures/test_tab5_camera_transform.c diff --git a/examples/m5stack-tab5-room-node/README.md b/examples/m5stack-tab5-room-node/README.md index c00e229..c1544b6 100644 --- a/examples/m5stack-tab5-room-node/README.md +++ b/examples/m5stack-tab5-room-node/README.md @@ -312,6 +312,14 @@ mislabeling the front image. openclaw nodes camera snap --node --facing front ``` +`maxWidth` is an upper bound, not an exact JPEG width. Rotation and downscaling +use one uniform PPA scale rounded down to a multiple of 1/16, without upscaling. +For the 90/270-degree orientation, `maxWidth: 640` produces 630x1120 pixels; +the JPEG and response report those actual dimensions. Packed RGB length and +PPA output pitch use the same dimensions, while the allocation retains its +cache-line alignment. This geometry correction does not qualify color, +exposure, warm-up or the complete camera pipeline. + ### Media stage diagnostics The capture owner emits one `camera_capture_diag` failure with a fixed `stage`, diff --git a/examples/m5stack-tab5-room-node/components/tab5_room_board/tab5_room_board.c b/examples/m5stack-tab5-room-node/components/tab5_room_board/tab5_room_board.c index fb43afe..1c30a21 100644 --- a/examples/m5stack-tab5-room-node/components/tab5_room_board/tab5_room_board.c +++ b/examples/m5stack-tab5-room-node/components/tab5_room_board/tab5_room_board.c @@ -46,7 +46,7 @@ #define CAMERA_SENSOR_WIDTH 1280U #define CAMERA_SENSOR_HEIGHT 720U #define CAMERA_MAX_PIXELS (1024U * 1024U) -#define CAMERA_DIMENSION_ALIGNMENT 8U +#define CAMERA_SCALE_DENOMINATOR 16U #define CAMERA_ALIGN_UP(value, alignment) (((value) + (alignment) - 1) & ~((alignment) - 1)) _Static_assert( @@ -941,17 +941,18 @@ static esp_err_t transform_camera_frame( uint32_t natural_width = swaps_dimensions ? camera->height : camera->width; uint32_t natural_height = swaps_dimensions ? camera->width : camera->height; ESP_RETURN_ON_FALSE( + natural_width != 0 && natural_height != 0 && (uint64_t)natural_width * natural_height <= CAMERA_MAX_PIXELS, ESP_ERR_NOT_SUPPORTED, TAG, "rotated camera frame exceeds one megapixel"); - uint32_t output_width = natural_width; - if ((uint32_t)max_width < output_width) output_width = (uint32_t)max_width; - output_width &= ~(CAMERA_DIMENSION_ALIGNMENT - 1U); - uint32_t output_height = (uint32_t)( - ((uint64_t)natural_height * output_width / natural_width) & - ~(uint64_t)(CAMERA_DIMENSION_ALIGNMENT - 1U)); + uint32_t width_limit = natural_width; + if ((uint32_t)max_width < width_limit) width_limit = (uint32_t)max_width; + /* PPA truncates scales to 1/16; geometry and DMA pitch must use that same scale. */ + uint32_t scale_steps = (uint64_t)width_limit * CAMERA_SCALE_DENOMINATOR / natural_width; + uint32_t output_width = (uint64_t)natural_width * scale_steps / CAMERA_SCALE_DENOMINATOR; + uint32_t output_height = (uint64_t)natural_height * scale_steps / CAMERA_SCALE_DENOMINATOR; ESP_RETURN_ON_FALSE( output_width != 0 && output_height != 0 && (uint64_t)output_width * output_height <= CAMERA_MAX_PIXELS, @@ -968,12 +969,7 @@ static esp_err_t transform_camera_frame( MALLOC_CAP_SPIRAM); ESP_RETURN_ON_FALSE(output != NULL, ESP_ERR_NO_MEM, TAG, "camera transform buffer allocation failed"); - float scale_x = (float)output_width / camera->width; - float scale_y = (float)output_height / camera->height; - if (swaps_dimensions) { - scale_x = (float)output_height / camera->width; - scale_y = (float)output_width / camera->height; - } + float scale = (float)scale_steps / CAMERA_SCALE_DENOMINATOR; const ppa_srm_oper_config_t config = { .in = { .buffer = camera->buffers[camera->frame.index], @@ -991,8 +987,8 @@ static esp_err_t transform_camera_frame( .srm_cm = PPA_SRM_COLOR_MODE_RGB888, }, .rotation_angle = rotation, - .scale_x = scale_x, - .scale_y = scale_y, + .scale_x = scale, + .scale_y = scale, .mode = PPA_TRANS_MODE_BLOCKING, }; esp_err_t result = ppa_do_scale_rotate_mirror(camera_ppa_srm, &config); diff --git a/scripts/tests/fixtures/test_tab5_camera_transform.c b/scripts/tests/fixtures/test_tab5_camera_transform.c new file mode 100644 index 0000000..a39455a --- /dev/null +++ b/scripts/tests/fixtures/test_tab5_camera_transform.c @@ -0,0 +1,191 @@ +#include +#include +#include +#include +#include +#include +#include + +typedef int esp_err_t; +enum { ESP_OK, ESP_ERR_INVALID_STATE, ESP_ERR_NOT_SUPPORTED, + ESP_ERR_INVALID_SIZE, ESP_ERR_NO_MEM, PPA_FAILURE }; +enum { MALLOC_CAP_SPIRAM = 1, PPA_SRM_COLOR_MODE_RGB565, + PPA_SRM_COLOR_MODE_RGB888, PPA_TRANS_MODE_BLOCKING }; +typedef enum { + PPA_SRM_ROTATION_ANGLE_0, PPA_SRM_ROTATION_ANGLE_90, + PPA_SRM_ROTATION_ANGLE_180, PPA_SRM_ROTATION_ANGLE_270, +} ppa_srm_rotation_angle_t; +typedef struct { + void *buffer; + size_t buffer_size; + uint32_t pic_w, pic_h, block_w, block_h; + int srm_cm; +} picture_t; +typedef struct { + picture_t in, out; + ppa_srm_rotation_angle_t rotation_angle; + float scale_x, scale_y; + int mode; +} ppa_srm_oper_config_t; +typedef struct { + uint32_t width, height; + void *buffers[2]; + struct { unsigned index; } frame; +} camera_frame_t; + +static int test_rotation; +#define BSP_CAMERA_ROTATION test_rotation +#define TAG "camera-transform-fixture" +#define ESP_LOGE(tag, ...) ((void)(tag)) +#define ESP_RETURN_ON_FALSE(condition, error, tag, ...) do { \ + if (!(condition)) return (error); \ +} while (0) +#define ESP_RETURN_ON_ERROR(call, tag, ...) do { \ + esp_err_t error = (call); \ + if (error != ESP_OK) return error; \ +} while (0) + +static void *camera_ppa_srm = (void *)&test_rotation; +static size_t camera_ppa_alignment = 64; +static void *allocation; +static size_t allocation_bytes; +static unsigned allocations, frees, ppa_calls; +static uint32_t written_width, written_height; +static bool fail_allocation, fail_ppa; + +static void *heap_caps_aligned_calloc(size_t alignment, size_t count, size_t size, int caps) +{ + assert(allocation == NULL && count == 1 && caps == MALLOC_CAP_SPIRAM); + assert(alignment == camera_ppa_alignment && size % alignment == 0); + ++allocations; + if (fail_allocation) return NULL; + allocation = aligned_alloc(alignment, size); + assert(allocation != NULL && (uintptr_t)allocation % 16 == 0); + allocation_bytes = size; + memset(allocation, 0, size); + return allocation; +} + +static void heap_caps_free(void *buffer) +{ + assert(buffer != NULL && buffer == allocation); + ++frees; + free(buffer); + allocation = NULL; +} + +static esp_err_t ppa_do_scale_rotate_mirror(void *client, const ppa_srm_oper_config_t *config) +{ + assert(client == camera_ppa_srm && config->mode == PPA_TRANS_MODE_BLOCKING); + assert(config->in.srm_cm == PPA_SRM_COLOR_MODE_RGB565); + assert(config->out.srm_cm == PPA_SRM_COLOR_MODE_RGB888); + assert(config->out.buffer == allocation && config->out.buffer_size == allocation_bytes); + assert(config->in.block_w == config->in.pic_w && config->in.block_h == config->in.pic_h); + ++ppa_calls; + if (fail_ppa) return PPA_FAILURE; + + /* Pinned PPA hardware truncates each scale to four fractional bits. */ + unsigned x_sixteenths = (unsigned)(config->scale_x * 16); + unsigned y_sixteenths = (unsigned)(config->scale_y * 16); + assert(x_sixteenths >= 1 && x_sixteenths <= 16); + assert(y_sixteenths >= 1 && y_sixteenths <= 16); + written_width = config->in.block_w * x_sixteenths / 16; + written_height = config->in.block_h * y_sixteenths / 16; + if (config->rotation_angle == PPA_SRM_ROTATION_ANGLE_90 || + config->rotation_angle == PPA_SRM_ROTATION_ANGLE_270) { + uint32_t swap = written_width; + written_width = written_height; + written_height = swap; + } + assert(written_width <= config->out.pic_w && written_height <= config->out.pic_h); + assert((size_t)config->out.pic_w * config->out.pic_h * 3 <= allocation_bytes); + uint8_t *output = config->out.buffer; + for (uint32_t y = 0; y < written_height; ++y) { + memset(output + (size_t)y * config->out.pic_w * 3, 0xa5, written_width * 3); + } + return ESP_OK; +} + +#include "camera_transform_under_test.inc" + +typedef struct { + int rotation, max_width; + uint32_t input_width, input_height, output_width, output_height; + esp_err_t result; +} geometry_case_t; + +static void check_geometry(geometry_case_t test) +{ + allocations = frees = ppa_calls = 0; + written_width = written_height = 0; + allocation_bytes = 0; + test_rotation = test.rotation; + uint8_t input; + camera_frame_t camera = { + .width = test.input_width, .height = test.input_height, + .buffers = {&input, NULL}, .frame.index = 0, + }; + camera_transformed_frame_t transformed; + memset(&transformed, 0x55, sizeof(transformed)); + esp_err_t result = transform_camera_frame(&camera, test.max_width, &transformed); + assert(result == test.result); + if (result != ESP_OK) { + assert(transformed.data == NULL && transformed.data_size == 0); + assert(transformed.width == 0 && transformed.height == 0 && allocation == NULL); + if (fail_allocation) assert(allocations == 1 && ppa_calls == 0 && frees == 0); + else if (fail_ppa) assert(allocations == 1 && ppa_calls == 1 && frees == 1); + else assert(allocations == 0 && ppa_calls == 0 && frees == 0); + return; + } + if (transformed.width != written_width || transformed.height != written_height) { + fprintf(stderr, "packed image %ux%u differs from PPA-written extent %ux%u\n", + transformed.width, transformed.height, written_width, written_height); + abort(); + } + assert(transformed.width == test.output_width && transformed.height == test.output_height); + assert(transformed.width <= (uint32_t)test.max_width); + assert(transformed.data_size == (size_t)test.output_width * test.output_height * 3); + assert(transformed.data_size <= CAMERA_MAX_PIXELS * 3U); + assert(allocations == 1 && ppa_calls == 1 && frees == 0); + /* JPEG receives exactly packed RGB888, never an unwritten margin or allocation tail. */ + for (size_t i = 0; i < transformed.data_size; ++i) assert(transformed.data[i] == 0xa5); + for (size_t i = transformed.data_size; i < allocation_bytes; ++i) assert(transformed.data[i] == 0); + heap_caps_free(transformed.data); + assert(frees == 1); +} + +int main(int argc, char **argv) +{ + struct rlimit core_limit = {0, 0}; + assert(setrlimit(RLIMIT_CORE, &core_limit) == 0); + assert(argc == 2); + geometry_case_t requested = {90, 640, 1280, 720, 630, 1120, ESP_OK}; + if (strcmp(argv[1], "requested-640") == 0) { + check_geometry(requested); + } else if (strcmp(argv[1], "geometry") == 0) { + const geometry_case_t cases[] = { + {270, 640, 1280, 720, 630, 1120, ESP_OK}, + {90, 720, 1280, 720, 720, 1280, ESP_OK}, + {270, 2048, 1280, 720, 720, 1280, ESP_OK}, + {0, 1280, 1280, 720, 1280, 720, ESP_OK}, + {180, 640, 1280, 720, 640, 360, ESP_OK}, + {90, 64, 1280, 720, 45, 80, ESP_OK}, + {0, 80, 1280, 720, 80, 45, ESP_OK}, + {0, 64, 1280, 720, 0, 0, ESP_ERR_INVALID_SIZE}, + {90, 629, 1280, 720, 585, 1040, ESP_OK}, + {90, 640, 2048, 1024, 0, 0, ESP_ERR_NOT_SUPPORTED}, + {90, 640, 1280, 0, 0, 0, ESP_ERR_NOT_SUPPORTED}, + }; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); ++i) check_geometry(cases[i]); + } else if (strcmp(argv[1], "allocation-failure") == 0) { + fail_allocation = true; + requested.result = ESP_ERR_NO_MEM; + check_geometry(requested); + } else { + assert(strcmp(argv[1], "ppa-failure") == 0); + fail_ppa = true; + requested.result = PPA_FAILURE; + check_geometry(requested); + } + return 0; +} diff --git a/scripts/tests/test_tab5_camera_diagnostics.py b/scripts/tests/test_tab5_camera_diagnostics.py index a465437..0f6f480 100644 --- a/scripts/tests/test_tab5_camera_diagnostics.py +++ b/scripts/tests/test_tab5_camera_diagnostics.py @@ -1,4 +1,4 @@ -"""Execute the real capture owner against bounded V4L2 and logging stubs.""" +"""Execute the real capture and transform owners against bounded SDK stubs.""" import os from pathlib import Path @@ -10,6 +10,7 @@ ROOT = Path(__file__).resolve().parents[2] SOURCE = ROOT / "examples/m5stack-tab5-room-node/components/tab5_room_board/tab5_room_board.c" FIXTURE = Path(__file__).parent / "fixtures/test_tab5_camera_capture.c" +TRANSFORM_FIXTURE = Path(__file__).parent / "fixtures/test_tab5_camera_transform.c" class CameraCaptureDiagnosticsTests(unittest.TestCase): @@ -55,5 +56,43 @@ def test_existing_initialization_cache_policy(self): self.run_case(scenario) +class CameraTransformTests(unittest.TestCase): + @classmethod + def setUpClass(cls): + source = Path(os.environ.get("TAB5_CAMERA_SOURCE", SOURCE)).read_text() + start = source.index("typedef struct {\n uint8_t *data;\n size_t data_size;") + end = source.index("\nstatic esp_err_t camera_snap(", start) + constants = "\n".join( + line for line in source.splitlines() if line.startswith("#define CAMERA_") + ) + temporary = tempfile.TemporaryDirectory(prefix="tab5-camera-transform-") + cls.addClassCleanup(temporary.cleanup) + directory = Path(temporary.name) + (directory / "camera_transform_under_test.inc").write_text( + constants + "\n" + source[start:end] + ) + cls.binary = directory / "camera-transform" + subprocess.run([ + os.environ.get("CC", "cc"), "-std=c11", "-Wall", "-Wextra", "-Werror", + "-fsanitize=address,undefined", "-fno-sanitize-recover=all", + "-I", str(directory), str(TRANSFORM_FIXTURE), "-o", str(cls.binary), + ], check=True) + + def run_case(self, scenario): + result = subprocess.run([str(self.binary), scenario], capture_output=True, text=True) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_requested_640_matches_written_extent(self): + self.run_case("requested-640") + + def test_native_minimum_scale_rotations_and_size_bounds(self): + self.run_case("geometry") + + def test_allocation_and_ppa_failure_cleanup(self): + for scenario in ("allocation-failure", "ppa-failure"): + with self.subTest(scenario=scenario): + self.run_case(scenario) + + if __name__ == "__main__": unittest.main()