Skip to content

Add Abode entity available property#32923

Merged
bdraco merged 5 commits intohome-assistant:devfrom
shred86:abode_available_property
Apr 3, 2020
Merged

Add Abode entity available property#32923
bdraco merged 5 commits intohome-assistant:devfrom
shred86:abode_available_property

Conversation

@shred86
Copy link
Contributor

@shred86 shred86 commented Mar 18, 2020

Proposed change

Adds the available property to Abode entities. This PR goes with some changes required to Abodepy which I have a pull request open for. The available property is updated based on the status of the web socket connection to Abode's servers.

Also adds the PlatformNotReady exception if the integration setup fails due to a connection issue. I can move this to a separate PR if desired but it's a one line change.

This should not be merged until my Abodepy pull request is merged and I bump the Abodepy version.
The reason I'm making this PR now is to get feedback since the two PRs work together.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

Copy link
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

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

Please see inline comments. Thank you!

@bdraco bdraco added the waiting-for-upstream We're waiting for a change upstream label Mar 19, 2020
@bdraco
Copy link
Member

bdraco commented Mar 19, 2020

CI Failure is unrelated

homeassistant/components/zone/__init__.py:240: error: unused 'type: ignore' comment

Copy link
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

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

Approved, subject to upstream change being merged and a re-run of the CI

@bdraco bdraco self-requested a review March 19, 2020 02:06
@bdraco
Copy link
Member

bdraco commented Mar 19, 2020

@shred86 mentioned more changes are coming. I'll come back for another review pass later.

@shred86
Copy link
Contributor Author

shred86 commented Mar 28, 2020

Finally figured out a fix to a small issue I was having. Just waiting for my PR in abodepy to be merged then I'll make a commit to this PR to bump the abodepy version and it should be good to go.

@shred86 shred86 force-pushed the abode_available_property branch from afd3cf8 to 7de0a87 Compare March 28, 2020 16:32
@bdraco bdraco requested review from bdraco and removed request for bdraco March 31, 2020 22:31
@bdraco
Copy link
Member

bdraco commented Mar 31, 2020

@shred86 Is this still waiting for upstream?

I'm assuming its this one MisterWil/abodepy#67

@shred86
Copy link
Contributor Author

shred86 commented Apr 1, 2020

I am. I sent @MisterWil a message on Discord but haven't heard from him yet. Once that upstream PR is merged, this should be good to go.

@shred86
Copy link
Contributor Author

shred86 commented Apr 3, 2020

PR was merged on abodepy. Just made a commit to bump the abodepy version.

@shred86 shred86 force-pushed the abode_available_property branch from 6292b2c to c415cf3 Compare April 3, 2020 15:15
Copy link
Member

@bdraco bdraco left a comment

Choose a reason for hiding this comment

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

LGTM

@bdraco bdraco removed the waiting-for-upstream We're waiting for a change upstream label Apr 3, 2020
@bdraco bdraco merged commit b8e9e3a into home-assistant:dev Apr 3, 2020

async def async_added_to_hass(self):
"""Subscribe to Abode connection status updates."""
self.hass.async_add_job(
Copy link
Member

Choose a reason for hiding this comment

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

hass.async_add_job is legacy for general use. Either use hass.async_create_task to schedule coroutines or hass.async_add_executor_job to schedule functions that need to run in the executor thread pool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fix this in a new PR, thanks!

@lock lock bot locked and limited conversation to collaborators Apr 4, 2020
@shred86 shred86 deleted the abode_available_property branch May 4, 2020 01:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants