Flow setup pulls incorrect variables from combined sensors into incorrect windows about thermal_comfort HOT 18 CLOSED

The list of sensors/switches in 1.4.1 was very long for me (probably 200-300 entries) which required long scrolling to get to sensor.outside..... etc.
What I realized now is that the a-z order inside the dropdown menu restarts more than once (2-3 times?) and thus there are several sensor.o(...) sections, which probably means I skipped over the first one and ended up going to the next one where I found the sensor.outside.rh as opposed to temp which was in the first sensor.o(...) section up top.

Ok that's very long. I didn't know about the a-z order repeating several times, thats not intentional. It's only intended to restart once. We have to look into this. The idea behind restarting the a-z order was to keep the better matching candidates, basically the list you get in non advanced, still on top in the advanced mode so you would find them faster.

I would reason however that using the global advanced/basic option from the general home assistant settings makes this very limiting. Likewise, I may be wrong, but I would think the vast majority of users are utilizing the advanced feature set (like dev tools), so an integration specific toggle for limiting the available sensors would be more convenient, but it's one man's opinion, for your consideration.

I fully understand your concern and reasoning. We also discussed between us how to best handle the issue before implementing this, and what you got is mostly a compromise between opposing concerns. The main problem is that as a custom integration we are kind of restricted in the frontend features we can use. In an ideal world I would have preferred to add text fields with autosuggestions for the entities including their friendly name and icon similar to other parts of the frontend. Right now it seems we have a solution that is usable but neither ideal for the average user nor the power user. The advanced mode toggle isn't ideal but just how home assistant implements this feature for config flow. It would be nice if they added a simple button to switch the mode just for the config flow. The best I can think of would be to add an additional step before the actual sensor selection that asks you if you want to do the setup in advanced mode or not.

from thermal_comfort.

If there is a chance to improve it further, to exclude completely unrelated sensors from the selection window, to minimize the need for sifting through them (e.g. battery or phone link quality), that would probably tie well into this issue.

If you disagree, I can put another ticket up, as an improvement idea.

from thermal_comfort.

Hi thank you for your report.

Regarding the wrong sensors could you make a screenshot of the state of them in developer tools?

Regarding available sensor selection. With advanced mode disabled you should only get sensors with the correct sensor device class and correct units of measurements. With advanced mode we don't filter out entities without device class or unit of measurement. But we filter those out that have the explicit wrong type of unit of measurement, domain or device class. The reason for this is, that not all sensor integrations support device class e.g. command_line or input_number and we want advanced users to still be able to select them. If you have any unit of measurement, domain or device_class in mind that we additionally should exclude from the selection your can open an issue for that.

from thermal_comfort.

Thank you very much for the prompt response and a great integration I hope to be able to use!

Just for your understanding, it's this exact Xiaomi module integrated via Zigbee2mqtt.
All published values can be found here:
https://www.zigbee2mqtt.io/devices/WSDCGQ11LM.html
Please see the screenshot requested:

I am grateful for the explanation of the exclusion/inclusion principles at play.
In this case, it may be exactly the root cause of the issue, since while the state is clearly indicated, it seems more than one variable is reported in the attributes of this sensor, which may through the integration into a tailspin if it considers both state and variable when looking for the sensor type.

Finally, my apologies if it's there, but I was not able to find the advanced mode toggle anywhere, to allow a richer or more stripped down list of sensors included.

from thermal_comfort.

@IATkachenko do you have any idea what could cause this?

from thermal_comfort.

Upon further consideration and reviewing the list it actually also including unrelated entities like switches and completely different class variables such as sun.sun.

Neither of these should be found under sensors in the list, unless I am misunderstanding your description of the exclusion principles.

from thermal_comfort.

In get_sensors_by_device_class() we allow "advanced" users select any type of sensor, but exclude obviously useless.
We have no filters by % unit of measurement for temperature sensor and Celsius for humidity. May be it is a good idea to have additional filter.

About switch filtering. Are you sure that nobody have switches for temperature state, for example? Like "day/night temperature switch". But it looks like switch should be excluded from list too, because it have state on or off, not a digital value.

from thermal_comfort.

I just checked. Switch and sun domain are still missing from the exclusion list. Makes sense to add them.

from thermal_comfort.

Done.
Colleagues, what about climate domain? Should it be excluded too? Looks like thermal_comfort may get data from climate, but not now.

from thermal_comfort.

Exclude it for now. That would require flow ui redesign. We can add it as feature in the future.

from thermal_comfort.

Thank you for the swift action on the excessive list of modules!

What remains an issue is the main one in this ticket, which is that the RH is presented in the temp sensor, but not a temp sensor for the same device, and vice versa.

I don't believe the changes in question would do anything for that.

Just to follow-up, how would one enable or disable the advanced mode, to limit the sensors available?

from thermal_comfort.

Just to follow-up, how would one enable or disable the advanced mode, to limit the sensors available?

Click at user avatar at bottom-left conner in UI.

from thermal_comfort.

What abut RH? What is wrong now, in master version of the integration, after #125?

from thermal_comfort.

@IATkachenko for reference regarding climate usage for a similar integration https://github.com/Limych/ha-temperature-feels-like

from thermal_comfort.

@convicte i released 1.4.3 that includes #125. Could you give it another try and compare the sensor lists between advanced and non advanced mode. I am not sure yet what causes the issue and will only have time later this week to dig deeper into the filters.

from thermal_comfort.

@rautesamtr I have now managed to add my first sensor combination successfully using the GUI under 1.4.3.

The number of sensors in Advanced mode shrunk down significantly, which is very appreciated.
I believe to have found a potential reason for my problems, which were due to a confusion rather than the RH/temp sensors truly missing. I know it sounds silly, but please bear with me as I explain.

The list of sensors/switches in 1.4.1 was very long for me (probably 200-300 entries) which required long scrolling to get to sensor.outside..... etc.
What I realized now is that the a-z order inside the dropdown menu restarts more than once (2-3 times?) and thus there are several sensor.o(...) sections, which probably means I skipped over the first one and ended up going to the next one where I found the sensor.outside.rh as opposed to temp which was in the first sensor.o(...) section up top.

I wonder if list restarting is due to their being different selection criterions for which sensors to include in the dropdown menu, and each is creating its own alphabetically ordered list?

@IATkachenko thank you for clarifying the advanced mode. I must admit having never toggled it off since I installed home assistant years ago and totally forgot about its existence.

I would reason however that using the global advanced/basic option from the general home assistant settings makes this very limiting. Likewise, I may be wrong, but I would think the vast majority of users are utilizing the advanced feature set (like dev tools), so an integration specific toggle for limiting the available sensors would be more convenient, but it's one man's opinion, for your consideration.

from thermal_comfort.

Thank you for all your detailed feedback and the look behind the developer's curtain.

1. In my case, even if it restarts twice, with over 50-60 good matches and 200-300 bad once was enough for my dyslexic mind to fail to realize. ;)
2. If there could be a differentiator between the lists, that could help (e.g.bold text)

Re:limitations: I understand and would think that potentially a context aware search would solve the issue all together, since differentiation would not be required. Though, from what you describe, this will also not work due to HA frontend.

from thermal_comfort.

Since the original issue isn't valid anymore i opened #135 so it's possible to discuss better alternatives for the input sensor selection there.

from thermal_comfort.