Skip to content
17 changes: 13 additions & 4 deletions custom_components/sat/mqtt/ems.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,9 @@ def get_tracked_entities(self) -> list[str]:
return [DATA_BOILER_DATA]

async def async_set_control_setpoint(self, value: float) -> None:
await self._publish_command(f'{{"cmd": "selflowtemp", "value": {0 if value == 10 else value}}}')
# Minimum valid setting for Bosch/Junkers boiler seems to be 12°.
# Lower values set the boiler to 12° except 0° which sets the boiler to 5°.
await self._publish_command(f'{{"cmd": "selflowtemp", "value": {max(value, 12)}}}')

await super().async_set_control_setpoint(value)
Comment on lines 111 to 116
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Clamp once and pass the clamped value to super() for state consistency.

You publish a clamped value but forward the original value to the superclass, which can leave the internal state/UI out of sync with the actual boiler command. Consider clamping once and using that value for both calls.

🔧 Proposed fix
-        await self._publish_command(f'{{"cmd": "selflowtemp", "value": {max(value, 12)}}}')
+        clamped_value = max(value, 12)
+        await self._publish_command(f'{{"cmd": "selflowtemp", "value": {clamped_value}}}')
 
-        await super().async_set_control_setpoint(value)
+        await super().async_set_control_setpoint(clamped_value)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async def async_set_control_setpoint(self, value: float) -> None:
await self._publish_command(f'{{"cmd": "selflowtemp", "value": {0 if value == 10 else value}}}')
# Minimum valid setting for Bosch/Junkers boiler seems to be 12°.
# Lower values set the boiler to 12° except 0° which sets the boiler to 5°.
await self._publish_command(f'{{"cmd": "selflowtemp", "value": {max(value, 12)}}}')
await super().async_set_control_setpoint(value)
async def async_set_control_setpoint(self, value: float) -> None:
# Minimum valid setting for Bosch/Junkers boiler seems to be 12°.
# Lower values set the boiler to 12° except 0° which sets the boiler to 5°.
clamped_value = max(value, 12)
await self._publish_command(f'{{"cmd": "selflowtemp", "value": {clamped_value}}}')
await super().async_set_control_setpoint(clamped_value)
🤖 Prompt for AI Agents
In `@custom_components/sat/mqtt/ems.py` around lines 111 - 116, Compute a single
clamped value once (e.g. clamped = max(value, 12)) inside
async_set_control_setpoint and use that clamped variable for both the
_publish_command payload and the call to super().async_set_control_setpoint so
the internal state/UI stays in sync with the actual command sent (update
references in async_set_control_setpoint to use the clamped value for both the
publish and the super call).


Expand All @@ -123,12 +125,19 @@ async def async_set_control_thermostat_setpoint(self, value: float) -> None:
await super().async_set_control_thermostat_setpoint(value)

async def async_set_heater_state(self, state: DeviceState) -> None:
await self._publish_command(f'{{"cmd": "heatingactivated", "value": "{DATA_ON if state == DeviceState.ON else DATA_OFF}"}}')

# Do not send 'heatingoff' command, as this leads to EMS toggling the boiler between
# pre-set heating and selected flow temperature. Instead, control on/off solely by setting
# a low flow temperature (SAT already does this).
# (see https://github.com/emsesp/EMS-ESP32/discussions/2641#discussioncomment-14611481)
# The alternative command 'heatingactivated` also interferes with EMS, so sending nothing
# here seems to be the correct way to handle it.
await super().async_set_heater_state(state)

async def async_set_control_max_relative_modulation(self, value: int) -> None:
await self._publish_command(f'{{"cmd": "burnmaxpower", "value": {max(value, 20)}}}')
# Do not set 'burnmaxpower' as this is an EEPROM-stored value and will wear out the EEPROM.
# Use 'selburnpow' instead.
# (see https://github.com/emsesp/EMS-ESP32/discussions/2641#discussioncomment-14611481)
await self._publish_command(f'{{"cmd": "selburnpow", "value": {max(value, 20)}}}')

await super().async_set_control_max_relative_modulation(value)
Comment on lines 136 to 142
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Keep super() in sync with the clamped modulation value.

The published value is clamped, but the superclass receives the unclamped input, which can desync reported vs. applied max modulation.

🔧 Proposed fix
-        await self._publish_command(f'{{"cmd": "selburnpow", "value": {max(value, 20)}}}')
+        clamped_value = max(value, 20)
+        await self._publish_command(f'{{"cmd": "selburnpow", "value": {clamped_value}}}')
 
-        await super().async_set_control_max_relative_modulation(value)
+        await super().async_set_control_max_relative_modulation(clamped_value)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async def async_set_control_max_relative_modulation(self, value: int) -> None:
await self._publish_command(f'{{"cmd": "burnmaxpower", "value": {max(value, 20)}}}')
# Do not set 'burnmaxpower' as this is an EEPROM-stored value and will wear out the EEPROM.
# Use 'selburnpow' instead.
# (see https://github.com/emsesp/EMS-ESP32/discussions/2641#discussioncomment-14611481)
await self._publish_command(f'{{"cmd": "selburnpow", "value": {max(value, 20)}}}')
await super().async_set_control_max_relative_modulation(value)
async def async_set_control_max_relative_modulation(self, value: int) -> None:
# Do not set 'burnmaxpower' as this is an EEPROM-stored value and will wear out the EEPROM.
# Use 'selburnpow' instead.
# (see https://github.com/emsesp/EMS-ESP32/discussions/2641#discussioncomment-14611481)
clamped_value = max(value, 20)
await self._publish_command(f'{{"cmd": "selburnpow", "value": {clamped_value}}}')
await super().async_set_control_max_relative_modulation(clamped_value)
🤖 Prompt for AI Agents
In `@custom_components/sat/mqtt/ems.py` around lines 136 - 142, The method
async_set_control_max_relative_modulation clamps the published value but still
passes the original unclamped value to the superclass, causing a desync; change
it to compute a single clamped variable (e.g., clamped = max(value, 20)), use
that clamped value in the JSON sent to selburnpow, and call
super().async_set_control_max_relative_modulation(clamped) so both the device
and the superclass state stay consistent.


Expand Down