Conversation
📝 WalkthroughWalkthroughThis PR introduces the jumpstarter-driver-noyito-relay package, a new driver supporting NOYITO relay boards via serial (NoyitoPowerSerial) and HID (NoyitoPowerHID) interfaces. Includes implementation, client, comprehensive documentation, examples, and test suite. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Driver as NoyitoPowerSerial<br/>or<br/>NoyitoPowerHID
participant HW as Serial Port<br/>or HID Device
Client->>Driver: on() / off()
activate Driver
Driver->>Driver: _build_command(channel, state)
Driver->>Driver: _channels() - determine targets
Driver->>HW: _send_command(bytes)
HW-->>Driver: ✓ sent
deactivate Driver
Driver-->>Client: return
Client->>Driver: status() [Serial only]
activate Driver
Driver->>HW: _query_status()
HW-->>Driver: response text
Driver->>Driver: parse status dict
deactivate Driver
Driver-->>Client: "on"/"off"/"partial"
Client->>Driver: read()
activate Driver
Driver-->>Client: Generator[PowerReading]
deactivate Driver
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
kirkbrauer
force-pushed
the
driver-noyito-power
branch
from
c717a9b to
10ce9f9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
python/packages/jumpstarter-driver-noyito-relay/.gitignore (1)
1-3: LGTM! Appropriate ignore patterns for Python package.The patterns correctly exclude Python bytecode caches and test coverage artifacts, which aligns with the test suite described in the PR.
Optionally, you may consider adding additional common Python patterns if they're not already covered by a root
.gitignore:
.pytest_cache/*.egg-info/dist/build/htmlcov/However, this is perfectly adequate if those are handled at the repository level.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-noyito-relay/.gitignore` around lines 1 - 3, Current .gitignore correctly excludes __pycache__, .coverage and coverage.xml; optionally add common Python ignore patterns to the same .gitignore (or confirm they're in the root) by including entries for .pytest_cache/, *.egg-info/, dist/, build/, and htmlcov/ so build artifacts and pytest caches are also ignored; update the file named .gitignore in the package (python/packages/jumpstarter-driver-noyito-relay/.gitignore) to add those lines if the repo root does not already cover them.python/packages/jumpstarter-driver-noyito-relay/pyproject.toml (1)
4-4: Package description undersells current functionality.The description mentions only the 2-channel serial board, but this package also ships HID 4/8-channel support. Updating this helps avoid confusion in package indexes.
💡 Proposed tweak
-description = "Jumpstarter driver for the NOYITO 5V 2-Channel USB Relay Module" +description = "Jumpstarter driver for NOYITO USB relay boards (serial 1/2-channel and HID 4/8-channel)"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-noyito-relay/pyproject.toml` at line 4, Update the package description value in pyproject.toml (the description field) to reflect all supported hardware: change the current "Jumpstarter driver for the NOYITO 5V 2-Channel USB Relay Module" to a broader string that mentions both the 2-channel serial board and HID 4/8-channel support (e.g., include "2-channel serial and HID 4/8-channel" or similar wording) so package indexes correctly describe the package capabilities.python/packages/jumpstarter-driver-noyito-relay/jumpstarter_driver_noyito_relay/driver_test.py (1)
217-312: Add a regression test for HID/status capability mismatch.Please add one integration-style test that verifies
statusis unavailable (or fails with a clear error) forNoyitoPowerHID. It would lock in expected behavior and guard against CLI/client drift.Based on learnings Add comprehensive tests and update documentation in the driver package.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@python/packages/jumpstarter-driver-noyito-relay/jumpstarter_driver_noyito_relay/driver_test.py` around lines 217 - 312, Add an integration test that asserts the status capability is not present for NoyitoPowerHID: create a new test function (e.g., test_hid_status_unavailable) in driver_test.py that patches hid.Device (reuse _make_hid_mock and the `@patch`("hid.Device") pattern), opens the device with serve(NoyitoPowerHID(...)) as client, and then assert that either getattr(client, "status", None) is None or that calling client.status() raises a clear exception (AttributeError or NotImplementedError) to lock in expected behavior; reference NoyitoPowerHID, serve, and _make_hid_mock when locating where to add the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@python/docs/source/reference/package-apis/drivers/noyito-relay.md`:
- Line 1: The docs page "noyito-relay.md" is not included in any Sphinx toctree;
open the drivers toctree index (the drivers reference index used for package
driver docs) and add an entry for "noyito-relay.md" to the toctree so Sphinx
will include it in the generated reference docs. Ensure the toctree entry
matches the document name "noyito-relay.md" (or the same relative doc path used
by other driver entries) and update the drivers index file so the page is
discoverable in navigation.
In
`@python/packages/jumpstarter-driver-noyito-relay/jumpstarter_driver_noyito_relay/conftest.py`:
- Around line 16-20: Replace the direct call to item.runtest() with a pytest
hookwrapper: add `@pytest.hookimpl`(hookwrapper=True) to pytest_runtest_call,
yield to let pytest run the test, capture the outcome via outcome = yield, then
call outcome.get_result() inside a try/except that catches
serial.SerialException and calls pytest.skip("Serial device not available");
ensure serial is imported and remove the manual item.runtest() invocation so
pytest's runner and plugins handle execution and reporting correctly.
In
`@python/packages/jumpstarter-driver-noyito-relay/jumpstarter_driver_noyito_relay/driver.py`:
- Around line 118-120: The client() classmethod on NoyitoPowerHID incorrectly
returns the status-capable
"jumpstarter_driver_noyito_relay.client.NoyitoPowerClient" causing CLI to expose
a status command for HID; update NoyitoPowerHID.client() to return the proper
HID-only client (or the module/class that does not export status) — e.g. point
it at the HID-specific client class name (the one that does not implement
status) instead of NoyitoPowerClient, or alternatively implement/export a safe
status() on NoyitoPowerHID if status behavior is required.
---
Nitpick comments:
In `@python/packages/jumpstarter-driver-noyito-relay/.gitignore`:
- Around line 1-3: Current .gitignore correctly excludes __pycache__, .coverage
and coverage.xml; optionally add common Python ignore patterns to the same
.gitignore (or confirm they're in the root) by including entries for
.pytest_cache/, *.egg-info/, dist/, build/, and htmlcov/ so build artifacts and
pytest caches are also ignored; update the file named .gitignore in the package
(python/packages/jumpstarter-driver-noyito-relay/.gitignore) to add those lines
if the repo root does not already cover them.
In
`@python/packages/jumpstarter-driver-noyito-relay/jumpstarter_driver_noyito_relay/driver_test.py`:
- Around line 217-312: Add an integration test that asserts the status
capability is not present for NoyitoPowerHID: create a new test function (e.g.,
test_hid_status_unavailable) in driver_test.py that patches hid.Device (reuse
_make_hid_mock and the `@patch`("hid.Device") pattern), opens the device with
serve(NoyitoPowerHID(...)) as client, and then assert that either
getattr(client, "status", None) is None or that calling client.status() raises a
clear exception (AttributeError or NotImplementedError) to lock in expected
behavior; reference NoyitoPowerHID, serve, and _make_hid_mock when locating
where to add the test.
In `@python/packages/jumpstarter-driver-noyito-relay/pyproject.toml`:
- Line 4: Update the package description value in pyproject.toml (the
description field) to reflect all supported hardware: change the current
"Jumpstarter driver for the NOYITO 5V 2-Channel USB Relay Module" to a broader
string that mentions both the 2-channel serial board and HID 4/8-channel support
(e.g., include "2-channel serial and HID 4/8-channel" or similar wording) so
package indexes correctly describe the package capabilities.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
python/docs/source/reference/package-apis/drivers/noyito-relay.mdpython/packages/jumpstarter-driver-noyito-relay/.gitignorepython/packages/jumpstarter-driver-noyito-relay/README.mdpython/packages/jumpstarter-driver-noyito-relay/examples/exporter.yamlpython/packages/jumpstarter-driver-noyito-relay/jumpstarter_driver_noyito_relay/__init__.pypython/packages/jumpstarter-driver-noyito-relay/jumpstarter_driver_noyito_relay/client.pypython/packages/jumpstarter-driver-noyito-relay/jumpstarter_driver_noyito_relay/conftest.pypython/packages/jumpstarter-driver-noyito-relay/jumpstarter_driver_noyito_relay/driver.pypython/packages/jumpstarter-driver-noyito-relay/jumpstarter_driver_noyito_relay/driver_test.pypython/packages/jumpstarter-driver-noyito-relay/pyproject.toml
| @@ -0,0 +1 @@ | |||
| ../../../../../packages/jumpstarter-driver-noyito-relay/README.md No newline at end of file | |||
There was a problem hiding this comment.
Docs page is currently orphaned from Sphinx navigation.
The docs pipeline warning shows this document is not included in any toctree, so it won’t be discoverable in generated reference docs. Please add it to the appropriate drivers toctree index.
🧰 Tools
🪛 GitHub Actions: Documentation
[warning] 1-1: Sphinx warning: document isn't included in any toctree
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@python/docs/source/reference/package-apis/drivers/noyito-relay.md` at line 1,
The docs page "noyito-relay.md" is not included in any Sphinx toctree; open the
drivers toctree index (the drivers reference index used for package driver docs)
and add an entry for "noyito-relay.md" to the toctree so Sphinx will include it
in the generated reference docs. Ensure the toctree entry matches the document
name "noyito-relay.md" (or the same relative doc path used by other driver
entries) and update the drivers index file so the page is discoverable in
navigation.
| def pytest_runtest_call(item): | ||
| try: | ||
| item.runtest() | ||
| except serial.SerialException: | ||
| pytest.skip("Serial device not available") # ty: ignore[call-non-callable] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual conftest.py file
cat -n python/packages/jumpstarter-driver-noyito-relay/jumpstarter_driver_noyito_relay/conftest.pyRepository: jumpstarter-dev/jumpstarter
Length of output: 871
🏁 Script executed:
# Run the verification script from the review comment
#!/bin/bash
set -euo pipefail
echo "Custom pytest_runtest_call hooks:"
rg -n --type=py 'def pytest_runtest_call\('
echo
echo "Direct calls to item.runtest() (should be absent in custom hooks):"
rg -n --type=py 'item\.runtest\s*\('
echo
echo "All pytest hook declarations:"
rg -n --type=py 'def pytest_' python/packages/jumpstarter-driver-noyito-relay/Repository: jumpstarter-dev/jumpstarter
Length of output: 1013
🌐 Web query:
pytest_runtest_call hook semantics documentation
💡 Result:
pytest_runtest_call(item) is a runtest-phase hook that pytest calls to execute the “call” phase of a single test item (i.e., the actual test body, after setup and before teardown). Its default implementation is essentially to run the test via item.runtest(). [1]
Where it fits in the per-item runtest protocol:
pytest_runtest_logstart(...)happens before setup/call/teardown. [2]pytest_runtest_setup(item)runs beforepytest_runtest_call(item). [2]pytest_runtest_call(item)runs the test body (call phase). [1]pytest_runtest_teardown(item, nextitem)runs teardown after the call phase. [1]pytest_runtest_logfinish(...)happens after setup/call/teardown. [2]
Implementation/usage notes:
- You can implement this hook in
conftest.py(subject to the usual conftest visibility rules). [1] - Unlike most non-
pytest_runtest_*hooks, runtest hooks are allowed to raise exceptions as part of reporting test failures/errors. [3]
Docs: see the pytest “API Reference” entry for pytest_runtest_call. [1]
Sources:
[1] pytest docs – API Reference (pytest_runtest_call) (docs.pytest.org)
[2] pytest docs – runtest hook order (logstart, setup, call, teardown, logfinish) (docs.pytest.org)
[3] pytest docs – hook-writing notes (exception rules; runtest hooks special) (docs.pytest.org)
Citations:
- 1: https://docs.pytest.org/en/stable/reference/reference.html?utm_source=openai
- 2: https://docs.pytest.org/en/4.6.x/reference.html?utm_source=openai
- 3: https://docs.pytest.org/en/7.1.x/how-to/writing_hook_functions.html?utm_source=openai
Implement pytest_runtest_call using the hookwrapper pattern to properly integrate with pytest's test execution protocol.
The current implementation directly calls item.runtest() without using @pytest.hookimpl(hookwrapper=True) and yield. This violates pytest's runtest hook semantics, which expects hooks to yield control back to pytest's runner rather than manually executing the test. Improper exception handling also prevents integration with pytest's error reporting and other plugins.
💡 Proposed fix
+@pytest.hookimpl(hookwrapper=True)
def pytest_runtest_call(item):
- try:
- item.runtest()
- except serial.SerialException:
- pytest.skip("Serial device not available") # ty: ignore[call-non-callable]
+ outcome = yield
+ if outcome.excinfo and issubclass(outcome.excinfo[0], serial.SerialException):
+ pytest.skip("Serial device not available")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@python/packages/jumpstarter-driver-noyito-relay/jumpstarter_driver_noyito_relay/conftest.py`
around lines 16 - 20, Replace the direct call to item.runtest() with a pytest
hookwrapper: add `@pytest.hookimpl`(hookwrapper=True) to pytest_runtest_call,
yield to let pytest run the test, capture the outcome via outcome = yield, then
call outcome.get_result() inside a try/except that catches
serial.SerialException and calls pytest.skip("Serial device not available");
ensure serial is imported and remove the manual item.runtest() invocation so
pytest's runner and plugins handle execution and reporting correctly.
| @classmethod | ||
| def client(cls) -> str: | ||
| return "jumpstarter_driver_noyito_relay.client.NoyitoPowerClient" |
There was a problem hiding this comment.
NoyitoPowerHID points to a status-capable client, but HID has no status export.
NoyitoPowerHID.client() currently returns NoyitoPowerClient, which adds status()/status CLI command. That command can be invoked for HID and then fail at runtime because NoyitoPowerHID does not implement/export status.
💡 Proposed fix
`@classmethod`
def client(cls) -> str:
- return "jumpstarter_driver_noyito_relay.client.NoyitoPowerClient"
+ return "jumpstarter_driver_power.client.PowerClient"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@python/packages/jumpstarter-driver-noyito-relay/jumpstarter_driver_noyito_relay/driver.py`
around lines 118 - 120, The client() classmethod on NoyitoPowerHID incorrectly
returns the status-capable
"jumpstarter_driver_noyito_relay.client.NoyitoPowerClient" causing CLI to expose
a status command for HID; update NoyitoPowerHID.client() to return the proper
HID-only client (or the module/class that does not export status) — e.g. point
it at the HID-specific client class name (the one that does not implement
status) instead of NoyitoPowerClient, or alternatively implement/export a safe
status() on NoyitoPowerHID if status behavior is required.
There was a problem hiding this comment.
Ahh, the status part is missing in this variant of the driver, can it be implemented?
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
NOYITO USB HID relay board status query support
💡 Result:
Many “NOYITO USB HID relay” boards are re-branded LCUS/CH-style HID relays that use the simple 4‑byte 0xA0 command protocol (often sent to the device’s HID endpoint / hidraw). A Linux user report for LCUS_HID_X1 shows the NOYITO/LCUS HID variant accepts the same A0 … on/off frames over HID. [2]
Does it support relay status queries?
Yes—if your NOYITO board uses this 0xA0 protocol, it supports a query operation where the 3rd byte is 0x02 (“query status”), and the device returns text like CH1:ON / CH1:OFF. [1]
Per-channel status query
Send:
A0 <CH> 02 <CHK>
Examples:
- CH1 status:
A0 01 02 A3→ response likeCH1:ON(ASCII) [1] - CH2 status:
A0 02 02 A4→ response likeCH2:ON(ASCII) [1]
All-channel status query
Send:
A0 0F 02 B1→ returns multiple lines (CH1:...,CH2:..., etc.) [1]
Checksum
CHK = (0xA0 + address + operation) & 0xFF (matches the examples above). [1]
Quick way to confirm on your system
If your board is supported by common HID-relay tooling, the usbrelay utility can query and print relay state (with no action it prints all relays). [3]
Sources: Diustou relay protocol + query/response examples [1], LCUS_HID_X1 controlled via A0 ... frames over HID (hidraw) [2], usbrelay tool supports querying HID relay state [3].
@mangelajo Yes, status can be implemented for the HID variant!
The web search confirms that NOYITO HID relay boards using the 0xA0 protocol support status queries:
Protocol for status query:
- Send:
0xA0 <channel> 0x02 <checksum>where checksum =(0xA0 + channel + 0x02) & 0xFF - For single channel:
0xA0 0x01 0x02 0xA3→ returns ASCII"CH1:ON"or"CH1:OFF" - For all channels:
0xA0 0x0F 0x02 0xB1→ returns multiple lines (CH1:...,CH2:..., etc.)
Implementation approach:
The existing code already has most pieces needed:
- The
_build_command(channel, state)helper can be used withstate=0x02for queries - The response format is identical to the Serial variant (ASCII text like
"CH1:ON") - You can adapt the
_query_status()andstatus()methods fromNoyitoPowerSerial(lines 51-96)
The main difference is sending/receiving via HID instead of serial. The _send_command method in NoyitoPowerHID would need a companion _query_status() that:
- Opens the HID device
- Sends the status query command
- Reads the HID response (ASCII text)
- Parses it the same way as the Serial variant
This would also resolve the original issue—once status() is exported on NoyitoPowerHID, the NoyitoPowerClient client becomes appropriate for both driver variants.
Would you like me to plan the implementation of the status query for the HID driver?
|
just a couple of warnings from the bot, and from docs compilation. |
| from jumpstarter_driver_power.client import PowerClient | ||
|
|
||
|
|
||
| class NoyitoPowerClient(PowerClient): |
There was a problem hiding this comment.
I would go ahead and add status and cli to all PowerClients, defaulting to "not implemented", then we can add it to others as well. WDYT?
There was a problem hiding this comment.
Saying because we have been talking about that sometimes. ... anyway, we can do that later TBH, not really this PR, please ignore :D
|
|
||
| @export | ||
| def read(self) -> Generator[PowerReading, None, None]: | ||
| yield PowerReading(voltage=0.0, current=0.0) |
There was a problem hiding this comment.
Here I think we should throw a non implemented exception
2 participants
Add
jumpstarter-driver-noyito-relay— NOYITO USB Relay Power DriverSummary
Adds a new driver package supporting the full NOYITO USB relay board family
across two distinct hardware series:
NoyitoPowerSerial— 1/2-channel boards using a CH340 USB-to-serial chip(9600 baud). Supports the
0xFFstatus query to read channel state.Controlled via
pyserial.NoyitoPowerHID— 4/8-channel "HID Drive-free" boards that enumerate asUSB HID devices (no serial port). Status query is not available on this
hardware. Controlled via the
hidPython package.Both classes share the same 4-byte command protocol
(
0xA0+ channel + state + checksum) via a common_build_commandhelper.Key configuration parameters
NoyitoPowerSerialport,channel(1–2),dualdual=Truefires both contacts simultaneously for high-current wiringNoyitoPowerHIDnum_channels(4 or 8),channel,all_channels,vendor_id,product_idall_channels=Truefires every channel simultaneouslymacOS / Apple Silicon note
The
hidpackage requires the nativehidapishared library(
brew install hidapi). On Apple Silicon, Homebrew installs to/opt/homebrew/libwhich is not indlopen's default search path.NoyitoPowerHID._send_commandautomatically prepends the Homebrew prefix toDYLD_FALLBACK_LIBRARY_PATHbefore the firstimport hid, so no manualenvironment setup is needed on Intel or Apple Silicon Macs.
Files added
jumpstarter_driver_noyito_relay/driver.pyNoyitoPowerSerial+NoyitoPowerHIDdriver classesjumpstarter_driver_noyito_relay/client.pyNoyitoPowerClient(thinPowerClientsubclass)jumpstarter_driver_noyito_relay/driver_test.pyjumpstarter_driver_noyito_relay/conftest.pyhidinsys.modulesso tests run without nativehidapiexamples/exporter.yamlREADME.mdpyproject.tomlpyserial>=3.5,hid>=1.0.4deps; registers both entry pointspython/uv.lockhid 1.0.9from pypi.orgTest plan
hid.Devicemocked (no hardware needed):make pkg-test-jumpstarter-driver-noyito-relayruff checkpasses cleanon/off/status/cycleon/off/cycle,confirm
statusis not availableSummary by CodeRabbit
Release Notes
New Features
Documentation