search

GuyKh / ims-custom-component

The Israel Meteorological Service (IMS) integration component for home assistant
MIT License
43 stars 8 forks source link

Feature Request | On config flow add option to choose only specific entities #75

Closed aviadlevy closed 7 months ago

aviadlevy commented 9 months ago

Not everyone needs all entities. You can let the user to select which entity to add.
If you think it's a good idea, I'm willing to try and implement it (but it'll probably will take time)

Inspired from PiratWeather Hacs integration

image
GuyKh commented 9 months ago

Sound good. I'll look into it

GuyKh commented 9 months ago

@aviadlevy - give a try with 0.1.21

aviadlevy commented 9 months ago

@GuyKh , thanks for the quick implementation.

Seems like something is broken :/ 

Logger: homeassistant.config_entries
Source: config_entries.py:406
First occurred: 17:02:02 (2 occurrences)
Last logged: 17:04:05

Error setting up entry IMS Weather for ims
Traceback (most recent call last):
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 406, in async_setup
    result = await component.async_setup_entry(hass, self)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/ims/__init__.py", line 53, in async_setup_entry
    conditions = _get_config_value(entry, CONF_MONITORED_CONDITIONS)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/config/custom_components/ims/__init__.py", line 148, in _get_config_value
    return config_entry.data[key]
           ~~~~~~~~~~~~~~~~~^^^^^
KeyError: 'monitored_conditions'

I guess it wasn't backward compatibly.
My suggestion is revert current implementation and test new version on beta release (if such exists)

P.S.

Another thing I noticed is the default behaviour is changed. instead of having all the conditions as the default behaviour when no condition is checked, you have empty list here: https://github.com/t0mer/ims-custom-component/blob/a852d133562128d46f2b53170790702c4f34f70a/custom_components/ims/config_flow.py#L71

GuyKh commented 9 months ago

Good feedback. Will check

aviadlevy commented 9 months ago

I now noticed that I can use Configure here: image

After choosing conditions there, my integration loaded as expected (unfortunately that mean I won't be able to test what might will fix the issue without doing that)

GuyKh commented 9 months ago

Also, try to remove and add. It should work

aviadlevy commented 9 months ago

Also, try to remove and add. It should work

This was the best solution for me after all, since configuring existing configuration didn't auto remove unselected entities (I guess due to home assistant limitation preventing integration to remove entity)

GuyKh commented 9 months ago

Should be fixed with 0.1.22