Skip to content

feat: breathing bandpass filter for stationary presence detection - #112

Closed
PeterkoCZ91 wants to merge 5 commits into
francescopace:developfrom
PeterkoCZ91:feat/breathing-bandpass-filter
Closed

feat: breathing bandpass filter for stationary presence detection#112
PeterkoCZ91 wants to merge 5 commits into
francescopace:developfrom
PeterkoCZ91:feat/breathing-bandpass-filter

Conversation

@PeterkoCZ91

@PeterkoCZ91 PeterkoCZ91 commented Apr 3, 2026

Copy link
Copy Markdown

Summary

Breathing bandpass filter for stationary presence detection. Isolates the 0.08-0.6 Hz band (5-36 BPM) from CSI amplitude sum using cascaded 1st-order Butterworth HP + LP filters, then estimates RMS energy via exponential moving average.

Elevated breathing_score indicates periodic amplitude variation consistent with human breathing -- useful for detecting stationary occupants (sitting, sleeping) who produce minimal motion variance.

Changes

C++ firmware (components/espectre/)

  • filters.h: breathing_filter_state_t struct and function declarations
  • csi_filters.cpp: breathing_filter_init(), breathing_filter_apply(), breathing_filter_get_score() with pre-computed coefficients for 100 Hz sample rate
  • base_detector.h: breathing_filter_ member, get_breathing_score() accessor
  • base_detector.cpp: filter applied per-packet in process_packet(), proper transfer in move constructor/assignment, reset in clear_buffer()

Python micro-espectre parity (micro-espectre/)

  • src/filters.py: BreathingFilter class with exact coefficient parity to C++
  • src/segmentation.py: enable_breathing parameter in SegmentationContext, integrated into add_turbulence() filter chain, get_breathing_score() accessor, breathing_score exposed in get_metrics(), reset in reset(full=True)

Tests

  • tests/test_filters.py: 14 unit tests -- initialization, reset, DC rejection, step transient, frequency response (breathing band passes, fast/slow signals rejected), C++ numerical parity, score consistency
  • tests/test_segmentation_additional.py: 4 integration tests -- enable/disable, CSI data processing, reset
  • tests/test_validation_real_data.py: 16 real-data validation tests across 5 chip datasets (C3, C5, C6, ESP32, S3) -- baseline stability (CV < 0.5), movement score finite/non-negative, metrics exposure

Filter design

amplitude_sum -> HP(0.08 Hz) -> LP(0.6 Hz) -> x^2 -> EMA(a=0.00333) -> sqrt -> score
Parameter Value Rationale
HP cutoff 0.08 Hz Removes DC and slow environmental drift
LP cutoff 0.6 Hz Removes motion artifacts and RF noise
Passband 0.08-0.6 Hz Covers 5-36 BPM (normal + elevated breathing)
Energy alpha 0.00333 ~3 second time constant at 100 Hz sample rate
Coefficients Bilinear transform of 1st-order Butterworth Pre-computed for zero runtime overhead

Validation results (real CSI data)

All 5 chip datasets pass:

Chip Baseline CV Baseline mean Movement max Status
C3 < 0.5 stable finite, >= 0 PASS
C5 < 0.5 stable finite, >= 0 PASS
C6 < 0.5 stable finite, >= 0 PASS
ESP32 < 0.5 stable finite, >= 0 PASS
S3 < 0.5 stable finite, >= 0 PASS

Baseline stability (low CV) confirms the filter does not produce false periodic signals in empty rooms. The absolute score scale depends on chip/amplitude, so downstream consumers should compare against a per-environment baseline rather than using fixed thresholds.

Test results

78 passed in 0.73s

@ghost

ghost commented Apr 3, 2026

Copy link
Copy Markdown

Thank you for contributing to ESPectre! We are glad to have your help.

Before we can merge this pull request, please review the Contributor License Agreement.
The CLA confirms contribution rights so we can legally review, distribute, and maintain the project long-term.

Missing signatures: @contributors-assistant[bot]

Once you have read it, reply to this thread with the exact text below:

I have read the CLA Document and I hereby sign the CLA

If you already signed in a previous PR, just comment recheck.

@PeterkoCZ91

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

@ghost

ghost commented Apr 3, 2026

