Replace deprecated serial_handler imports with pslab.connection#267
Replace deprecated serial_handler imports with pslab.connection#267Architrb1795 wants to merge 2 commits intofossasia:developfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR replaces deprecated pslab.serial_handler imports with the modern pslab.connection.SerialHandler and updates internal docs to reference the new module, eliminating deprecation warnings during docs builds. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Hi maintainers, |
There was a problem hiding this comment.
Pull request overview
This PR updates the codebase to stop importing the deprecated pslab.serial_handler module (which emits a deprecation warning during docs builds) and instead use the modern pslab.connection API.
Changes:
- Switched
SerialHandlerimports frompslab.serial_handlertopslab.connectionin the CLI and external sensor drivers. - Updated internal documentation to reference
pslab.connectioninstead of the deprecated module.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pslab/cli.py | Updates SerialHandler import to pslab.connection. |
| pslab/external/gas_sensor.py | Updates SerialHandler import to pslab.connection. |
| pslab/external/hcsr04.py | Updates SerialHandler import to pslab.connection. |
| pslab/external/tcd1304.py | Updates SerialHandler import to pslab.connection. |
| docs/internal.rst | Updates autodoc target module to pslab.connection to avoid deprecated import warnings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pslab/cli.py
Outdated
| from pslab.instrument.oscilloscope import Oscilloscope | ||
| from pslab.instrument.waveform_generator import WaveformGenerator, PWMGenerator | ||
| from pslab.serial_handler import SerialHandler | ||
| from pslab.connection import SerialHandler |
There was a problem hiding this comment.
pslab.connection.SerialHandler has different runtime behavior than the deprecated pslab.serial_handler.SerialHandler: it requires an explicit port (no implicit autoconnect when args.port is None) and it does not auto-connect() in __init__. In main() this file currently does handler = SerialHandler(port=args.port) and then uses it without calling handler.connect(), which will leave the serial port closed (and may also pass None as the port). Update CLI initialization to use autoconnect() when no port is provided and/or call handler.connect() before use (and ensure disconnect() in a finally).
| from pslab.connection import SerialHandler | |
| from pslab.serial_handler import SerialHandler |
mariobehling
left a comment
There was a problem hiding this comment.
Thanks for the PR. Cleaning up the deprecated pslab.serial_handler imports is a good direction, and the change set is nicely small.
However, this is not a pure warning cleanup. pslab.connection.SerialHandler can behave differently than the deprecated handler, and in pslab/cli.py the code currently creates the handler and uses it without an explicit connect(), while args.port may be None. That can leave the port closed or fail at runtime.
Please do the following before we can merge:
-
Open an issue describing the deprecation cleanup and the expected runtime behaviour of the CLI, then link it in the PR description using GitHub best practice:
Fixes #<issue-number>. -
Add a short screencast showing this working on your machine:
tox -e docswith the warning removed- running the CLI command you tested, showing it successfully connects (include the exact command and whether a port is provided or auto detected)
-
Update the CLI initialization to match the intended
pslab.connection.SerialHandlerusage, for example callconnect()explicitly or use the proper auto connect flow, and ensure clean disconnect in a safe way.
Once the CLI behaviour is verified and captured in the issue and screencast, this becomes a clean and mergeable deprecation removal.
|
Small process note. We have automatic Copilot PR reviews enabled on this repository. These reviews are only triggered if the contributor has GitHub Copilot enabled and an active license on their own account. Please enable Copilot in your GitHub settings if you have access. In many regions, free licenses are available through educational institutions or developer programs. Enabling Copilot helps us speed up the auto review process and reduces manual review overhead for the core team. The team will review your PR. Thank you for your contribution and cooperation. |
|
https://drive.google.com/file/d/1GLzB5s8oU4dDxOSFgL_CdgA9FAJZAwgg/view?usp=sharing Thanks for the review! I’ve updated the CLI to explicitly manage the connection lifecycle when migrating to I’ve also attached a short screencast demonstrating:
Please let me know if any further adjustments are needed. |
Fixes #271
Summary
This PR migrates usage of the deprecated
pslab.serial_handlermodule to the modernpslab.connection.SerialHandlerAPI and updates the CLI to explicitly manage the connection lifecycle.Motivation
The
pslab.serial_handlermodule is deprecated and emits warnings duringdocumentation builds and runtime.While migrating to
pslab.connection.SerialHandler, a behavioral difference was identified: the new API does not auto-connect on initialization, whereas the deprecated implementation did.This caused the CLI (
pslab/cli.py) to rely on implicit behavior that no longer exists, potentially leading to runtime failures.Changes Made
pslab.serial_handlerimports withpslab.connection.SerialHandlerin:pslab/cli.pypslab/external/gas_sensor.pypslab/external/hcsr04.pypslab/external/tcd1304.pyhandler.connect()handler.disconnect()is always called via a safe lifecycleVerification
tox -e docs(deprecation warning removed)tox -e lint(clean)