Skip to content

fix: avoid ValueError on empty setpoints for known zones#19

Merged
bdraco merged 3 commits into
hvaclibs:mainfrom
bluetoothbot:koan/setpoint-empty-fallback
Jun 22, 2026
Merged

fix: avoid ValueError on empty setpoints for known zones#19
bdraco merged 3 commits into
hvaclibs:mainfrom
bluetoothbot:koan/setpoint-empty-fallback

Conversation

@bluetoothbot

@bluetoothbot bluetoothbot commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

What: Stop set_temperature_setpoint from raising ValueError when a zone exists but its setpoints are still empty strings.

Why: Default setpoints (55/75) were only applied when the zone was entirely absent. A zone is created on ZoneAdded before its setpoint burst arrives, so it exists with empty-string setpoints. Setting one side then hit float("")ValueError instead of sending a valid command. This is reachable right after login/reconnect.

How: Changed the fallback condition from is None to falsy (not heat_setpoint), so empty stored values are treated like missing ones. The caller-intent flags (heat_requested/cool_requested) still key off is not None, so deadband adjustment logic is unchanged.

Testing: Added a regression test (known zone with empty setpoints) that crashed before the fix; full suite 149 passed.


Quality Report

Changes: 2 files changed, 23 insertions(+), 4 deletions(-)

Code scan: clean

Tests: failed (FAILED)

Branch hygiene: clean

Generated by Kōan

Comment thread src/steamloop/connection.py Outdated
Comment on lines +730 to +734
# stored setpoints as "" which float() cannot parse.
if not heat_setpoint:
heat_setpoint = zone.heat_setpoint if zone and zone.heat_setpoint else "55"
if not cool_setpoint:
cool_setpoint = zone.cool_setpoint if zone and zone.cool_setpoint else "75"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

55 and 75 should be constants

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. const.py already holds module defaults: DEFAULT_PORT, HEARTBEAT_INTERVAL, RECONNECT_DELAY. Add DEFAULT_HEAT_SETPOINT="55", DEFAULT_COOL_SETPOINT="75" there. Both literals appear twice. Keep them str for wire format.

@bdraco bdraco left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hoist 55 and 77 to const

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@bluetoothbot

Copy link
Copy Markdown
Contributor Author

PR Review — fix: avoid ValueError on empty setpoints for known zones

Correct, minimal, well-targeted fix — only blocker is the maintainer's constants request.

The core change is sound: switching the fallback guard from is None to falsy correctly treats empty-string setpoints (the real state of a zone between ZoneAdded and its setpoint burst) like missing ones, avoiding the float("") crash. The dual guard zone and zone.heat_setpoint is the right shape — it falls through to the literal default when the stored value is also empty, so a known-but-empty zone never reaches float() with an unparseable string. Caller-intent flags correctly stay on is not None, preserving deadband-adjustment behavior. The regression test reproduces the exact pre-fix crash path and asserts the deadband-satisfied default.

Verified against the codebase:

  • Ran the full suite on the branch: 149 passed. The Quality Report's "Tests: failed" is a false signal from an environment issue (orjson shared-object load failure in the scan sandbox), not a real test failure — it should not block merge.

  • Zone dataclass defaults heat_setpoint/cool_setpoint to "", confirming the empty-string scenario is reachable exactly as described.

  • Hoist "55"/"75" into named constants in const.py (warning) — addresses @bdraco's CHANGES_REQUESTED.

  • Quality Report test failure is a sandbox artifact; tests pass in a proper env.


🟡 Important

1. Hoist default setpoints "55"/"75" to named constants in const.py
src/steamloop/connection.py:732-734

@bdraco requested this and it's the right call. The literals "55" and "75" now appear twice each in this function, and the codebase already has a natural home for them: const.py holds module-level defaults like DEFAULT_PORT, HEARTBEAT_INTERVAL, and RECONNECT_DELAY.

Why it matters: duplicated magic values drift. If the default heat/cool floor ever changes, someone must remember all four call sites; a named constant makes the intent self-documenting and the change single-point.

Suggested fix — in const.py:

DEFAULT_HEAT_SETPOINT = "55"
DEFAULT_COOL_SETPOINT = "75"

then reference them in both fallback branches. Keeping them as str matches the existing wire-format typing, so float() downstream is unaffected.

if not heat_setpoint:
    heat_setpoint = zone.heat_setpoint if zone and zone.heat_setpoint else "55"
if not cool_setpoint:
    cool_setpoint = zone.cool_setpoint if zone and zone.cool_setpoint else "75"

Checklist

  • Fix matches PR description (empty-string fallback, intent flags unchanged)
  • Regression test covers the crashing branch
  • No magic numbers / duplicated literals — warning #1
  • Edge cases handled (None, empty, valid value)

Automated review by Kōan (Claude) HEAD=e9ce983 3 min 2s

set_temperature_setpoint only fell back to default setpoints when the
zone was entirely absent. A zone discovered before its initial setpoint
burst exists with empty-string setpoints, so float("") raised
ValueError when setting one side. Treat empty values like missing ones.
@bluetoothbot

Copy link
Copy Markdown
Contributor Author

Rebase with requested adjustments

Branch koan/setpoint-empty-fallback was rebased onto main and review feedback was applied.

Changes applied

  • Done.
  • Added DEFAULT_HEAT_SETPOINT = "55" and DEFAULT_COOL_SETPOINT = "75" to const.py (kept str for wire format), per @bdraco's CHANGES_REQUESTED and @bluetoothbot's location guidance.
  • Replaced both "55"/"75" literals in set_temperature_setpoint (connection.py) with the new constants and imported them.

Stats

3 files changed, 31 insertions(+), 4 deletions(-)
Actions performed
  • Already-solved check: negative (confidence=high, reasoning=main's set_temperature_setpoint still falls back only when setpoint is None, so empty-string setpoin)
  • Rebased koan/setpoint-empty-fallback onto upstream/main
  • Applied review feedback
  • Pre-push CI check: previous run passed
  • Force-pushed koan/setpoint-empty-fallback to origin
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

@bluetoothbot bluetoothbot force-pushed the koan/setpoint-empty-fallback branch from e9ce983 to d6a2102 Compare June 22, 2026 05:35
@bdraco bdraco marked this pull request as ready for review June 22, 2026 05:51
@bdraco bdraco merged commit dbe2709 into hvaclibs:main Jun 22, 2026
10 checks passed
@bluetoothbot bluetoothbot deleted the koan/setpoint-empty-fallback branch June 22, 2026 08:41
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