Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix to unload all platforms when reloading the integration #604

Merged
merged 3 commits into from
Oct 16, 2021
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions custom_components/tahoma/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,15 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
coordinator.update_interval = UPDATE_INTERVAL_ALL_ASSUMED_STATE

platforms = defaultdict(list)

# Sensor and Binary Sensor will be added dynamically, based on the device states
# Switch will be added dynamically, based on device features (e.g. low speed cover switch)
# Number will be added dynamically, based on device features (e.g. My position)
default_platforms = [BINARY_SENSOR, SENSOR, SWITCH, NUMBER]

for platform in default_platforms:
platforms[platform] = {}
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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..

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, ...]

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about the constant?

I'm OK to merge anyway.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's something I often see within Core. But I will not block for this.


platforms[SCENE] = scenarios

hass.data[DOMAIN][entry.entry_id] = {
Expand All @@ -199,13 +208,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
):
log_device("Unsupported device detected", device)

supported_platforms = set(platforms.keys())

# Sensor and Binary Sensor will be added dynamically, based on the device states
# Switch will be added dynamically, based on device features (e.g. low speed cover switch)
# Number will be added dynamically, based on device features (e.g. My position)
supported_platforms.update((BINARY_SENSOR, SENSOR, SWITCH, NUMBER))
for platform in supported_platforms:
for platform in platforms:
hass.async_create_task(
hass.config_entries.async_forward_entry_setup(entry, platform)
)
Expand Down