Skip to content

Polish, CI gates, and CLI smoke tests (P1-P8 + C1/C2 + T1/T4)#1

Merged
bonev-st merged 1 commit into
mainfrom
chore/code-review-followups
May 19, 2026
Merged

Polish, CI gates, and CLI smoke tests (P1-P8 + C1/C2 + T1/T4)#1
bonev-st merged 1 commit into
mainfrom
chore/code-review-followups

Conversation

@bonev-st
Copy link
Copy Markdown
Contributor

Summary

  • Polish (P1-P8): extracted shared int16 range-check helper, named the retry-delay constant, dropped redundant slice/import/try-except code, replaced the linear Register.from_name scan with an O(1) module-level index, documented retry behaviour on each ModbusCommError subclass, and promoted interpret_raw to the public surface.
  • CI gates (C1/C2/T4): pyproject dev/test extras + [tool.ruff] + [tool.mypy] config; GitHub Actions matrix now runs Python 3.10/3.11/3.12 with pip cache, py_compile example.py, ruff lint, mypy type-check, and pytest with coverage.
  • CLI smoke tests (T1): new tests/test_cli.py with 27 tests covering parsers, formatters (incl. the B5 hex-twin regression), --branch deprecation, and main(argv) for list-models / list-registers / port-guard.

Ruff autofixes also normalised imports across the package, rewrote typing.Iterablecollections.abc.Iterable, and removed redundant quoted-string forward references under from __future__ import annotations.

Test plan

  • CI: tests workflow passes on Python 3.10, 3.11, and 3.12
  • CI: ruff check . reports clean
  • CI: mypy smartpower_modbus reports clean
  • CI: python -m py_compile example.py succeeds
  • CI: pytest --cov=smartpower_modbus reports 179 passing and ≥88% line coverage
  • Sanity: python -m smartpower_modbus.cli list-models lists all four SmartPower models
  • Sanity: python -m smartpower_modbus.cli --model SmartPowerGen_2.0 list-registers prints the GEN_2_0 register table
  • Sanity (real hardware, optional): example.py --port <PORT> --slave <ID> still connects, auto-identifies, and reads telemetry

Local verification (already run)

  • pytest -q --cov: 179 passed (152 pre-existing + 27 new), 88% line coverage
  • ruff check .: clean
  • mypy smartpower_modbus: clean
  • python -m py_compile example.py: clean

🤖 Generated with Claude Code

Polish (P1-P8):
- P1: extracted _validate_int16 in client.py; write() and write_value()
  now share the int16/uint16 range-check logic.
- P2: time.sleep(0.05) promoted to a named _RETRY_DELAY_SEC constant.
- P3: dropped redundant bits[:count] slice in read_coils/read_discretes
  (_extract_result already trims pymodbus's whole-byte padding).
- P4: removed redundant lazy UnsupportedRegisterError import inside
  Register.from_name.
- P5: dropped unreachable try/except around bytes.decode(..., errors="replace").
- P6: built module-level _NAME_INDEX dict for O(1) Register.from_name;
  preserves historical "first match wins" on ambiguous suffixes and
  raises at import on canonical/legacy collisions.
- P7: documented retry behaviour on every ModbusCommError subclass
  (which are retried vs deterministic).
- P8: exported interpret_raw via __init__.py __all__; CLI imports from
  the public path now.

CI + packaging (C1, C2, T4):
- pyproject.toml: dev/test extras (pytest-cov, ruff, mypy);
  [tool.ruff] (E/F/W/I/UP/B/SIM rule set, py310 target, line-length 100,
  E501 ignored, B011 ignored in tests); [tool.mypy] gradual-typing
  config with check_untyped_defs + warn_unreachable + pymodbus override.
- .github/workflows/ci.yml: Python 3.10/3.11/3.12 matrix with pip cache,
  py_compile example.py, ruff lint, mypy type-check, and pytest with
  --cov term-missing.

CLI smoke tests (T1):
- tests/test_cli.py: 27 new tests covering _parse_raw_value,
  _parse_interpreted_value, _format_raw, _format_interpreted (incl. the
  B5 regression that --interpret no longer emits a misleading hex twin),
  _resolve_model_arg + --branch DeprecationWarning, and end-to-end
  main(argv) for list-models, list-registers, and the port/slave guard.

Ruff autofixes also normalised imports across the package and rewrote
typing.Iterable -> collections.abc.Iterable plus dropped quoted-string
forward references under `from __future__ import annotations`.

Verification: pytest 179 passed (152 + 27 new), 88% line coverage,
ruff clean, mypy clean, python -m py_compile example.py clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bonev-st bonev-st merged commit 3c0a3bd into main May 19, 2026
3 checks passed
@bonev-st bonev-st deleted the chore/code-review-followups branch May 19, 2026 10:31
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