Copy link
Copy Markdown

Thanks for signing the CLA. I recorded your signature. If the check is still red, comment recheck.

@francescopace

Copy link
Copy Markdown
Owner

Thanks for the contribution and for the thoughtful idea.
Detecting stationary presence from breathing-band CSI energy is a valid research direction, and I appreciate the effort.

That said, I can’t merge this PR in its current state yet. Here is why, in detail:

  1. No user-facing behavior change yet
    The PR adds get_breathing_score(), but this new signal is not integrated into the actual detection pipeline (state transitions/fusion logic) and is not exposed as a user-facing sensor/configurable feature.
    In practice, users get no measurable functional benefit after merging.

  2. Missing proof that it improves detection quality
    For a signal-processing feature like this, we need evidence on real scenarios, not only a plausible implementation.
    The PR does not provide dedicated validation for:

    • empty-room baseline stability (low false activation),
    • stationary-person detection lift,
    • robustness after reset/recalibration,
    • behavior at different packet rates.
      Without this, we can’t assess precision/recall trade-offs or regression risk.
  3. Technical correctness gaps to fix first
    There are implementation-level issues that should be addressed before merge:

    • breathing filter state is not reset in all relevant reset paths,
    • move semantics consistency is incomplete after adding new state.
      These are subtle but important for runtime correctness and long-term maintainability.
  4. Not aligned with repository workflow yet
    This repository requires algorithm evolution to stay aligned across both stacks and documentation:

    • parity/validation path in micro-espectre,
    • tests that capture the intended behavior,
    • documentation/changelog updates reflecting the new capability.
      That alignment is currently missing.

If you want to continue this feature, please address the points above.
I’ll be happy to review it again.

@francescopace francescopace added the enhancement New feature or request label Apr 4, 2026
@francescopace
francescopace self-requested a review April 4, 2026 16:39
@PeterkoCZ91
PeterkoCZ91 changed the base branch from main to develop April 13, 2026 08:32
@PeterkoCZ91

Copy link
Copy Markdown
Author

recheck

@PeterkoCZ91
PeterkoCZ91 force-pushed the feat/breathing-bandpass-filter branch from a6acdee to 21c9c6e Compare April 13, 2026 11:13
@codecov

codecov Bot commented Apr 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.37864% with 13 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
micro-espectre/src/segmentation.py 75.00% 7 Missing ⚠️
components/espectre/base_detector.cpp 54.54% 5 Missing ⚠️
components/espectre/csi_filters.cpp 96.55% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@PeterkoCZ91
PeterkoCZ91 force-pushed the feat/breathing-bandpass-filter branch from 3b072bf to 843b39b Compare April 13, 2026 16:44
@PeterkoCZ91

Copy link
Copy Markdown
Author

Hi Francesco, thanks for the detailed review. I've addressed all four points:

1. User-facing integration

  • breathing_score is now exposed in get_metrics() when the filter is enabled, making it accessible to downstream consumers
  • get_breathing_score() accessor available on both BaseDetector (C++) and SegmentationContext (Python)
  • The filter is opt-in via enable_breathing parameter (disabled by default), consistent with the lowpass/hampel pattern

2. Validation on real data

  • Added 16 real-data validation tests across all 5 chip datasets (C3, C5, C6, ESP32, S3)
  • Baseline stability: coefficient of variation < 0.5 on all chips (confirms no false periodic signals in empty rooms)
  • Movement data: all scores remain finite and non-negative
  • Note: the absolute score scale is chip/amplitude-dependent, so downstream fusion should compare against a per-environment baseline rather than fixed thresholds

3. Technical correctness

  • Fixed breathing_filter_ in move constructor and move assignment operator (was missing, causing stale state after detector move)
  • Added breathing_filter_init() to clear_buffer() reset path
  • Filter state is now properly managed across all lifecycle events

4. Repository workflow alignment

  • Full Python parity in micro-espectre: BreathingFilter class in filters.py with exact coefficient match to C++
  • SegmentationContext integration following the existing lowpass/hampel filter pattern
  • 34 new tests total: 14 unit tests (init, reset, frequency response, C++ parity), 4 integration tests, 16 real-data validation tests
  • All 78 breathing-related tests pass

