emsesp / EMS-ESP32

ESP32 firmware to read and control EMS and Heatronic compatible equipment such as boilers, thermostats, solar modules, and heat pumps
https://emsesp.github.io/docs
GNU Lesser General Public License v3.0
589 stars 100 forks source link

Invalid value for number.thermostat_hc1_selected_room_temperature: 30 (range 0.0 - 29.0) #647

Closed nagyrobi closed 2 years ago

nagyrobi commented 2 years ago

Bug description Home assistant spams the following message on every thermostat mqtt message when thermostat_hc1_selected_room_temperature goes to 30:

[homeassistant.components.mqtt.number] Invalid value for number.thermostat_hc1_selected_room_temperature: 30 (range 0.0 - 29.0)

This is on a Generic HT3, with a controller detected as thermostat FW200 (not having an actual EMS thermostat in the system), in a Junkers Ceraclass Excellence ZSC 28-3 MFA E 23.

Observed that this value is tied to climate.thermostat_hc1's temperature value. The thermostat entry has max_temp: 30, and when it's set to that, number.thermostat_hc1_selected_room_temperature tries to go there by itself to the same value.

Similar to https://github.com/emsesp/EMS-ESP32/issues/469, only that the top value is capped the code. Perhaps it would suffice not to hardcode the max to 30, but instead make it equal with thermostat's max_temp.

ems-esp32: v3.4.2 home assistant: 2022.9.6

MichaelDvP commented 2 years ago

@proddy Seems that bosch changes the limits with every device. Should we adapt the limit dynamically for ems-reported values out of range. e.g. add to emsdevice::generate values

                if ((output_target == OUTPUT_TARGET::MQTT) && (dv.min != 0 || dv.max != 0)) {
                    if (json[name].is<float>() || json[name].is<int>()) {
                        int v = json[name];
                        if (fahrenheit) {
                            v = (v - (32 * (fahrenheit - 1))) / 1.8; // reset to °C
                        }
                        if (v < dv.min) {
                            dv.min = v;
                            dv.remove_state(DeviceValueState::DV_HA_CONFIG_CREATED);
                        } else if (v > dv.max) {
                            dv.max = v;
                            dv.remove_state(DeviceValueState::DV_HA_CONFIG_CREATED);
                        }
                    }
                }

This should hopefully end all HA complains about min/max values.

MichaelDvP commented 2 years ago

I thought about user defined min/max limits without wasting a lot of RAM. I think i've found a solution storing user defined values in the customization in a single string only if set e.g. 00hc2/daytemp|Tagestemp>5<30 if Name and both limits set . The input looks like this: grafik and does not show the default min/max, because they are overwritten in RAM on change and ems-esp does not hold the initial values in ram. When resetting customizations to default or clearing a min or max value the last set value stays until reboot. But i think customizations are rarly used and mainly set once, so it's not a big disadvantage. @proddy Do you think it's usefull? Should i make a pr?

proddy commented 2 years ago

hehe...I was lying in bed and thinking almost the same solution as you. I had (x,y) but your > and < is much cooler

do the PR!

proddy commented 2 years ago

Did some testing on the min/max. it works well. two things I'll look into

proddy commented 2 years ago

I'm working on a PR to address those two points above

MichaelDvP commented 2 years ago

Take a look at this approach. Not tested yet, For HA checks i need a running mosquitto and reconfigure the ems-connected gateway to it.

proddy commented 2 years ago

Take a look at this approach. Not tested yet, For HA checks i need a running mosquitto and reconfigure the ems-connected gateway to it.

that's similar what I had. Let's go with that. I can test locally if you want. I do most of my tests using the standalone/make run way and controlling the output of the MQTT topics being created. But I can also test on HA with it's embedded mosquitto - it's just a container I need to spin up