Fix invalid climate min/max range and non-functional heat setpoint on default-mapping devices - #124
Open
ChrisReini wants to merge 3 commits into
Conversation
For devices without a dedicated product ID mapping file (falling back to mapping.default.json), the min/max temperature registers can be reported the wrong way round for a given HVAC mode (minimum > maximum), which produces an invalid range for the climate entity (e.g. min_temp=35.0, max_temp=27.0 for product ID 1548963836789501952). Instead of relying on a per-product mapping file, add a validation in AquaTempAPI: minimum and maximum are now fetched together and, if minimum > maximum, automatically swapped, with a warning logged. This protects any device regardless of whether its product ID is recognized. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
On devices without a dedicated product ID mapping file (falling back to mapping.default.json), the heat-mode target register (e.g. R02) can be stale/unused on the physical device, so changing the setpoint from Home Assistant never reaches the unit even though the entity itself updates. On the reported device, the real live setpoint is exposed through the cool-mode target register (e.g. R01) regardless of the active HVAC mode. Add AquaTempConfigManager.is_default_mapping() and a new _get_effective_target_temperature_pc() in AquaTempAPI, wired into _get_target_temperature_protocol_code() (used by both get_device_target_temperature and set_temperature, so reading and writing stay consistent). It falls back from the heat target register to the cool target register only when: HVAC mode is HEAT, the device uses the generic default mapping, and the heat register's own value is missing or outside the device's validated min/max range for heat mode. Cool mode is never touched by this change, and devices with a dedicated mapping file are unaffected. This is deliberately conservative since the reporting user's hardware is heat-only and cool mode could not be tested. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ping devices Real-device testing showed that the heat-mode target register (e.g. R02) on devices using the generic default mapping isn't just occasionally stale: the device accepts and stores writes to it, but they have no effect on the physical unit at all. A plausibility/range check on the register's value can't catch this, since the device happily stores an in-range value there that still does nothing. Replace the previous conditional (missing-or-out-of-range) fallback in _get_effective_target_temperature_pc() with an unconditional one: for HVAC mode HEAT on devices using the default mapping, always use the cool-mode target register (e.g. R01) instead, which is the one that actually drives the device and matches what the vendor app shows. Cool mode and devices with a dedicated mapping file remain completely unaffected, same as before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
This PR addresses two related bugs affecting devices that fall back to the generic
mapping.default.jsonbecause they have no dedicatedmapping.<product_id>.jsonfile. It was developed and tested against product ID1548963836789501952(a Planet Pool-type heat pump), but the underlying issue is not specific to that one unit — see #90 ("Bad mapping of parameters for PLANET POOL TEP0001"), which reports a very similar mapping discrepancy between the vendor app and this integration on a different, also-unlisted Planet Pool device. This suggests the vendor's register layout is not consistent enough across models formapping.default.jsonto safely assume any single fixed layout, and that a defensive/validating approach (rather than trying to hand-craft a "correct" mapping for every unlisted model) is the more robust way to handle this family of devices.Both fixes are scoped as tightly as possible: they only ever engage for devices using the default mapping, and only for behavior that was demonstrably broken. Devices with their own dedicated mapping file (currently product IDs
1245226668902080512and1442284873216843776) are not touched by either fix — their code paths are unchanged.Fix 1: invalid min/max temperature range in heat mode
Problem:
climate.warmepumpereportedmin_temp=35.0andmax_temp=27.0in heat mode — an invalid range (minimum greater than maximum). The app showed a valid heating range for the same device at the same time.Root cause: In heat mode,
mapping.default.jsonreads the minimum temperature from registerR10("Min. heat") and the maximum from registerR11("Max. heat"). On this device, those two registers are effectively swapped relative to what the generic mapping assumes, soR10ends up holding the larger value andR11the smaller one.Fix: Added
_get_device_temperature_range()incustom_components/aqua_temp/managers/aqua_temp_api.py. It now fetches the minimum and maximum temperature together (instead of via two independent, unrelated method calls as before), and ifminimum > maximum, it swaps the two values and logs a_LOGGER.warningso the correction is visible in the log. This is a purely defensive check — it only changes behavior when the reported range is already invalid, and it works for any device regardless of whether its product ID is recognized, so it doesn't depend on correctly identifying the device model.Tested: Confirmed on a real HA instance. After the fix,
climate.warmepumpereports a valid range (min_temp=27.0,max_temp=35.0) instead of the previous invalidmin_temp=35.0,max_temp=27.0.Fix 2: heat-mode setpoint changes never reach the physical device
Problem: Changing the target temperature in heat mode from Home Assistant had no effect on the physical unit at all. The Home Assistant entity would show the newly-set value, but the official vendor app kept showing the old, unchanged setpoint — i.e. the command was silently not applied by the device.
Root cause:
mapping.default.jsonuses registerR02("Heating set") as the heat-mode target-temperature register. On this device,R02is writable and reads back whatever was last written to it (so it looks like it's working from the integration's point of view), but it has no effect whatsoever on the device's actual behavior. The register that actually drives the device — both for what the vendor app displays and for real setpoint changes — isR01("Cooling set"), and this appears to hold true regardless of which HVAC mode is currently active.Evidence: To confirm this before writing the fix, a new setpoint was written from Home Assistant using the pre-fix code.
R02updated immediately to the new value (i.e. the write "succeeded" from the integration's perspective), but the official app — even after restarting the app — kept showing the previous, unchanged setpoint. After applying the fix so that writes go toR01instead, the official app (after an app restart, since it appears to cache the last known value) correctly showed the new setpoint. This demonstrates conclusively thatR01is the register that actually controls the device, and thatR02is non-functional for this class of device — not just occasionally stale or out of range, which means a plausibility/range check onR02's value alone could never have reliably caught this.Fix: Added
AquaTempConfigManager.is_default_mapping()inaqua_temp_config_manager.py, and_get_effective_target_temperature_pc()inaqua_temp_api.py, wired into the existing_get_target_temperature_protocol_code()(which is used by bothget_device_target_temperature, for reading, andset_temperature, for writing — so both directions automatically stay consistent with each other through this single method). Whenever the current HVAC mode isHEATand the device is using the default mapping, this now unconditionally uses the cool-mode target register (R01) instead of the heat-mode target register (R02) for both reading and writing the heat setpoint, and logs a_LOGGER.warningeach time it does so.Tested: Confirmed on a real HA instance. Writing a new setpoint from Home Assistant in heat mode is now correctly reflected in the official vendor app.
The reporter's physical unit is a heat-only pool heat pump with no cooling function, so cool mode could not be tested at any point during this work. Both fixes were deliberately kept as conservative as possible for exactly this reason:
minimum > maximum). For a device whose cool-mode range is already valid, this fix is a complete no-op.hvac_mode == HVACMode.HEAT. The cool-mode code path is never entered by this change and behaves exactly as it did before this PR, on every device.That said, maintainers or users with a device that actually supports cooling should verify cool mode still behaves correctly after this change, since it could not be verified by the reporter. If anything about the cool-mode assumptions here turns out to be wrong, please flag it — happy to adjust.
Scope / risk for other users
AquaTempConfigManager.is_default_mapping()check, which isTrueonly for devices without their ownmapping.<product_id>.jsonfile.1245226668902080512and1442284873216843776) are completely unaffected by both fixes — their code paths are unchanged.hvac_mode == HVACMode.HEAT, so cool and auto modes are unaffected even on default-mapping devices.mapping.default.jsonor any other mapping/entity-description JSON file — both are implemented as runtime validation/fallback logic inaqua_temp_api.py/aqua_temp_config_manager.py, so they apply automatically to any current or future unlisted device without needing a dedicated mapping file to be authored and shipped for it first.Version
This PR spans manifest versions
3.0.37→3.0.40(three commits, each bumping the version and adding aCHANGELOG.mdentry, per this repo's existing convention):3.0.38— Fix 1 (invalid min/max range)3.0.39— Fix 2, first iteration (conditional fallback based on a plausibility/range check)3.0.40— Fix 2, final iteration (unconditional fallback for HEAT + default mapping, after real-device testing showed the plausibility check couldn't reliably detect the non-functional register — see "Evidence" above)Testing
Both fixes were tested against a real, physical device (product ID
1548963836789501952) via a live Home Assistant instance, not just by code review:min_temp/max_tempbecoming a valid range, and via the new warning log line appearing when the swap is applied.Cool mode, as noted above, remains untested since the reporter's hardware cannot cool.