The unrelated CI workflow version changes from the original PR have been cleaned up (resolved by rebasing on current develop).

@PeterkoCZ91

Copy link
Copy Markdown
Author

recheck

@PeterkoCZ91

Copy link
Copy Markdown
Author

Hey Francesco, the CLA check is failing but it's not related to my changes — the bot is trying to verify itself:

Missing CLA signatures for: contributors-assistant[bot]

Looks like the CLA workflow config might need an update (also shows deprecation warnings for app-idclient-id).

@francescopace
francescopace force-pushed the feat/breathing-bandpass-filter branch 3 times, most recently from 98275fc to 296d219 Compare April 24, 2026 21:20
… detection

Signed-off-by: Petr <pisakpetr@gmail.com>
Add breathing_filter_ to move constructor, move assignment operator,
and clear_buffer() reset path. Without this, the filter carries stale
state after detector move or buffer cold-restart.

Signed-off-by: Petr <pisakpetr@gmail.com>
Add BreathingFilter class to filters.py with exact coefficient parity
to C++ breathing_filter_apply(). Integrate into SegmentationContext
via enable_breathing parameter, following the existing lowpass/hampel
filter pattern. Add get_breathing_score() accessor and reset support.

Signed-off-by: Petr <pisakpetr@gmail.com>
Add 14 tests covering BreathingFilter:
- Initialization, reset, coefficient parity with C++
- DC rejection, step transient, score consistency
- Frequency response: breathing band passes, fast/slow signals rejected
- Numerical parity with C++ filter math
- SegmentationContext integration: enable/disable, CSI data, reset

Signed-off-by: Petr <pisakpetr@gmail.com>
- Add breathing_score to get_metrics() when filter is enabled
- Add real-data validation tests across 5 chip datasets (C3, C5, C6,
  ESP32, S3): baseline stability (CV < 0.5), movement score
  finite/non-negative, metrics key presence
- Fix test_filters.py docstring to include BreathingFilter

Signed-off-by: Petr <pisakpetr@gmail.com>
@PeterkoCZ91
PeterkoCZ91 force-pushed the feat/breathing-bandpass-filter branch from 296d219 to d844d21 Compare April 25, 2026 15:48
@francescopace

Copy link
Copy Markdown
Owner

Hi @PeterkoCZ91. I removed the CLA check and simply moved to a DCO check, easier and enough for this project.
Coming back to the PR, Thanks again for the work — I’d like to propose a scope adjustment to make this mergeable.

At this stage, please remove the C++ core changes from this PR and focus only on the Micro-ESPectre side (Python), where this belongs as an experimental/research feature.

Rationale:

  • We still need strong real-world evidence that stationary-presence-from-breathing works reliably.
  • Shipping partial C++ core plumbing before feature-level validation adds production risk without user-facing benefit.
  • Project workflow is prototype/validate in Micro-ESPectre first, then port to ESPectre C++.

Please keep in this PR:

  • micro-espectre/src/* breathing implementation
  • related Python tests and validation tooling
  • documentation of collection/validation protocol

Please remove from this PR:

  • components/espectre/* C++ changes

Validation required before any future C++ port:

  1. Real sample collection using ./me collect (baseline + stationary-presence scenarios, repeated sessions, multi-environment/chip).
  2. Feature-level metrics on real data (not only signal stability): precision/recall/FP trade-offs for occupied-stationary vs empty baseline.
  3. Reproducible protocol and result summary.

Once the Python/R&D track is merged and evidence is solid, I will take care of the C++ port in a dedicated follow-up PR.

@francescopace francescopace added the platform: micro-espectre Python R&D platform label Apr 26, 2026
@francescopace
francescopace force-pushed the develop branch 2 times, most recently from d2f9bd9 to 29e457a Compare May 21, 2026 10:54
@github-actions

Copy link
Copy Markdown

This PR has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. If you're still working on this, please leave a comment or push new commits.

@github-actions github-actions Bot added the stale No activity label Jun 21, 2026
@github-actions

Copy link
Copy Markdown

This PR has been automatically closed due to inactivity. Feel free to reopen if you'd like to continue working on it.

@github-actions github-actions Bot closed this Jun 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request platform: micro-espectre Python R&D platform stale No activity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants