Skip to content

fix: stop ultrasonic trigger pulses when measurement is interrupted - #314

Open
YashAnand69 wants to merge 1 commit into
fossasia:mainfrom
YashAnand69:fix/ultrasonic-trigger-cleanup
Open

YashAnand69 wants to merge 1 commit into
fossasia:mainfrom
YashAnand69:fix/ultrasonic-trigger-cleanup

Conversation

@YashAnand69

@YashAnand69 YashAnand69 commented Oct 10, 2026 •

Copy link
Copy Markdown

Interrupting HC-SR04 distance measurement with Ctrl+C during its acquisition wait leaves the recurring trigger pulses enabled. The command that returns the trigger output LOW is skipped when the wait raises.

Put the existing wait in try/finally so the LOW command is attempted before propagating the interruption. Successful capture timing, pulse settings, command order and distance calculation remain unchanged. The logic analyzer is not stopped or reconfigured by this change.

Validation: four interruption regressions fail on unchanged main; four normal/error controls pass. The fixed branch passes eight new tests plus 33 existing hardware-independent CLI, Pico and gas-sensor tests (41 total). Tests construct HCSR04 with controlled instrument interfaces, verify SQ1/SQ4 cleanup, retain the original exception and check successful distance calculation. Configured Black, Flake8, Bandit and pydocstyle, wheel/sdist and Sphinx builds pass; seven existing documentation warnings remain.

No physical PSLab/HC-SR04 or full hardware integration run was performed. Cleanup still requires a working device transport; it cannot guarantee the output changes if the LOW command itself fails. Current rules, issue claims and all open PR patches were checked; no overlapping cleanup fix found. Developed and tested with Codex assistance.

Summary by Sourcery

Guarantee that HC-SR04 measurement interruptions disable the trigger output before propagating the error.

Bug Fixes:

  • Ensure ultrasonic sensor trigger outputs are stopped when measurement acquisition is interrupted, while preserving the original exception.
  • Preserve trigger cleanup across capture timeout and invalid echo errors.

Tests:

  • Add hardware-independent regression coverage for interrupted waits, successful measurements, trigger-channel variants, and capture errors.

Summary by CodeRabbit

  • Bug Fixes
    • The ultrasonic distance sensor now resets its trigger after a measurement, including when the measurement is interrupted or encounters an error. This helps prevent the trigger from being left active after an unsuccessful reading.

@sourcery-ai

sourcery-ai Bot commented Oct 10, 2026

Copy link
Copy Markdown

Sorry @YashAnand69, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 1 day and 18 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Oct 10, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR makes HC-SR04 measurement cleanup exception-safe by placing the existing wait in a try/finally that drives the trigger LOW, and adds controlled-interface tests covering interrupted waits, both trigger pins, command ordering, and existing success/error outcomes.

Sequence diagram for exception-safe HC-SR04 measurement cleanup

sequenceDiagram
    participant Caller
    participant HCSR04
    participant PWM
    participant Timer
    participant LogicAnalyzer

    Caller->>HCSR04: estimate_distance(average)
    HCSR04->>PWM: set_state(trigger HIGH, duty_cycles)
    HCSR04->>Timer: sleep(measure_period * (average + 1))
    alt wait completes
        Timer-->>HCSR04: return
    else wait interrupted
        Timer-->>HCSR04: raise interruption
    end
    HCSR04->>PWM: set_state(trigger LOW)
    alt wait was interrupted
        HCSR04-->>Caller: propagate original interruption
    else wait completed
        HCSR04->>LogicAnalyzer: fetch_data()
        HCSR04-->>Caller: return distance
    end
Loading

File-Level Changes

Change Details Files
Guarantee trigger-output cleanup when acquisition waiting is interrupted while preserving normal measurement behavior.
  • Wrap the post-capture wait in try/finally.
  • Attempt to drive the configured trigger pin LOW before propagating the original exception.
  • Leave capture, timing, pulse generation, fetch, and distance-calculation flow unchanged.
pslab/external/hcsr04.py
Add hardware-independent regression coverage for interruption cleanup and unchanged successful/error behavior.
  • Test KeyboardInterrupt and RuntimeError cleanup for SQ1 and SQ4 while preserving exception identity and skipping fetch.
  • Verify successful command order, pulse parameters, timing, cleanup, and distance calculation.
  • Verify timeout and high-echo errors still occur after the trigger is stopped.
tests/test_hcsr04_cleanup.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3a547c83-97cf-41f5-8bad-b3aea7e4a6be

📥 Commits

Reviewing files that changed from the base of the PR and between 08de0d0 and 2dfd4e3.


📒 Files selected for processing (2)
  • pslab/external/hcsr04.py
  • tests/test_hcsr04_cleanup.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.



📝 Walkthrough

Walkthrough

estimate_distance now resets the trigger pin in a finally block. Tests check trigger cleanup when waiting raises, when capture data times out, when the initial echo state is high, and after a successful measurement.

Changes

HCSR04 trigger cleanup

Layer / File(s) Summary
Trigger reset and measurement tests
pslab/external/hcsr04.py, tests/test_hcsr04_cleanup.py
estimate_distance resets the trigger pin in a finally block. Mock-based tests cover wait exceptions, successful measurements, timeouts, and a high initial echo state.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix


Merge Risk | ⚪ Minimal · up to 2dfd4

Merge Risk: ⚪ Minimal · up to 2dfd4

This change makes the HC-SR04 trigger output get stopped when a measurement is interrupted, and normal measurements behave as before. It was not tested on physical hardware, but no concrete merge-blocking risk remains.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 2dfd4

The change improves interruption cleanup without expanding access to the device. Remaining uncertainty concerns failed cleanup commands and overlapping use of the same hardware; stopping the output is not guaranteed in those conditions.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The direct changed outcome is a LOW command for the configured trigger on the existing PSLab connection after an interrupted wait. Any interference with another operation sharing that hardware remains within the existing shared-device control scope; supported concurrent sharing is not established.

Trust Boundaries and Controls

  • observed — The device-transport boundary remains unchanged. Cleanup uses the same device command and acknowledgement sequence that previously ran after successful waits, rather than acquiring additional privileges or selecting a new destination.

Resilience and Maintainability Implications

  • inferred — Finally improves containment of interrupted waits but provides an attempted stop, not an unconditional hardware-state guarantee. The unchanged set_state implementation updates local channel state before transmitting the command, so a transport failure can leave local state inconsistent with hardware. That failure mode predates this PR, although cleanup is now attempted on additional exception paths.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: stopping ultrasonic trigger pulses when measurement is interrupted.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant