[1.8] RC 1.8.0

[KartoffelToby]

Motivation:

This is the pre release branche

Summary by CodeRabbit

• New Features

  • SELECT-type calibration support for more valve types.
  • TRV "ignore_trv_states" option to prevent unwanted state adoption.
  • New min_update_interval_s advanced configuration option.

• Improvements

  • Dynamic algorithm-aware sensors that auto-create and clean up related entities.
  • Safer, more robust device handling, reduced redundant commands, and improved boost/safety valve behavior.
  • README/docs and UI text streamlined.

• Bug Fixes

  • Aggressive calibration now skips inappropriate tolerance adjustments; improved calibration value parsing.

• Tests

  • Expanded coverage for calibration modes, aggressive calibration, PID, valve resolution, boost mode, and TRV ignore-state behavior.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds SELECT-based calibration support, dispatcher-driven config-change signals, dynamic per-entry algorithm sensors with cleanup/unload hooks, translation_key-based entity discovery, refined valve/control helpers and boost handling, TRV ignore-state short-circuiting, logging hardening, tests, docs, and tooling configs.

Changes

Cohort / File(s)	Summary
Project config & docs /.gitignore, /.pylintrc, /.vscode/settings.json, /pyproject.toml, /README.md, docs/...	Normalize ignores; add pylint and pyright configs; remove VSCode settings; README/docs formatting/content tweaks.
Adapters custom_components/better_thermostat/adapters/__init__.py, custom_components/better_thermostat/adapters/delegate.py, custom_components/better_thermostat/adapters/generic.py	Add adapters package; remove stray pass; ensure default step; add SELECT calibration support (strip suffix, infer min/max, snap/select options vs number.set_value).
Core: climate & config flow custom_components/better_thermostat/climate.py, custom_components/better_thermostat/config_flow.py	Add SIGNAL_BT_CONFIG_CHANGED and _signal_config_change; cache device_info; detect calibration-algorithm changes in flows and dispatch config-change signals.
Dynamic entities & lifecycle custom_components/better_thermostat/sensor.py, custom_components/better_thermostat/number.py, custom_components/better_thermostat/switch.py	Per-entry dynamic algorithm sensor setup; trackers for active algorithms/presets/PID/switches; async_unload_entry hooks; creation and cleanup logic for MPC/PID/preset entities.
Events & TRV handling custom_components/better_thermostat/events/*, custom_components/better_thermostat/events/trv.py, custom_components/better_thermostat/events/*	Add events package; add None/attribute guards; adopt parameterized logging; short-circuit TRV processing when ignore_trv_states is set.
Model fixes & quirks custom_components/better_thermostat/model_fixes/*	Add package init; null-check entity-registry entries; tighten exception handling in TRVZB; safer capability access and logging refinements.
Helpers & control utilities custom_components/better_thermostat/utils/...	Add utils/calibration package; translation_key-first valve/calibration discovery; new rounding helpers; rework controlling.py (boost helpers, TaskManager, _get/_apply valve control, safety reset); remove solar_gain from MPC baseline calc.
Calibration logic & tests custom_components/better_thermostat/calibration.py, tests/test_aggressive_calibration.py	Skip tolerance-based post-adjustment for AGGRESIVE_CALIBRATION; add comprehensive aggressive-calibration tests and hysteresis scenarios.
Entity discovery & unit tests tests/test_helpers_valve.py, tests/unit/test_trv_ignore_states.py, tests/test_pid.py, many unit tests under tests/unit/*	Add translation_key-focused valve/calibration tests; TRV ignore_states tests; adjust PID test; add numerous unit tests for helpers and boost/control behavior.
Localization & strings custom_components/better_thermostat/strings.json, translations/en.json	Shorten/help text strings; add min_update_interval_s entry; simplify calibration/help wording.
Misc / simulators & temp files tmp/mpc_simulator.py, tmp/pid_simulator.py	Add module docstrings and minor formatting/import tweaks for simulators.

Sequence Diagram(s)

sequenceDiagram
    participant ConfigFlow as ConfigFlow/OptionsFlow
    participant Dispatcher as Dispatcher
    participant SensorMgr as sensor.py
    participant NumberMgr as number/switch managers

    ConfigFlow->>ConfigFlow: _check_calibration_changes(old, new)
    alt algorithms changed
        ConfigFlow->>Dispatcher: dispatcher_send(bt_config_changed_{entry_id})
        Dispatcher->>SensorMgr: bt_config_changed_{entry_id}
        SensorMgr->>SensorMgr: _handle_dynamic_entity_update(entry_id)
        SensorMgr->>NumberMgr: create/remove algorithm sensors, numbers, switches
        NumberMgr->>NumberMgr: _cleanup_stale_algorithm_entities(entry_id)
    else no change
        Note over ConfigFlow: no dispatch
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related issues

• Native Support for SELECT Entities as Calibration Offset for HomeMatic Thermostats #1888 — Implements SELECT-based temperature_offset handling in adapters/generic and related helpers.
• [1.8] TRVZB external_temperature_input number entity not found #1836 — Changes to translation_key-based discovery and calibration lookup address entity-detection concerns described there.
• [1.8] It is not clear how to setup Direct Valve Control #1837 — Reworks valve/control helpers and boost handling affecting direct-valve-control behavior.

Possibly related PRs

• Fix issue with translated entities use translation_key #1896 — Similar translation_key-based entity classification and tests for valve/calibration discovery.
• fix: Add boost mode support for available TRVs #1873 — Overlaps on utils/controlling.py changes (boost helpers, TaskManager, valve control flow).
• feature/dynamic entity cleanup #1897 — Related dynamic entity tracking, dispatcher signaling, and SELECT calibration support across sensors/number/switch/config_flow.

Poem

🐰 I hop where offsets hide in strings,

I trim the 'k' and wake the things,
I send a signal, sensors sprout,
Old numbers leave, new checks come out,
A cozy home with warmer wings.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description is largely incomplete. It only contains a single-line motivation ('This is the pre release branche') and is missing all other required template sections including Changes, Related issue, and Checklist.	Complete the description by filling in the Changes section with a summary of modifications, selecting a Related issue option, and checking appropriate Checklist items. Include test hardware and device mapping information if applicable.
Title check	❓ Inconclusive	The title '[1.8] RC 1.8.0' is vague and generic, using non-descriptive release notation that doesn't convey the main changes in the changeset.	Provide a more descriptive title that highlights key improvements, such as 'Add boost mode support and improve robustness for 1.8.0 release' or 'Refactor calibration handling and valve control for 1.8.0'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	Docstring coverage is 94.04% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch develop

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)

custom_components/better_thermostat/utils/controlling.py (1)

386-448: ⚠️ Potential issue | 🟠 Major

Remove duplicate cooler control from control_trv().

Lines 386-448 duplicate cooler control logic that already executes at line 94 via the centralized control_cooler() call. Since control_cooler() is invoked before any control_trv() tasks and includes optimizations (state checking, conditional command sending), this block should be removed to avoid sending duplicate commands to the cooler.

tmp/mpc_simulator.py (1)

6-20: ⚠️ Potential issue | 🟡 Minor

sys.path.append is placed after the imports it's meant to enable.

Line 20 appends the workspace root to sys.path, but the custom_components import on lines 12–17 executes first. If this script is ever run from a directory where the module isn't already discoverable, the import will fail before the path fix takes effect.

Proposed fix

+import os
+import sys
+
+# Add workspace root to sys.path (must precede local imports)
+sys.path.append(os.getcwd())
+
 from dataclasses import dataclass
 import logging
-import os
-import sys
 from unittest.mock import patch
 
 from custom_components.better_thermostat.utils.calibration.mpc import (
     _MPC_STATES,
     MpcInput,
     MpcParams,
     compute_mpc,
 )
-
-# Add workspace root to sys.path
-sys.path.append(os.getcwd())

custom_components/better_thermostat/utils/retry.py (2)

44-48: ⚠️ Potential issue | 🟡 Minor

Off-by-one in entity_id extraction from positional args.

The comment says "self and entity_id are first two args" (indices 0 and 1), but the guard requires len(args) > 2 (i.e., at least 3 args). This means entity_id won't be extracted from calls with exactly 2 positional args (self, entity_id). Should be len(args) > 1.

Proposed fix

             entity_id = kwargs.get("entity_id", None)
-            if (
-                entity_id is None and len(args) > 2
-            ):  # Assuming self and entity_id are first two args
+            if (
+                entity_id is None and len(args) > 1
+            ):  # Assuming self is args[0] and entity_id is args[1]
                 entity_id = args[1]

---

89-92: ⚠️ Potential issue | 🟡 Minor

_LOGGER.exception outside an active except block won't capture a traceback.

At line 91, execution has left the except scope, so sys.exc_info() returns (None, None, None). Using _LOGGER.exception here logs the message but appends a useless NoneType: None instead of the actual traceback. Consider switching to _LOGGER.error or passing exc_info=last_exception.

Proposed fix

-            log_message = f"{log_prefix}{func.__name__} failed after {retries + 1} attempts: {last_exception}{entity_suffix}"
-            _LOGGER.exception(log_message)
+            log_message = f"{log_prefix}{func.__name__} failed after {retries + 1} attempts: {last_exception}{entity_suffix}"
+            _LOGGER.error(log_message, exc_info=last_exception)

custom_components/better_thermostat/events/trv.py (1)

301-309: ⚠️ Potential issue | 🟡 Minor

Pre-existing: both cooltemp guards always resolve to the same assignment.

The two conditions on lines 302 and 306 (<= and >=) together cover every possible relationship between bt_target_temp and bt_target_cooltemp, so bt_target_cooltemp is unconditionally set to bt_target_temp - bt_target_temp_step. If the intent was to only adjust when the heat/cool targets cross, the second branch likely needs a different assignment (e.g., bt_target_temp + step).

custom_components/better_thermostat/translations/en.json (1)

106-106: ⚠️ Potential issue | 🟡 Minor

Typo: "Aggresive" → "Aggressive" in the options calibration_mode description.

Also, the MPC description here says "hydraulic balance algorithm" while the config step (line 41) says "predictive controller" — these should be consistent. The strings.json file (line 39) uses "Aggressive" (correct) and a different MPC phrasing as well.

🤖 Fix all issues with AI agents

In `@custom_components/better_thermostat/events/cooler.py`:
- Around line 29-36: The check builds a tuple including new_state.attributes
before testing for None, which can raise when new_state is None; change the
guard in the Cooler event handler to first check new_state and old_state (e.g.,
if new_state is None or old_state is None) and only then check
new_state.attributes (e.g., if new_state.attributes is None) before logging and
returning; update the condition around the _LOGGER.debug/return so you never
access new_state.attributes unless new_state is not None.

In `@custom_components/better_thermostat/model_fixes/BTH-RM.py`:
- Around line 49-52: entity_reg.async_get(entity_id) can return None so change
the handling around entry in the function that reads entry.capabilities (lookup
using entity_reg and async_get) to guard against None before accessing
attributes: check if entry is None and if so set hvac_modes = [] (or an
appropriate default) or early-return, otherwise read
entry.capabilities.get("hvac_modes", []); apply the same None-guard pattern to
the analogous code in BTH-RM230Z.py to avoid AttributeError when the entity is
not registered.

In `@custom_components/better_thermostat/number.py`:
- Around line 64-71: The code directly compares
advanced.get(CONF_CALIBRATION_MODE) to CalibrationMode.PID_CALIBRATION which
fails when the stored value is a string; mirror the normalization used in
switch.py by first normalizing/mapping the raw value to the CalibrationMode enum
(e.g. read calib = advanced.get(CONF_CALIBRATION_MODE), if calib is a str
attempt to map it to CalibrationMode via CalibrationMode(calib) or
CalibrationMode[calib.upper()] in a try/except) and then compare that enum to
CalibrationMode.PID_CALIBRATION before creating BetterThermostatPIDNumber
instances (refer to CONF_CALIBRATION_MODE, CalibrationMode,
BetterThermostatPIDNumber and trv_conf to locate the change).

In `@custom_components/better_thermostat/sensor.py`:
- Around line 160-182: The code is recreating sensors for all active algorithms
even if already registered, causing duplicate unique_id collisions; modify the
logic so that only algorithms that are newly added are passed to or created by
_setup_algorithm_sensors (or add a guard inside _setup_algorithm_sensors) by
using algorithms_added (and filtering out any algorithm keys present in
_ACTIVE_ALGORITHM_ENTITIES) before creating sensor objects for bt_climate;
ensure _setup_algorithm_sensors either accepts a list/set of
algorithms_to_create or checks _ACTIVE_ALGORITHM_ENTITIES and skips creation for
algorithms already tracked so no duplicate entities with the same unique_id are
instantiated.

In `@custom_components/better_thermostat/utils/controlling.py`:
- Around line 177-192: The current check uses "if current_temp != desired_temp"
which treats current_temp=None as always different; change it to explicitly
handle unknown current_temp by sending the command when current_temp is None or
different (e.g., "if current_temp is None or current_temp != desired_temp"), and
add a debug log via _LOGGER.debug when current_temp is None to state that
temperature is unknown for self.cooler_entity_id so the set_temperature command
(the async_call to "climate.set_temperature" using self.cooler_entity_id and
desired_temp with context self.context) is being sent.
- Around line 167-175: The conditional that sets desired_mode uses
self.cur_temp, self.bt_target_cooltemp, self.tolerance (and compares to
self.bt_target_temp) without None-safety; add an early guard before that block
to check if any of self.cur_temp, self.bt_target_cooltemp, self.tolerance, or
self.bt_target_temp is None and if so return or set desired_mode = HVACMode.OFF
(i.e. bail out early) to avoid TypeError; update the method containing this
conditional to perform this None-check before evaluating the comparison.

In `@docs/Configuration/configuration.md`:
- Line 82: The docs callout uses a single device-specific parameter name
TEMPERATURE_OFFSET:MASTER@HM-CC-RT-DN which may not apply to all HomeMatic
models; update the text to clarify that parameter paths can vary by model (e.g.,
HmIP-eTRV, HM-CC-RT-DN-BoM) and instruct users to check their device-specific
parameter name in the device manual or interface, and give
TEMPERATURE_OFFSET:MASTER@HM-CC-RT-DN as an example rather than the only valid
key.

In `@pyproject.toml`:
- Around line 64-83: Ruff's target-version setting ("py313") and Pyright's
pythonVersion ("3.12") are inconsistent; update one so both match (e.g., set
Ruff's target-version to "py312" or Pyright's pythonVersion to "3.13") to ensure
consistent linting/type-checking; edit the Ruff config entry that defines
target-version and/or the [tool.pyright] pythonVersion entry so they use the
same Python version string.

🧹 Nitpick comments (12)

tmp/pid_simulator.py (1)

17-18: sys.path.append is executed after the import it's meant to support.

The custom_components import on lines 11–15 runs before sys.path.append(os.getcwd()) on line 18. This works only if the working directory is already on sys.path (e.g., via PYTHONPATH or IDE config). If this script is ever invoked standalone from a different directory, the import will fail. Consider moving the path manipulation above the custom_components import.

Proposed reorder

+import os
+import sys
+
+# Add workspace root to path so custom_components is importable
+sys.path.append(os.getcwd())
+
 """PID controller simulator for testing the PID calibration logic.
 ...
 """
 
 import logging
-import os
-import sys
 from unittest.mock import patch
 
 from custom_components.better_thermostat.utils.calibration.pid import (
     PIDParams,
     compute_pid,
     reset_pid_state,
 )
-
-# Add workspace root to path
-sys.path.append(os.getcwd())

tmp/mpc_simulator.py (1)

1-4: Consider whether tmp/mpc_simulator.py should be committed to the repository.

This file lives under a tmp/ directory, which conventionally holds throwaway/scratch files. If it's meant as a development utility, consider moving it to a proper location (e.g., scripts/ or tools/) or adding tmp/ to .gitignore to avoid cluttering the repo.

.pylintrc (2)

27-29: max-line-length inconsistent with Ruff.

Ruff enforces line-length = 88 (pyproject.toml line 3) while pylint allows 150. If Ruff is the primary linter/formatter, consider aligning pylint's limit or disabling its line-length check entirely (disable=C0301) to avoid confusion.

---

18-25: Disabling W0718 while also listing overgeneral exceptions is redundant.

W0718 (broad-exception-caught) is disabled in [MESSAGES CONTROL], but [EXCEPTIONS] still defines overgeneral-exceptions. The disable takes precedence, so the [EXCEPTIONS] block is effectively dead config for this warning. This is fine functionally but may confuse future maintainers.

custom_components/better_thermostat/utils/calibration/mpc.py (1)

1373-1401: Remove unused solar_gain_est field from state, export list, and initialization.

The solar gain logic is fully disabled (lines 1375–1400 all commented out), yet solar_gain_est continues to be initialized at line 914–915 and exported/imported via _STATE_EXPORT_FIELDS (line 161). This dead field wastes cycles on initialization and persists across state save/load cycles without ever being consumed. Remove the field definition (line 121), the export entry (line 161), and the initialization block (lines 914–915).

custom_components/better_thermostat/events/trv.py (2)

459-460: Bare _LOGGER.error(e) loses the traceback.

In convert_outbound_states, the catch-all logs only the exception message. Use _LOGGER.error("...: %s", e, exc_info=True) or _LOGGER.exception(...) to preserve the stack trace for debugging.

Proposed fix

     except Exception as e:
-        _LOGGER.error(e)
+        _LOGGER.exception(
+            "better_thermostat %s: convert_outbound_states failed for %s: %s",
+            self.device_name,
+            entity_id,
+            e,
+        )
         return None

---

201-202: Silent except Exception: pass swallows errors in hvac_action / valve_position handling.

If an unexpected error occurs while caching hvac_action or valve_position, it will be silently ignored, making debugging difficult. Consider at minimum logging at debug level.

Proposed fix

-    except Exception:
-        pass
+    except Exception:
+        _LOGGER.debug(
+            "better_thermostat %s: failed to update hvac_action/valve_position for %s",
+            self.device_name,
+            entity_id,
+            exc_info=True,
+        )

.gitignore (1)

78-82: Duplicate entries: *.swp and *~ already appear in the IDEs section (lines 42, 44).

Lines 81–82 duplicate rules from lines 42 and 44. While git handles duplicates gracefully, removing them keeps the file tidy.

Proposed fix

 # Temporary files
 *.tmp
 *.bak
-*.swp
-*~

custom_components/better_thermostat/model_fixes/TRVZB.py (1)

25-31: Tightened exception handling in _cancel_pending_valve_bump — minor note.

task.cancel() itself doesn't raise CancelledError (it only schedules cancellation on the task). The CancelledError catch here is harmless but effectively dead code. RuntimeError (e.g., closed event loop) is the realistic exception to guard against.

custom_components/better_thermostat/utils/helpers.py (1)

697-729: find_local_calibration_entity doesn't break after finding the first translation_key match — last match wins silently.

In the first pass (lines 699-710), if multiple entities on the same device have a matching translation_key, the loop keeps overwriting calibration_entity without breaking. The same applies in the string-match fallback (lines 714-729). This means if two calibration-like entities exist (e.g., one for local temp calibration and one for external offset), the last one iterated wins non-deterministically.

Consider adding a break after finding the first match in each pass, or implementing a scoring/priority mechanism similar to find_valve_entity.

♻️ Proposed fix — break after first match in each pass

             if tk and tk in _CALIBRATION_TRANSLATION_KEYS:
                 _LOGGER.debug(
                     "better thermostat: Found local calibration entity %s for %s (translation_key=%s)",
                     entity.entity_id,
                     entity_id,
                     tk,
                 )
                 calibration_entity = entity.entity_id
+                break

     # Second pass: fallback to string matching on unique_id / entity_id / original_name
     if calibration_entity is None:
         for entity in entity_entries:
             if entity.device_id != reg_entity.device_id:
                 continue
             descriptor = f"{entity.unique_id} {entity.entity_id} {getattr(entity, 'original_name', '') or ''}".lower()
             if (
                 "temperature_calibration" in descriptor
                 or "temperature_offset" in descriptor
                 or "temperatur_offset" in descriptor
                 or "local_temperature" in descriptor
             ):
                 _LOGGER.debug(
                     "better thermostat: Found local calibration entity %s for %s (string match)",
                     entity.entity_id,
                     entity_id,
                 )
                 calibration_entity = entity.entity_id
+                break

tests/test_helpers_valve.py (1)

453-495: Test may not catch the "last match wins" bug in production code.

This test (line 491) passes [ent_str, ent_tk] — since ent_str has translation_key=None, it won't match in the first pass, so ent_tk (with translation_key="temperature_calibration") will be the only first-pass match and calibration_entity will be set correctly. However, if two entities both had valid translation keys, the test wouldn't catch the non-deterministic "last wins" behavior noted in the find_local_calibration_entity review. Consider adding a test case with two entities that both have valid translation keys to verify which one is selected.

tests/unit/test_trv_ignore_states.py (1)

64-88: Tests duplicate production condition logic instead of exercising the actual function — reduces regression coverage.

All four tests inline-reconstruct the temperature adoption condition instead of calling the real trigger_trv_change() function from events/trv.py. The condition at lines 73–86 (and repeated in subsequent tests) mirrors the production check at events/trv.py:271–282 exactly. If the production condition changes—e.g., a clause is reordered, a new check is added, or a field is renamed—these tests will still pass while silently diverging from the actual behavior.

Consider extracting the condition into a helper method in the BetterThermostat class and having tests call it directly (passing the necessary temperature and TRV state parameters). This would provide genuine regression coverage.

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

custom_components/better_thermostat/calibration.py (2)

92-120: ⚠️ Potential issue | 🟠 Major

Copy-paste bug: cloud coverage check duplicated, UV index block shadowed.

Lines 102–110 are an exact duplicate of lines 92–100 (both check cloud_coverage with identical logic). Since the first block returns on a match, the duplicate on lines 102–110 is unreachable dead code. The real UV index check on lines 112–120 is fine but the duplicate block above it should be removed.

🐛 Proposed fix: remove duplicate block

             except (ValueError, TypeError):
                 pass
 
-    # 2. UV Index (0-10+) -> Higher is better
-    for source in sources:
-        cc = _get_val(source, "cloud_coverage")
-        if cc is not None:
-            try:
-                # 0% clouds = 1.0 intensity, 100% clouds = 0.0 intensity
-                return max(0.0, min(1.0, (100.0 - float(cc)) / 100.0))
-            except (ValueError, TypeError):
-                pass
-
     # 2. UV Index (0-10+) -> Higher is better
     for source in sources:
         uv = _get_val(source, "uv_index")

---

575-578: ⚠️ Potential issue | 🟠 Major

Aggressive calibration condition mismatch with commit message — code is intentional but documentation is misleading.

The commit message describes two changes, but they contradict each other and the code. The first part claims: "Change the condition for applying aggressive calibration from checking hvac_action == HEATING to checking cur_temp < bt_target_temp." The second part states: "Skip the tolerance delay for AGGRESIVE_CALIBRATION mode while keeping the -2.5 offset behavior unchanged (only applies when HEATING)."

The actual code on line 576 still checks self.attr_hvac_action == HVACAction.HEATING, not cur_temp < bt_target_temp. The promised condition change was never applied. The real fix (evident from the code diff) was skipping the tolerance delay elsewhere, not changing this condition.

The code is intentional—the -2.5 offset applies only when actively HEATING—but the commit message is inaccurate about what was changed. Update the commit message to reflect that the fix skipped the tolerance delay, not the heating condition.

🤖 Fix all issues with AI agents

In `@tests/test_aggressive_calibration.py`:
- Around line 37-51: The test incorrectly computes _skip_post_adjustments
compared to production: align the test with calculate_calibration_local() by
including CalibrationMode.DEFAULT in the tuple used to set
_skip_post_adjustments (so DEFAULT skips post-adjustments), or alternatively
update the failing assertions in tests test_default_idle_has_delay,
test_setpoint_default_has_tolerance_delay, and
test_scenario_temperature_drops_below_tolerance to expect no tolerance delay for
DEFAULT; locate the test code using the variable _skip_post_adjustments and the
test functions named above and make the change so the test behavior matches the
production logic in calculate_calibration_local().
- Around line 23-340: Replace the duplicated inline calibration logic in these
tests with integration-style calls to the real production functions: call
calculate_calibration_local() and calculate_calibration_setpoint() instead of
reimplementing branches; construct a minimal/mock "self" object (or fixture)
with the required attributes (cur_temp, bt_target_temp, real_trvs, tolerance,
attr_hvac_action, base_setpoint, etc.) to pass into those functions, invoke them
for each calibration_mode (CalibrationMode.DEFAULT, AGGRESIVE_CALIBRATION,
MPC_CALIBRATION, etc.), and assert on the returned calibration values (or
mutated state) matching the expected outcomes; update tests like
test_default_mode_has_tolerance_delay_when_idle,
test_aggressive_mode_skips_tolerance_delay_when_idle,
test_setpoint_default_has_tolerance_delay, etc., to call
calculate_calibration_local() / calculate_calibration_setpoint() and assert
results rather than duplicating the arithmetic.

🧹 Nitpick comments (1)

tests/test_aggressive_calibration.py (1)

305-340: Hysteresis scenario test is a good illustration but uses abs() comparisons unnecessarily.

Since all values are computed from exact floating-point arithmetic (no division or irrational operations), exact equality checks would work fine here (as used in all other tests). Using abs(... - ...) < 0.001 is fine but inconsistent with the rest of the file. Minor nit — not blocking.

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

custom_components/better_thermostat/utils/controlling.py (1)

416-477: ⚠️ Potential issue | 🟠 Major

Stale cooler control path in the TRV-unavailable branch lacks new guards and duplicates work.

control_cooler is already called at line 94 (before any control_trv call). Yet when a TRV is unavailable, this block unconditionally issues set_temperature and set_hvac_mode to the cooler again, without any of the new safeguards:

1. TypeError risk – self.cur_temp, self.bt_target_cooltemp, and self.tolerance are used at lines 418/440 without None checks (the new control_cooler guards against this at line 167).
2. Redundant / conflicting commands – The cooler has already been set to desired_mode by control_cooler; this block may flip it back to a different mode because it uses _new_hvac_mode (which factors in window-open state but not the same decision logic).
3. No state-diffing – Commands are always sent even if the cooler is already in the desired state.

Consider removing this block entirely (consistent with the available-TRV path at line 714, which has already been cleaned up) and relying on the centralized control_cooler.

Proposed fix – remove the duplicate cooler section

         _new_hvac_mode = handle_window_open(self, _remapped_states)
 
-        # New cooler section
-        if self.cooler_entity_id is not None:
-            if (
-                self.cur_temp >= self.bt_target_cooltemp - self.tolerance
-                and _new_hvac_mode is not HVACMode.OFF
-                and self.cur_temp > self.bt_target_temp
-            ):
-                await self.hass.services.async_call(
-                    "climate",
-                    "set_temperature",
-                    {
-                        "entity_id": self.cooler_entity_id,
-                        "temperature": self.bt_target_cooltemp,
-                    },
-                    blocking=True,
-                    context=self.context,
-                )
-                await self.hass.services.async_call(
-                    "climate",
-                    "set_hvac_mode",
-                    {"entity_id": self.cooler_entity_id, "hvac_mode": HVACMode.COOL},
-                    blocking=True,
-                    context=self.context,
-                )
-            elif (
-                self.cur_temp <= (self.bt_target_cooltemp - self.tolerance)
-                and _new_hvac_mode is not HVACMode.OFF
-            ):
-                await self.hass.services.async_call(
-                    "climate",
-                    "set_temperature",
-                    {
-                        "entity_id": self.cooler_entity_id,
-                        "temperature": self.bt_target_cooltemp,
-                    },
-                    blocking=True,
-                    context=self.context,
-                )
-                await self.hass.services.async_call(
-                    "climate",
-                    "set_hvac_mode",
-                    {"entity_id": self.cooler_entity_id, "hvac_mode": HVACMode.OFF},
-                    blocking=True,
-                    context=self.context,
-                )
-            else:
-                await self.hass.services.async_call(
-                    "climate",
-                    "set_temperature",
-                    {
-                        "entity_id": self.cooler_entity_id,
-                        "temperature": self.bt_target_cooltemp,
-                    },
-                    blocking=True,
-                    context=self.context,
-                )
-                await self.hass.services.async_call(
-                    "climate",
-                    "set_hvac_mode",
-                    {"entity_id": self.cooler_entity_id, "hvac_mode": HVACMode.OFF},
-                    blocking=True,
-                    context=self.context,
-                )
-
         # if we don't need to heat, we force HVACMode to be off

🤖 Fix all issues with AI agents

In `@custom_components/better_thermostat/events/cooler.py`:
- Around line 38-52: The code accesses new_state.attributes before verifying
types; move the isinstance checks so that both new_state and old_state are
confirmed to be instances of State (using the existing isinstance(new_state,
State) and isinstance(old_state, State) guard) before any access to
new_state.attributes, and only then check if new_state.attributes is None and
log/return as currently done (preserve the existing debug messages referencing
self.device_name and entity_id).

In `@custom_components/better_thermostat/sensor.py`:
- Around line 404-411: The current cleanup logic (looping tracked_pid_numbers
and splitting on "_pid_") is fragile because it reverse-parses unique_id
strings; change the tracking structure (_ACTIVE_PID_NUMBERS and related PID
tracking maps) from a list of unique_id strings to a map/dict keyed by unique_id
with stored metadata { "trv": trv_entity_id, "param": parameter } and update all
places that create PID entities to populate that metadata; then modify cleanup
routines (the loop using tracked_pid_numbers, _cleanup_pid_switch_entities, and
_cleanup_preset_number_entities) to read the stored "trv" and "param" values
directly (e.g., replace the split/replace logic that uses bt_climate.unique_id
and parts = pid_unique_id.split("_pid_")) so you no longer reconstruct
trv_entity_id from the unique_id string.

In `@docs/Configuration/configuration.md`:
- Line 11: Replace the typo "Goto" with "Go to" in the markdown heading text
that currently reads "**Goto: `Settings` -> `Devices & Services` ->
`Integrations` -> `+ Add Integration` -> `Better Thermostat`**" so it becomes
"**Go to: `Settings` -> `Devices & Services` -> `Integrations` -> `+ Add
Integration` -> `Better Thermostat`**"; update the string in configuration.md
accordingly to preserve existing formatting and punctuation.

🧹 Nitpick comments (8)

custom_components/better_thermostat/model_fixes/BTH-RM.py (1)

44-47: Good: parameterized logging replaces f-strings.

Note that the sibling file BTH-RM230Z.py (lines 45 and 71) still uses f-string logging for the equivalent messages. Consider aligning both files for consistency.

Also applies to: 72-77

custom_components/better_thermostat/model_fixes/BTH-RM230Z.py (1)

44-45: Inconsistent logging style: f-strings here vs. parameterized logging in BTH-RM.py.

BTH-RM.py was updated to use %s placeholders (lines 45-47, 73-76), but these two log calls still use f-strings. Parameterized logging defers string interpolation until the message is actually emitted, which is the recommended practice.

Proposed fix

         _LOGGER.debug(
-            f"better_thermostat {self.device_name}: TRV {entity_id} device quirk bth-rm230z for set_temperature active"
+            "better_thermostat %s: TRV %s device quirk bth-rm230z for set_temperature active",
+            self.device_name,
+            entity_id,
         )

         _LOGGER.debug(
-            f"better_thermostat {self.device_name}: TRV {entity_id} device quirk bth-rm230z found hvac_modes {hvac_modes}"
+            "better_thermostat %s: TRV %s device quirk bth-rm230z found hvac_modes %s",
+            self.device_name,
+            entity_id,
+            hvac_modes,
         )

Also applies to: 70-72

custom_components/better_thermostat/utils/controlling.py (1)

320-405: Duplicated valve-control logic across TRV-unavailable and TRV-available paths.

The CalibrationMode-based valve-percent selection (lines 320-405 and 630-706) is nearly identical copy-paste. Any future calibration-mode addition or bug fix must be applied in both places. Consider extracting a helper like _resolve_valve_balance(self, heater_entity_id, calibration_mode, calibration_type).

custom_components/better_thermostat/number.py (2)

96-104: Duplicate cleanup of tracking dicts across number.py and sensor.py.

Both number.py (lines 101-102) and sensor.py (lines 566-567) pop the same _ACTIVE_PRESET_NUMBERS[entry_id] and _ACTIVE_PID_NUMBERS[entry_id]. The double-pop is safe but redundant — whichever platform unloads second operates on already-removed keys.

This is a consequence of the shared mutable state noted above. If ownership is moved to a shared module, cleanup should be consolidated in one place (e.g., __init__.py's async_unload_entry).

---

13-13: Cross-module shared mutable state creates tight coupling.

number.py imports and mutates _ACTIVE_PRESET_NUMBERS and _ACTIVE_PID_NUMBERS from sensor.py. While functionally safe (both modules' async_unload_entry methods safely call .pop with default), the ownership of these tracking structures is unclear and creates cross-module dependency. Consider moving these dicts to utils/const.py to make the dependency direction explicit and improve code clarity.

custom_components/better_thermostat/sensor.py (3)

28-36: German comments mixed with English codebase.

Several comments throughout this file are in German (e.g., "Globale Tracking-Variablen für aktive algorithmus-spezifische Entitäten", "Konvertiere String zu Enum falls nötig", "Prüfe auf Änderungen bei den Algorithmen", etc.). For consistency with the rest of the codebase (which uses English), consider translating these to English.

---

134-158: _ENTITY_CLEANUP_CALLBACKS is stored but never invoked — dead state.

Line 151 stores the _on_config_change callback in _ENTITY_CLEANUP_CALLBACKS, and line 565 pops it during unload, but nothing in the codebase ever reads or invokes these stored callbacks. The actual cleanup is handled by _DISPATCHER_UNSUBSCRIBES. Consider removing _ENTITY_CLEANUP_CALLBACKS to avoid confusion.

Proposed cleanup

-# Store callback für späteren Cleanup
-_ENTITY_CLEANUP_CALLBACKS[entry.entry_id] = _on_config_change
-
 # Listen to configuration change signals

And in async_unload_entry:

 _ACTIVE_ALGORITHM_ENTITIES.pop(entry_id, None)
-_ENTITY_CLEANUP_CALLBACKS.pop(entry_id, None)
 _ACTIVE_PRESET_NUMBERS.pop(entry_id, None)

And remove the module-level dict:

 _ACTIVE_ALGORITHM_ENTITIES = {}
-_ENTITY_CLEANUP_CALLBACKS = {}
 _DISPATCHER_UNSUBSCRIBES = {}

---

269-293: CalibrationMode string-to-enum normalization is repeated in 5+ places.

The same isinstance(calibration_mode, str) → CalibrationMode(calibration_mode) try/except pattern appears in _get_active_algorithms (line 280), _cleanup_pid_number_entities (line 392), _cleanup_pid_switch_entities (line 466), number.py (line 68), and config_flow.py. Extract a small helper to DRY this up.

Suggested helper

In utils/const.py alongside the CalibrationMode enum:

def normalize_calibration_mode(value) -> CalibrationMode | None:
    """Normalize a raw calibration mode value to a CalibrationMode enum, or None."""
    if isinstance(value, CalibrationMode):
        return value
    if isinstance(value, str):
        try:
            return CalibrationMode(value)
        except ValueError:
            return None
    return None

[KartoffelToby]

-> This state is in Beta 13