kodebach / hacs-idm-heatpump

HACS integration for IDM heat pumps
MIT License
20 stars 1 forks source link

Assertionerror when writing new pv values #79

Closed AndyNew2 closed 5 months ago

AndyNew2 commented 5 months ago

I noticed there is an asertionerror when I try to write the new pv values (house consumption, battery power). In case the values are bigger than 0 (usual day work) the values are not written to the heatpump and the write is aborted with an asertionerror. I have following in the log. Reading the logic, I think the assertion is wrong?

AssertionError 2024-01-27 17:46:30.457 ERROR (MainThread) [homeassistant.components.automation.idm_waermepumpe_pv_schreiben] While executing automation automation.idm_waermepumpe_pv_schreiben Traceback (most recent call last): File "/usr/src/homeassistant/homeassistant/components/automation/init.py", line 669, in async_trigger await self.action_script.async_run( File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 1587, in async_run return await asyncio.shield(run.async_run()) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 426, in async_run await self._async_step(log_exceptions=False) File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 479, in _async_step self._handle_exception( File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 502, in _handle_exception raise exception File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 476, in _async_step await getattr(self, handler)() File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 713, in _async_call_service_step response_data = await self._async_run_long_action( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/src/homeassistant/homeassistant/helpers/script.py", line 675, in _async_run_long_action return long_task.result() ^^^^^^^^^^^^^^^^^^ File "/usr/src/homeassistant/homeassistant/core.py", line 2149, in async_call response_data = await coro ^^^^^^^^^^ File "/usr/src/homeassistant/homeassistant/core.py", line 2186, in _execute_service return await target(service_call) ^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/config/custom_components/idm_heatpump/sensor.py", line 48, in handle_set_power await entity.async_write_value(value) File "/config/custom_components/idm_heatpump/entity.py", line 87, in async_write_value return await self.coordinator.async_write_value(self.sensor_address, value) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/config/custom_components/idm_heatpump/coordinator.py", line 47, in async_write_value raise exception File "/config/custom_components/idm_heatpump/coordinator.py", line 44, in async_write_value return await self.heatpump.async_write_value(address, value) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/config/custom_components/idm_heatpump/idm_heatpump.py", line 310, in async_write_value address.encode(builder, value) File "/config/custom_components/idm_heatpump/sensor_addresses.py", line 166, in encode self.max_value is None or value <= self.max_value ^^^^^^^^^^^^^^^^^^^^^^^

AndyNew2 commented 5 months ago

Ah found the issue. it is in the file sensor_addresses.py but in a different location: the line 608 needs removing because the sensor for
address=84, name="power_drain_battery", The power drain for battery can be negative, in case it is charged. IDM GUI shows the arrow accordingly, therefore negative values are perfectly right. Too much checking ;-) I think the other restrictions are fine, but you know me optinion for these checks...

kodebach commented 5 months ago

Ah of course negative drain values make sense for "battery is charging". I'll fix that for the next release.

I think the other restrictions are fine, but you know me optinion for these checks...

The checks are there, because without them the error scenario would be even more confusing. Not sure what exactly would happen, but there are only two options:

  1. You get an error with stack trace like this one. But instead of leading to an assertion checking value ranges, it points to the code sending the Modbus request and the error message will just be something like "Modbus request failed" without clear indication why. That's because Modbus has some error codes, but can't report a message saying "this value is out of range, the max value is X".
  2. It fails completely silently. The Modbus request succeeds, but because the value is out of range, the heat pump either ignores it completely or uses a different value (e.g. the closest allowed value).

Both options are worse than the current situation and judging by your experience in https://github.com/kodebach/hacs-idm-heatpump/issues/69#issuecomment-1877298669, I'd say option 2 is more likely and silent failures are always really confusing.

That said, maybe I can find a way to report a more user friendly error message so it is immediately clear what happened.

kodebach commented 5 months ago

Fixed in v0.7.1