Clean up OpenUV config flow #17349
Clean up OpenUV config flow #17349
Conversation
ghost
added
the
bachya
changed the title
| self.hass, TOPIC_UPDATE, self._update_data) | ||
| self.async_on_remove(self._dispatch_remove) | ||
| @callback | ||
| def update(self): |
There was a problem hiding this comment.
This should not be a method if we nest it. Make it a function.
| })) | ||
|
|
||
| hass.data[DOMAIN][CONF_SCAN_INTERVAL] = conf[CONF_SCAN_INTERVAL] | ||
| hass.async_add_job( |
There was a problem hiding this comment.
Use hass.async_create_task.
|
|
||
| latitude = user_input.get(CONF_LATITUDE) | ||
| if not latitude: | ||
| latitude = self.hass.config.latitude |
There was a problem hiding this comment.
Maybe move defaults to form schema?
|
|
||
| identifier = '{0}, {1}'.format(latitude, longitude) | ||
| if identifier in configured_instances(self.hass): | ||
| return await self._show_form({'base': 'identifier_exists'}) |
There was a problem hiding this comment.
Mark the specific entry in the form that is causing the error, by using the corresponding form key instead of 'base' . Eg if it's the latitude that is bad, use CONF_LATITUDE.
| self.hass, TOPIC_UPDATE, self._update_data) | ||
| self.async_on_remove(self._dispatch_remove) | ||
| @callback | ||
| def update(self): |
|
|
||
| for component in ('binary_sensor', 'sensor'): | ||
| hass.async_create_task(hass.config_entries.async_forward_entry_setup( | ||
| config_entry, component)) | ||
|
|
||
| async def refresh_sensors(event_time): | ||
| async def refresh(event_time): | ||
| """Refresh OpenUV data.""" | ||
| _LOGGER.debug('Refreshing OpenUV data') | ||
| await openuv.async_update() | ||
| async_dispatcher_send(hass, TOPIC_UPDATE) | ||
|
|
||
| hass.data[DOMAIN][DATA_OPENUV_LISTENER][ | ||
| config_entry.entry_id] = async_track_time_interval( |
There was a problem hiding this comment.
Unsubscribe this listener before unloading the platforms. Otherwise there could perhaps be a race condition and another update come in before we disconnect the update callbacks in the platforms.
There was a problem hiding this comment.
Even if we do this, there could perhaps be a race if the state update has already been scheduled?
There was a problem hiding this comment.
Does 851ff3f address what you were thinking?
There was a problem hiding this comment.
Yes, but I'm not sure it actually prevents anything. I was thinking about the problem you described earlier, and that a scheduled update could be the cause.
There was a problem hiding this comment.
Got it. Let me experiment with removing the dispatcher altogether and see if that clears up my prior issue.
There was a problem hiding this comment.
I tried commenting all out all references to the dispatcher, but when I remove the config entry, I'm back to getting this weird exception:
Traceback (most recent call last):
File "/Users/abach/.local/share/virtualenvs/home-assistant-E4Cz_ciq/lib/python3.7/site-packages/aiohttp/web_protocol.py", line 390, in start
resp = await self._request_handler(request)
File "/Users/abach/.local/share/virtualenvs/home-assistant-E4Cz_ciq/lib/python3.7/site-packages/aiohttp/web_app.py", line 366, in _handle
resp = await handler(request)
File "/Users/abach/.local/share/virtualenvs/home-assistant-E4Cz_ciq/lib/python3.7/site-packages/aiohttp/web_middlewares.py", line 106, in impl
return await handler(request)
File "/Users/abach/Git/home-assistant/homeassistant/components/http/static.py", line 66, in staticresource_middleware
return await handler(request)
File "/Users/abach/Git/home-assistant/homeassistant/components/http/real_ip.py", line 34, in real_ip_middleware
return await handler(request)
File "/Users/abach/Git/home-assistant/homeassistant/components/http/ban.py", line 66, in ban_middleware
return await handler(request)
File "/Users/abach/Git/home-assistant/homeassistant/components/http/auth.py", line 68, in auth_middleware
return await handler(request)
File "/Users/abach/Git/home-assistant/homeassistant/components/http/real_ip.py", line 34, in real_ip_middleware
return await handler(request)
File "/Users/abach/Git/home-assistant/homeassistant/components/http/ban.py", line 66, in ban_middleware
return await handler(request)
File "/Users/abach/Git/home-assistant/homeassistant/components/http/auth.py", line 68, in auth_middleware
return await handler(request)
File "/Users/abach/Git/home-assistant/homeassistant/components/http/view.py", line 113, in handle
result = await result
File "/Users/abach/Git/home-assistant/homeassistant/components/config/config_entries.py", line 69, in delete
result = await hass.config_entries.async_remove(entry_id)
File "/Users/abach/Git/home-assistant/homeassistant/config_entries.py", line 392, in async_remove
entity_registry.async_clear_config_entry(entry_id)
File "/Users/abach/Git/home-assistant/homeassistant/helpers/entity_registry.py", line 254, in async_clear_config_entry
self._async_update_entity(entity_id, config_entry_id=None)
File "/Users/abach/Git/home-assistant/homeassistant/helpers/entity_registry.py", line 196, in _async_update_entity
new.update_listeners.remove(ref)
ValueError: list.remove(x): x not in list
I don't understand what listener the config entry is trying to remove?
There was a problem hiding this comment.
FYI, tracking this issue separately: #17370.
| hass.data[DOMAIN][DATA_OPENUV_CLIENT].pop(config_entry.entry_id) | ||
|
|
||
| remove_listener = hass.data[DOMAIN][DATA_OPENUV_LISTENER].pop( | ||
| config_entry.entry_id) | ||
| remove_listener() | ||
|
|
Description:
The
openuvconfig flow was my first; as such, there were lots of inefficiencies. This PR cleans them up and makes things slightly easier to read. Additionally, it fixes a bug wherein an explicitly configuredscan_interval(inconfiguration.yaml) was not handled properly.Related issue (if applicable): N/A
Pull request in home-assistant.io with documentation (if applicable): N/A
Example entry for
configuration.yaml(if applicable):Checklist:
tox. Your PR cannot be merged unless tests pass