Repository navigation
fix: validate logic capture modes before clearing data - #313
Open
YashAnand69 wants to merge 1 commit into
Open
YashAnand69 wants to merge 1 commit into
YashAnand69 wants to merge 1 commit into
Conversation
Reviewer's GuideThe capture path now resolves mode names before any state mutation or device command, preventing invalid requests from destroying an existing acquisition while preserving KeyError behavior and the valid-mode protocol. New mock-connection tests cover state preservation across channel and mode combinations and confirm unchanged behavior for all supported modes. Sequence diagram for logic analyzer capture mode validationsequenceDiagram
participant Caller
participant LogicAnalyzer
participant Device
Caller->>LogicAnalyzer: capture(channels, events, modes)
LogicAnalyzer->>LogicAnalyzer: MODES[m]
alt invalid mode
MODES[m]-->>LogicAnalyzer: KeyError
LogicAnalyzer-->>Caller: KeyError
Note over LogicAnalyzer,Device: Existing acquisition and device state remain unchanged
else valid modes
LogicAnalyzer->>LogicAnalyzer: _check_arguments(channels, events)
LogicAnalyzer->>LogicAnalyzer: stop()
LogicAnalyzer->>Device: clear_buffer(CP.MAX_SAMPLES)
LogicAnalyzer->>LogicAnalyzer: _invalidate_buffer()
LogicAnalyzer->>Device: _configure_trigger(channels)
LogicAnalyzer->>Device: capture protocol commands
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk | ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 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 Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: validating logic capture modes before clearing data. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check 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.
Comment @coderabbitai help to get the list of available commands.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This branch has not been deployed
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.
An invalid logic-analyzer capture mode currently raises only after stopping the previous capture, clearing the device buffer and invalidating its local metadata. A typo can therefore destroy an acquisition before reporting the error.
Resolve the mode names before changing channel mappings or issuing device commands. Preserve the existing KeyError and valid-mode protocol behavior. This change does not expand the set of accepted mode names.
Validation: ten regressions fail on unchanged main and six valid-mode controls pass. All 16 new cases plus 33 existing hardware-independent CLI, Pico and gas-sensor tests pass (49 total). Configured Black, Flake8, Bandit and pydocstyle checks, package and Sphinx builds pass; existing documentation warnings remain. Tests record device commands and verify all prior channel metadata is preserved. No physical-device or full hardware integration validation was performed. Developed and tested with Codex assistance.
No matching issue ownership claim or overlapping open PR found. Capture timing, event counts and firmware protocol remain unchanged.
Summary by Sourcery
Validate logic-analyzer capture modes before modifying acquisition state or issuing device commands.
Bug Fixes:
Tests:
Summary by CodeRabbit