search

denpamusic / homeassistant-plum-ecomax

Plum ecoMAX boiler controller integration for Home Assistant.
MIT License
33 stars 8 forks source link

Set parameter confirmation failed #156

Open gednet opened 6 days ago

gednet commented 6 days ago

Is there an existing issue for this?

I'm having the following issue:

When i try to set parameter for mixer or boiler i have error "Request to set a parameter failed (heating_curve)", but when i clicked second time without any changes it works without error. But after first click the parameter is seted. Every time when i change value of the parameter error is showed, but parameter is seted. For example:

action: plum_ecomax.set_parameter
target:
  device_id:
    - a215485a9d0b73acdaaa14c459efd6a6
data:
  name: heating_curve
  value: "0.7"

Perform action will set parameter, but without success confirmation and error will showed. Next time when i perform action with the same value it will end without error and with success. But when i change parameter to different vaule it will set parameter, but with error.

I have following devices connected:

I'm connecting to my devices using:

Ethernet/WiFi to RS-485 converter

I'm seeing following log messages:

Ten błąd pochodzi z niestandardowej integracji.

Rejestrator: pyplumio.helpers.parameter
Źródło: custom_components/plum_ecomax/services.py:225
integracja: Plum ecoMAX (dokumentacja, Problemy)
Pierwsze zdarzenie: 10:39:07 (5 zdarzenia)
Ostatnio zalogowany: 11:01:14

Timed out while trying to set 'heating_curve' parameter

My diagnostics data:

config_entry-plum_ecomax-01JANKZFDAJZSHVGKM66N96QJP (4).json

Code of Conduct

denpamusic commented 6 days ago

Hi,

Thank you for the feedback!

This is actually known issue for some of ecoMAX'es (EM). It has to do with frame versioning that PyPlumIO (PIO) uses for confirmation.

I noticed that some EM models increase frame 49 version number on local changes on the panel itself, but don't increase it on remote change, which makes PIO think that set command has failed.

There're a couple way to address this, for example by forcing update via queuing up a parameter request or by directly changing the version number to 0. The only issue I have with doing it this way is because it breaks inheritance a litte bit, but it should be fine.

Related to: #59

gednet commented 6 days ago

Ok, but in my disgnostic data in frame_version data i don't see type 49. Maybe here the problem is? How i can directly change the version number to 0 when i don't have that type? Second time when i try to save the same value i will have "ok". Why? Maybe i can help you to resolve this issue? If i can chceck some solution for you give me a sign.

denpamusic commented 6 days ago

Not having 49 in frame version data will indeed cause the described problem. Both options that I described require software change in PIO, that I'm going to implement.

You can help by testing them once they are done. I'll try to do it tomorrow and contact you with a way to install test version and check whether issue is resolved or not. Thank you for your help!

edit. Setting same value for a second time works because when values stay the same, no checks are performed whatsoever and operation is assumed as successful.

denpamusic commented 5 days ago

I added confirmation fallback for the cases when the refresh request is not listed in versioning table, like with you device.

Each parameter instance now contains property is_tracking_changes that returns True if device versioning table contains required frame id or False otherwise. We use this property in Parameter.set() method to schedule manual refresh via awaiting Parameter.force_refresh() right after queuing up parameter change request if Parameter.is_tracking_change is False. Doing it this way ensures that if required frame id does exist in the versioning table, we don't perform refresh request twice.

Conversely, it also enables us to use Parameter.force_refresh() method anywhere in the code to refresh any parameter we want from the remote via high level API, which was previously only possible by using low level API.

I'll appreciate, if you can test these changes on your device, by copying following contents to config/custom_components/plum_ecomax/manifest.json file and restarting homeassistant:

{
  "domain": "plum_ecomax",
  "name": "Plum ecoMAX",
  "codeowners": [
    "@denpamusic"
  ],
  "config_flow": true,
  "dependencies": [
    "network"
  ],
  "documentation": "https://github.com/denpamusic/homeassistant-plum-ecomax",
  "integration_type": "hub",
  "iot_class": "local_push",
  "issue_tracker": "https://github.com/denpamusic/homeassistant-plum-ecomax/issues",
  "loggers": [
    "pyplumio"
  ],
  "requirements": [
    "git+https://github.com/denpamusic/pyplumio.git@parameter_refresh_fallback"
  ],
  "version": "0.4.9"
}

Please note that getting confirmation from EM can take up to three seconds, so don't click "Perform action" button multiple time until previous request is finished with success or failure.

gednet commented 5 days ago

It seems to work very well. The response from the controller takes longer, about 4 seconds. Thank you very much! After couple different tests the changes working.

denpamusic commented 5 days ago

Awesome! Glad I was able to help. I'll merge this change into main and make new release a bit later.

For the response time, there's sadly not a lot I can do in PIO without converting request queue into priority queue which will complicate queue handling by a lot.

I think this can be improved by adding a checkbox in HASS that will determine if we need to wait for confirmation from the controller or just send change request and assume that it successfully got through.