-
Notifications
You must be signed in to change notification settings - Fork 240
Add MockHandler and allow ScienceLab(mock=True) #274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,114 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||
| """Mock connection handler for PSLab. | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| This module provides a minimal in-memory `ConnectionHandler` implementation for | ||||||||||||||||||||||||||||||||||||||||||||||||||
| use in tests and development without physical PSLab hardware. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||||||||||||||||||||||||||||||||||
| from collections import deque | ||||||||||||||||||||||||||||||||||||||||||||||||||
| import pslab.protocol as CP | ||||||||||||||||||||||||||||||||||||||||||||||||||
| from pslab.connection.connection import ConnectionHandler | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| class MockHandler(ConnectionHandler): | ||||||||||||||||||||||||||||||||||||||||||||||||||
| """In-memory mock implementation of `ConnectionHandler`. | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| The handler queues deterministic responses based on bytes written via | ||||||||||||||||||||||||||||||||||||||||||||||||||
| `write()` so higher-level code can be exercised without an actual device. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| def __init__(self, version: str = "PSLab V6 ", fw=(3, 0, 0)) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| def __init__(self, version: str = "PSLab V6 ", fw=(3, 0, 0)) -> None: | |
| def __init__(self, version: str = "PSLab V6 ", fw: tuple[int, int, int] = (3, 0, 0)) -> None: |
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fw parameter is not validated. If non-integer values or values outside the valid byte range (0-255) are provided, it will cause an error later when constructing the response in _maybe_respond(). Consider adding validation in __init__ to ensure fw contains three integers in the valid range, or document this requirement in the docstring.
| Firmware version as a (major, minor, patch) tuple. Default is (3, 0, 0). | |
| """ | |
| self._rx = deque() # bytes to be read | |
| self._tx = bytearray() # bytes written by client | |
| self.version = version # convenient attribute for callers | |
| self._fw = fw | |
| Firmware version as a (major, minor, patch) tuple of integers in the | |
| range 0–255. Default is (3, 0, 0). | |
| """ | |
| self._rx = deque() # bytes to be read | |
| self._tx = bytearray() # bytes written by client | |
| self.version = version # convenient attribute for callers | |
| # Validate firmware version: must be three integers in byte range 0–255 | |
| if not isinstance(fw, (tuple, list)) or len(fw) != 3: | |
| raise ValueError( | |
| "fw must be a tuple or list of three integers in the range 0–255" | |
| ) | |
| for part in fw: | |
| if not isinstance(part, int) or not (0 <= part <= 255): | |
| raise ValueError( | |
| "fw must contain three integers in the range 0–255" | |
| ) | |
| self._fw = tuple(fw) |
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The __init__ method should have a Parameters section in its docstring following the NumPy docstring conventions used throughout the codebase. The version and fw parameters should be documented. For example:
Parameters
version : str, optional
Version string to return when queried. Default is "PSLab V6 ".
fw : tuple of int, optional
Firmware version tuple (major, minor, patch). Default is (3, 0, 0).
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When insufficient bytes are available in the receive buffer, this method returns fewer bytes than requested instead of blocking or raising an exception. This differs from the behavior of real connection handlers (which would timeout). Consider documenting this behavior, or optionally raising an exception when insufficient data is available to help catch test setup issues early.
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comparison logic is incorrect. self._tx[0] returns an integer (since _tx is a bytearray), but CP.COMMON, CP.GET_VERSION, and CP.GET_FW_VERSION are bytes objects created with Byte.pack(). Comparing an integer with a bytes object will always be False, so the mock handler won't correctly detect commands. The comparisons should unpack the protocol constants to get the integer values, for example: if self._tx[0] != CP.Byte.unpack(CP.COMMON)[0]:.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (testing): The version string is truncated/padded to 9 bytes; confirming this matches protocol expectations might avoid subtle bugs.
The mock forces a 9-byte GET_VERSION reply via [:9].ljust(9, b" "). If the real device’s version length or encoding differs, tests may diverge from real behavior. Prefer deriving the length from a shared protocol constant (e.g., in CP or ConnectionHandler), or add an assertion that the mock length matches what the production handler expects.
Suggested implementation:
from collections import deque
import pslab.protocol as CP
from pslab.connection.connection import ConnectionHandler
# Length of the GET_VERSION reply in bytes.
# Prefer a shared protocol constant if available to keep the mock in sync
# with the real ConnectionHandler implementation.
GET_VERSION_REPLY_LEN = getattr(CP, "GET_VERSION_REPLY_LEN", 9) # GET_VERSION: ConnectionHandler.get_version reads GET_VERSION_REPLY_LEN bytes
# and checks b"PSLab"
if cmd == CP.GET_VERSION:
self._tx = self._tx[2:]
version_bytes = self.version.encode("utf-8")
reply_len = GET_VERSION_REPLY_LEN
assert reply_len > 0, "GET_VERSION_REPLY_LEN must be positive"
self._queue(version_bytes[:reply_len].ljust(reply_len, b" "))
continueIf the production protocol already defines a constant length for the GET_VERSION payload (for example, CP.GET_VERSION_REPLY_LEN or something similar), it would be better to:
- Add that constant to
pslab.protocol(if it does not exist yet). - Ensure
GET_VERSION_REPLY_LENin this mock module is set directly from that constant (and possibly drop thegetattr(..., 9)fallback to fail fast if the protocol changes without updating the mock).
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comparison logic is incorrect. self._tx[1] (an integer) is being compared with CP.GET_VERSION and CP.GET_FW_VERSION (bytes objects). This will always be False. The comparisons should unpack the protocol constants to get the integer values, for example: if cmd == CP.Byte.unpack(CP.GET_VERSION)[0]:.
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an unknown command is encountered under CP.COMMON, the loop breaks instead of continuing to process remaining bytes. This means if multiple commands are written in sequence and one is unknown, subsequent commands won't be processed. Consider using continue after incrementing past the unknown command, or at minimum, consuming the unknown command bytes to prevent them from blocking future command processing.
| # Unknown command under CP.COMMON: drop and stop | |
| break | |
| # Unknown command under CP.COMMON: drop it and continue parsing | |
| self._tx = self._tx[2:] | |
| continue |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,7 +11,7 @@ | |||||||||||||||||||||||||||||||||
| from typing import Iterable, List | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| import pslab.protocol as CP | ||||||||||||||||||||||||||||||||||
| from pslab.connection import ConnectionHandler, SerialHandler, autoconnect | ||||||||||||||||||||||||||||||||||
| from pslab.connection import ConnectionHandler, SerialHandler, MockHandler, autoconnect | ||||||||||||||||||||||||||||||||||
| from pslab.instrument.logic_analyzer import LogicAnalyzer | ||||||||||||||||||||||||||||||||||
| from pslab.instrument.multimeter import Multimeter | ||||||||||||||||||||||||||||||||||
| from pslab.instrument.oscilloscope import Oscilloscope | ||||||||||||||||||||||||||||||||||
|
|
@@ -34,15 +34,31 @@ class ScienceLab: | |||||||||||||||||||||||||||||||||
| nrf : pslab.peripherals.NRF24L01 | ||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||
| nrf : pslab.peripherals.NRF24L01 | |
| nrf : pslab.peripherals.NRF24L01 | |
| These attributes are set to None when initialized with mock=True. |
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring for the ScienceLab class should document the new mock parameter. Following the pattern seen in other classes in the codebase (e.g., LogicAnalyzer), a Parameters section should be added documenting both the device and mock parameters. For example:
Parameters
device : ConnectionHandler, optional
Connection handler for communicating with the PSLab device. If not provided, a new one will be created via autoconnect.
mock : bool, optional
If True, use a mock handler instead of connecting to physical hardware. Instruments will not be instantiated in mock mode. The default is False.
| nrf : pslab.peripherals.NRF24L01 | |
| nrf : pslab.peripherals.NRF24L01 | |
| Parameters | |
| ---------- | |
| device : ConnectionHandler, optional | |
| Connection handler for communicating with the PSLab device. If not | |
| provided, a new one will be created via autoconnect. | |
| mock : bool, optional | |
| If True, use a mock handler instead of connecting to physical | |
| hardware. Instruments will not be instantiated in mock mode. The | |
| default is False. |
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When both device and mock=True are provided, the device parameter silently takes precedence and the mock parameter is ignored. This behavior may be unexpected for users. Consider either raising a ValueError when both are provided, or documenting this precedence in the docstring if it's intentional.
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment is positioned awkwardly between the firmware retrieval and the conditional initialization logic. Consider either moving it to be a docstring parameter description or rephrasing it as an inline comment closer to the conditional logic, such as: "# In mock mode, skip instrument initialization to avoid hardware dependencies".
Copilot
AI
Feb 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conditional logic here only checks the mock parameter, which can lead to incorrect behavior when both device and mock are provided. For example:
ScienceLab(device=MockHandler(), mock=False)would attempt to initialize instruments with a MockHandlerScienceLab(device=real_device, mock=True)would skip instrument initialization despite having a real device
The logic should determine whether to initialize instruments based on the actual type of the device, not just the mock parameter. Consider checking isinstance(self.device, MockHandler) instead of relying solely on the mock parameter value.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,25 @@ | ||||||||||||||||||
| from unittest.mock import patch | ||||||||||||||||||
|
|
||||||||||||||||||
| from pslab.sciencelab import ScienceLab | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| def test_sciencelab_mock_does_not_autoconnect(): | ||||||||||||||||||
| # If autoconnect is called, the test should fail immediately. | ||||||||||||||||||
| with patch( | ||||||||||||||||||
| "pslab.sciencelab.autoconnect", | ||||||||||||||||||
| side_effect=AssertionError("autoconnect should not be called"), | ||||||||||||||||||
| ): | ||||||||||||||||||
| psl = ScienceLab(mock=True) | ||||||||||||||||||
|
|
||||||||||||||||||
| # It should initialize and provide a firmware version object. | ||||||||||||||||||
| assert psl.firmware.major >= 0 | ||||||||||||||||||
| assert psl.firmware.minor >= 0 | ||||||||||||||||||
| assert psl.firmware.patch >= 0 | ||||||||||||||||||
|
||||||||||||||||||
| # It should initialize and provide a firmware version object. | |
| assert psl.firmware.major >= 0 | |
| assert psl.firmware.minor >= 0 | |
| assert psl.firmware.patch >= 0 | |
| # It should initialize and provide the expected mock firmware version object. | |
| assert psl.firmware.major == 3 | |
| assert psl.firmware.minor == 0 | |
| assert psl.firmware.patch == 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statement uses an absolute path
from pslab.connection.mock import MockHandlerwhile other imports in this file use relative imports (e.g.,from .connection import ConnectionHandler). For consistency with the existing import style, this should be changed tofrom .mock import MockHandler.