arendst / Tasmota

Alternative firmware for ESP8266 and ESP32 based devices with easy configuration using webUI, OTA updates, automation using timers or rules, expandability and entirely local control over MQTT, HTTP, Serial or KNX. Full documentation at
https://tasmota.github.io/docs
GNU General Public License v3.0
21.74k stars 4.72k forks source link

Quick Fix: KNX loose config on restart #21378

Closed barbudor closed 2 months ago

barbudor commented 2 months ago

Description:

Related issue (if applicable): fixes https://github.com/arendst/Tasmota/issues/21379 (initially reported here https://github.com/arendst/Tasmota/issues/20834#issuecomment-2096809259)

This restore the way @ascillato was testing the KNX configuration against some sensors enabled.

Solve the above issue but prevent to use any temperature or humidity sensor than those listed, excluding also all I2C sensors.

Checklist:

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

barbudor commented 2 months ago

Hello @ascillato I know that you may not have time but just in case, what do you think of my proposal to remove that configuration check ? I think it would be better as it allows to use any T& H sensor, including I2C ones Thanks

ascillato commented 2 months ago

Hello,

what do you think of my proposal to remove that configuration check ? I think it would be better as it allows to use any T& H sensor, including I2C ones

I wouldn't remove the whole check. I think, we should add the new sensor types you added to the driver instead. The check is mainly to not mix the config when changing the module type. Without this check, in the case an user adds a KNX config and then decides to change the module type to a template or whatever, the config will be scrambled and the only solution is a full reset.

So, I think we should just add to that check the new types. What do you think?

I would not be able to do that these days since I'm not at home but I can look into any change you propose.

barbudor commented 2 months ago

It is not possible to add a check for sensors that will be detected later in the process such as i2c sensors There's no way at that time to know if a i2c T/H sensor is present

I would compare that to Tasmota fully erasing it's configuration is the user configure a DS18X20 and then the DS driver doesn't find the sensor

I will revert to your previous code but it's a pity if KNX couldn't work with i2c sensors

barbudor commented 2 months ago

@ascillato could you please provide a scénario where not having such test would be a problem ?

barbudor commented 2 months ago

Except for the format of Energy payload, I reverted to the orginal code from @ascillato including the configuration check and the limitation to only some temperature and humidity sensors

That should be good to merge now

barbudor commented 2 months ago

Thanks Theo

pbrinette commented 1 month ago

Hi @barbudor. I've tested the latest commits on my ESP32 + DHT11 enabled. it seems to work as expected. The KNX configuration is persistant after the restart. Thanks.