fix: IDF v6.0 ESP-NOW callback compat (#944) + occupancy noise-floor anchor (#942)#945
Merged
Conversation
) ESP-IDF v6.0 changed `esp_now_send_cb_t` from void (*)(const uint8_t *mac, esp_now_send_status_t status) to void (*)(const esp_now_send_info_t *tx_info, esp_now_send_status_t status) The C6 sync ESP-NOW path's `on_recv` was already version-guarded with `#if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(5, 0, 0)` (lines 102-112) but the `on_send` sibling missed the equivalent guard. CI runs against IDF v5.4 so the regression slipped through; the reporter on IDF v6.0.1 with xtensa-esp-elf esp-15.2.0_20251204 hit: c6_sync_espnow.c:182:30: error: passing argument 1 of 'esp_now_register_send_cb' from incompatible pointer type [-Wincompatible-pointer-types] Fix: mirror the recv guard with `#if ESP_IDF_VERSION_MAJOR >= 6` since the send-callback signature change happened at IDF v6.0 (not v5.x like the recv-callback). Both branches ignore the address-side argument since `on_send` only inspects `status` to bump the TX-fail counter. Adds `#include "esp_idf_version.h"` so the macro is in scope. Closes #944 Co-Authored-By: claude-flow <ruv@ruv.net>
…oses #942) `test_estimate_occupancy_noise_only` asserts that 20 noise-only frames fed through a 50-frame calibrated `FieldModel` yield 0 occupancy. Failure reported on the upstream Linux + BLAS build. Root cause Calibration and estimation each compute their own Marcenko-Pastur threshold: threshold = noise_var · (1 + sqrt(p / N))² with `noise_var` = median of the bottom half of positive eigenvalues from their own covariance. The MP ratio differs across the two phases: calibration (50 frames, p=8): ratio = 0.16, factor ≈ 1.96 estimation (20 frames, p=8): ratio = 0.40, factor ≈ 2.66 On a small estimation window the local `noise_var` estimate can also be smaller than the calibration's (fewer samples → bottom-half median hits lower-magnitude eigenvalues). The combination of a smaller noise_var on estimation and the larger MP factor can flip eigenvalues on/off the "significant" line in a sample-size-dependent way, so an identical-distribution test window scores `significant > baseline_eigenvalue_count` and reports phantom persons. Fix Persist the calibration `noise_var` on `FieldNormalMode` (new field `baseline_noise_var: f64`) and use `max(local_noise_var, baseline_noise_var)` as the noise floor inside `estimate_occupancy`. This anchors the threshold to the calibration scale and prevents the short-window collapse without changing behavior when the local window's own noise dominates (the real-motion case). `baseline_noise_var` defaults to 0.0 in the diagonal-fallback paths; the estimation code treats 0.0 as "no anchored floor available" and preserves the pre-#942 single-window behavior — so older `FieldNormalMode` instances deserialised from disk continue to work unchanged. Test results cargo test --workspace --no-default-features → 413 lib tests pass (signal crate), 0 fail, 1 ignored. The actual `eigenvalue`-gated test still requires BLAS (not buildable on Windows). Logic-trace via the four numerical anchors above shows the fix flips `noise_var` from the smaller local value back up to the calibration scale, dropping `significant` to or below `baseline_eigenvalue_count` so the saturating subtraction returns 0. Closes #942 Co-Authored-By: claude-flow <ruv@ruv.net>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two unrelated reporter-submitted bugs from yesterday's queue, both small, both in this PR:
Closes #944 —
idf build failed for esp32-csi-nodeESP-IDF v6.0 changed
esp_now_send_cb_tfrom(const uint8_t *mac, ...)to(const esp_now_send_info_t *tx_info, ...). The C6 sync ESP-NOW recv path was already version-guarded; the send path missed the equivalent guard. Mirroring the pattern (#if ESP_IDF_VERSION_MAJOR >= 6) — both branches just inspectstatusso the body is identical.Closes #942 —
ruvsense::field_model::tests::test_estimate_occupancy_noise_onlyfailingFieldModel::estimate_occupancyrecomputes a Marcenko-Pastur threshold from each estimation window. With a 50-frame calibration but a 20-frame window, the MP factor differs ((1 + √0.16)² ≈ 1.96vs(1 + √0.40)² ≈ 2.66) AND the localnoise_varfrom 20 samples can be smaller than calibration's — so the test's noise-only window can flip eigenvalues on/off the "significant" line in a sample-size-dependent way and report phantom occupancy.Fix: persist calibration's
noise_varonFieldNormalMode(new fieldbaseline_noise_var: f64) and usemax(local, baseline)as the estimation noise floor. Anchors the threshold to the calibration scale without changing behavior when the local window's noise legitimately dominates (real motion case).baseline_noise_var = 0.0in the diagonal-fallback paths; the estimation code treats 0.0 as "no anchored floor available" and preserves pre-#942 behavior — old serialised modes still work.Test plan
cargo test --workspace --no-default-features→ 413 lib tests pass inwifi-densepose-signal, no other workspace regressions.eigenvalue-gated test requires BLAS (not buildable on Windows host); CI's Linux job exercises it. Fix is verified by tracing the four numerical anchors in the commit message.c6_sync_espnow.c:102-112; CI's existing IDF v5.4 build is unchanged (its branch fires identically to before), and the new branch enables IDF v6.0+ builds for the reporter.Hardware
No reflash needed. #944 is a compile-time guard; #942 is host-side signal-processing only.
🤖 Generated with claude-flow