Add new integration dyson_local #53801
Conversation
probot-home-assistant
bot
added
has-tests
new-integration
integration: dyson_local
labels
|
Working on the documentation |
There was a problem hiding this comment.
Limit the initial PR to 1 platform so I can continue with the review.
https://developers.home-assistant.io/docs/creating_component_code_review#5-make-your-pull-request-as-small-as-possible
|
|
||
| from .const import CONF_CREDENTIAL, CONF_DEVICE_TYPE, CONF_SERIAL, DATA_DEVICES, DOMAIN | ||
|
|
||
| _PLATFORMS = ["binary_sensor", "sensor", "vacuum"] |
There was a problem hiding this comment.
Limit initial PR to 1 platform.
| async def async_setup(hass: HomeAssistant, config: dict) -> bool: | ||
| """Set up Dyson integration.""" | ||
| hass.data[DOMAIN] = { | ||
| DATA_DEVICES: {}, | ||
| } | ||
| return True |
There was a problem hiding this comment.
Move this to async_setup_entry so async_setup can be removed.
| for component in _PLATFORMS: | ||
| hass.async_create_task( | ||
| hass.config_entries.async_forward_entry_setup(entry, component) | ||
| ) |
There was a problem hiding this comment.
| for component in _PLATFORMS: | |
| hass.async_create_task( | |
| hass.config_entries.async_forward_entry_setup(entry, component) | |
| ) | |
| hass.config_entries.async_setup_platforms(entry, PLATFORMS) |
|
|
||
| from .const import CONF_CREDENTIAL, CONF_DEVICE_TYPE, CONF_SERIAL, DATA_DEVICES, DOMAIN | ||
|
|
||
| _PLATFORMS = ["binary_sensor", "sensor", "vacuum"] |
There was a problem hiding this comment.
| _PLATFORMS = ["binary_sensor", "sensor", "vacuum"] | |
| PLATFORMS = ["binary_sensor", "sensor", "vacuum"] |
| return unload_ok | ||
|
|
||
|
|
||
| class DysonEntity(Entity): |
There was a problem hiding this comment.
Use entity class or instance attributes instead of properties if possible:
https://developers.home-assistant.io/docs/core/entity/#entity-class-or-instance-attributes
| """Dyson local config flow.""" | ||
|
|
||
| VERSION = 1 | ||
| CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_PUSH |
There was a problem hiding this comment.
This was moved to the manifest.
| CONNECTION_CLASS = config_entries.CONN_CLASS_LOCAL_PUSH |
|
|
||
|
|
||
| class CannotConnect(HomeAssistantError): | ||
| """Represents connection failure.""" | ||
|
|
||
|
|
||
| class CannotFind(HomeAssistantError): | ||
| """Represents discovery failure.""" | ||
|
|
||
|
|
||
| class InvalidAuth(HomeAssistantError): | ||
| """Represents invalid authentication.""" |
There was a problem hiding this comment.
Not used.
| class CannotConnect(HomeAssistantError): | |
| """Represents connection failure.""" | |
| class CannotFind(HomeAssistantError): | |
| """Represents discovery failure.""" | |
| class InvalidAuth(HomeAssistantError): | |
| """Represents invalid authentication.""" |
|
Hi @shenxn, thanks for the integration PR. Can the current Dyson integration be replaced by this one? According to the analytics data, there are no users. And in issues #46400, etheralm/libpurecool#37 users indicate that the current integration is not working since 11/01/2021. Can you confirm that the current integration is no longer working for anyone? |
|
Yeah, we should probably merge this into the existing |
|
Will do. |
|
Would it be easier to review if we got rid of the existing I could pick that up if it would be helpful. Regarding putting it in |
Some people reported being able to use the old integration by signing in on the mobile app just before or something like that at some point, but I think the analytics mostly report that because nobody is able to use it anymore, confirming that it's entirely broken today. I for one still have it enabled and configured - it just fails to start. |
|
Yeah i spent some time trying to get it working (having had it configured and working previously) and it wouldn't. It just doesn't know the device credentials if it can't login to dyson cloud (as it doesn't cache them AIUI) so (again AIUI) it can't possibly ever work. I think the dyson cloud API has changed again since those people were able to trick it into working with the mobile app. (For comparison for reviewers not familiar, this new integration is able to use the password printed on a sticker that came with the fan and completely bypass their cloud, worked first time. It's great). |
Yes, we can create a separate target branch, copied from dev, then PR a removal of the old dyson integration to that branch, and then rebase here onto that branch. |
|
Since this is kind of not moving forward, I have created the removal PR #59401. |
|
This PR should probably be closed, shenxn is currently too busy to maintain the dyson_local repo anymore. I took over the maintenance of the integration, I can make a new PR to integrate it here. |
|
Ok, I'll close here for now. |
Proposed change
This PR adds a new integration
dyson_local. This merges the custom integration ha-dyson. This PR only contains the support to Dyson 360 Eye vacuum. Supports to more devices will be added in followup PRs.Type of change
Additional information
Checklist
black --fast homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all..coveragerc.The integration reached or maintains the following Integration Quality Scale:
To help with the load of incoming pull requests: