Skip to content

Tighten bypass target read contract - #123

Merged
ronaldvdmeer merged 1 commit into
mainfrom
fix/bypass-target-contract
Jul 19, 2026
Merged

Tighten bypass target read contract#123
ronaldvdmeer merged 1 commit into
mainfrom
fix/bypass-target-contract

Conversation

@ronaldvdmeer

@ronaldvdmeer ronaldvdmeer commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Summary

This PR tightens the contract of async_get_bypass_supply_temperature_target().

A parameter-specific GET /config?...&parameter=TempSupTgtZoneX read now either:

  • returns a typed BypassSupplyTemperatureTarget
  • raises DucoUnsupportedCapabilityError for unsupported zones
  • raises DucoError if the requested field is missing from an otherwise successful response

Why

Live validation against 192.168.3.179 showed a clear distinction in API behavior:

  • supported zones return the requested target
  • unsupported zones return 400 {"Code":3,"Result":"FAILED"}

Because this helper already wraps the parameter-specific endpoint, silently returning None for a missing requested field is less precise than surfacing that as an error. The broader config surface can still model omission separately.

Included

  • Client contract update
  • Unit tests updated
  • Generated API reference updated
  • README and config docs updated

Validation

  • PYTHONPATH=$PWD/src:$PWD .venv/bin/pytest
  • .venv/bin/ruff check src tests
  • .venv/bin/ruff format --check src tests
  • PYTHONPATH=$PWD/src .venv/bin/mypy src
  • .venv/bin/bandit -r src -ll

pip-audit only flagged the local .venv tool package pip 26.1.1 with an available fix in 26.1.2; no project dependency issue.

Copilot AI review requested due to automatic review settings July 19, 2026 08:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request tightens the contract of the bypass supply temperature target convenience reader so that successful parameter-specific /config reads always produce a typed BypassSupplyTemperatureTarget, and missing target fields now raise an explicit DucoError instead of returning None. This aligns the helper’s behavior with observed device responses (supported zones return the field; unsupported zones return a typed “unsupported capability” error).

Changes:

  • Updated DucoClient.async_get_bypass_supply_temperature_target() to raise DucoError when the requested target field is absent from an otherwise valid /config response, and to return a non-optional typed model.
  • Updated unit tests and API-reference generation tests to reflect the new error/return contract.
  • Updated generated docs and user-facing documentation (README + config docs + API reference + changelog) to match the tightened behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tools/api_reference.py Updates generated API reference metadata notes to document the new “raise on missing field” contract.
tests/test_client.py Adjusts unit test expectations from None to DucoError for missing bypass target fields.
tests/test_api_reference.py Updates API reference content assertion to match new documentation text.
src/duco_connectivity/client.py Changes helper return type to non-optional and raises DucoError if the target field is missing.
README.md Updates user-facing documentation to describe the new behavior (typed result or explicit error).
docs/config.md Updates behavior bullets and example code to reflect non-optional return and error on missing field.
docs/api-reference.md Updates generated API reference signature and notes for the tightened contract.
CHANGELOG.md Records the breaking/behavioral change under Unreleased “Changed”.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ronaldvdmeer
ronaldvdmeer merged commit 6899d23 into main Jul 19, 2026
6 checks passed
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.

2 participants