Fix to unload all platforms when reloading the integration #604
Conversation
custom_components/tahoma/__init__.py
Outdated
| default_platforms = [BINARY_SENSOR, SENSOR, SWITCH, NUMBER] | ||
|
|
||
| for platform in default_platforms: | ||
| platforms[platform] = {} |
There was a problem hiding this comment.
To be honest, I would like a nicer solution, but I couldn't get it working. Not sure what kind of datastructure we should use..
There was a problem hiding this comment.
I don’t like it too. But we can see later to clean this. You can one line if you want:
>>> p = defaultdict(list)
>>> p.update(dict.fromkeys(("a", "b"), [])) # [] and not {}
>>> p
defaultdict(<class 'list'>, {'a': [], 'b': []})
We can also just declare a constant with all the supported platform:
SUPPORTED_PLATFORMS = [BINARY_SENSOR, SENSOR, SWITCH, NUMBER, COVER, ...]
There was a problem hiding this comment.
@tetienne I tried the defaultdict approach with the fromkeys(), but that was also not my favourite codestyle :(
Shall we merge this one in for now, and look for a nicer solution? (perhaps during core review)
There was a problem hiding this comment.
What about the constant?
I'm OK to merge anyway.
There was a problem hiding this comment.
@tetienne I thought about a constant, however we only need it at a single location. That's why I moved it back.
Should we put the constant + comments at the top for readability?
There was a problem hiding this comment.
That's something I often see within Core. But I will not block for this.
|
We can still ease the code, see https://github.com/home-assistant/core/blob/6a72af63c28e27845d456aadad105677e9aaf07e/homeassistant/components/tuya/__init__.py#L142 and |
Co-authored-by: Thibaut <[email protected]>
Looks great, shall we do this optimalisation in a follow up PR? |
|
It would be nice yes. |
Fix #560