-
-
Notifications
You must be signed in to change notification settings - Fork 33
Improve/fix support for Bosch Junkers boilers through EMS-ESP #154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
8fce77d
3dd08c6
6f49276
338b615
be3750f
67e2d18
1a6ed15
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clamp once and pass the clamped value to You publish a clamped value but forward the original 🔧 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
Suggested change
🤖 Prompt for AI Agents
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: Length of output: 12714
You're absolutely right - with the new code, when the heater is off and SAT sends its default 10°C setpoint, it will now be clamped to 12°C at the device instead of the previous behavior where it sent 0°C (resulting in 5°C at the device). Old behavior:
New behavior:
This is a significant difference (7°C higher when off). If maintaining the lower temperature when the heater is off was intentional for energy efficiency or to keep the boiler truly idle, you may want to preserve that special case. Here's how you could do that: 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°.
# When heater is off (value == 10), send 0 to keep device at 5°C as before.
if value == 10:
adjusted_value = 0
else:
adjusted_value = max(value, 12)
await self._publish_command(f'{{"cmd": "selflowtemp", "value": {adjusted_value}}}')
await super().async_set_control_setpoint(value)Is the new 12°C behavior when off acceptable for your use case, or would you prefer to keep the device at 5°C when the heater is off? |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
@@ -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
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keep 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We never send zero here. When the heater is off, we default to 10 degrees, which translates to 12 degrees at the device according to your code. Is this expected?