Skip to content

Add Abode camera on and off support#35164

Merged
bdraco merged 5 commits intohome-assistant:devfrom
shred86:abode_camera_control
Jul 26, 2020
Merged

Add Abode camera on and off support#35164
bdraco merged 5 commits intohome-assistant:devfrom
shred86:abode_camera_control

Conversation

@shred86
Copy link
Contributor

@shred86 shred86 commented May 4, 2020

Proposed change

Add support to turn off Abode cameras using the camera.turn_on and camera.turn_off service. This PR requires an upstream PR merge (abodepy). I will make another commit to bump the abodepy version once the upstream PR is merged.

Update: Upstream PR has been merged. This PR is ready to be merged once reviewed.

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: N/A
  • This PR is related to issue: N/A
  • Link to documentation pull request: #13324

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

@shred86
Copy link
Contributor Author

shred86 commented May 7, 2020

Upstream PR has been merged. I also made a PR for documentation updates.

@bdraco bdraco removed the waiting-for-upstream We're waiting for a change upstream label May 25, 2020
@bdraco bdraco assigned bdraco and unassigned bdraco May 25, 2020
@bdraco bdraco self-requested a review May 25, 2020 20:35
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.

Looks like a bit more coverage is needed, and then this should be good to go.
pytest --cov=homeassistant/components/abode/ --cov-report term-missing -- tests/components/abode/test_*.py

----------- coverage: platform linux, python 3.8.1-final-0 -----------
Name                                                    Stmts   Miss  Cover   Missing
-------------------------------------------------------------------------------------
homeassistant/components/abode/__init__.py                176     14    92%   97-105, 121-123, 171-172, 239-254, 311-312, 341, 389
homeassistant/components/abode/alarm_control_panel.py      38      0   100%
homeassistant/components/abode/binary_sensor.py            20      0   100%
homeassistant/components/abode/camera.py                   58     18    69%   60-61, 65-74, 78-83, 95-97, 102
homeassistant/components/abode/config_flow.py              39      0   100%
homeassistant/components/abode/const.py                     5      0   100%
homeassistant/components/abode/cover.py                    18      0   100%
homeassistant/components/abode/light.py                    51      0   100%
homeassistant/components/abode/lock.py                     18      0   100%
homeassistant/components/abode/sensor.py                   45      1    98%   30
homeassistant/components/abode/switch.py                   43      0   100%
-------------------------------------------------------------------------------------
TOTAL                                                     511     33    94%


@stale
Copy link

stale bot commented Jun 26, 2020

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

@stale stale bot added the stale label Jun 26, 2020
@shred86
Copy link
Contributor Author

shred86 commented Jun 26, 2020

Unfortunately this is about all I’m able to do with tests.

@stale stale bot removed the stale label Jun 26, 2020
@bdraco
Copy link
Member

bdraco commented Jun 26, 2020

Should we schedule a state update after the turn on and turn off?

@shred86
Copy link
Contributor Author

shred86 commented Jun 29, 2020

I didn't add it because it won't change anything right now. The camera status is just based on whether it's actually online or offline (i.e. connected to the Abode gateway). This privacy mode just makes it so the live stream is not accessible, but it's not a property that Home Assistant currently uses.

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.

Just needs the conflict resolved

@shred86 shred86 force-pushed the abode_camera_control branch from eb23bc5 to b067bc1 Compare July 26, 2020 17:40
@shred86
Copy link
Contributor Author

shred86 commented Jul 26, 2020

@balloob Just made a commit which bumps the abodepy version and updates this component to reflect the changes in abodepy.

  1. True and False were swapped in camera.py to reflect the changes made to abodepy.
  2. There's a change in abodepy that fixes Abode randomly becomes "unavailable", restored by restart. #38136
  3. Also added a new event type in init.py to reflect changes made to abodepy that fixes this issue.

Sorry for adding onto this PR and if you want me to split off the last one, I can but I figured it's extremely minor and is needed to resolve an issue so might as well just add it here with this abodepy version bump.

Edit: Not sure why hassfest is failing. For some reason the latest manifest wasn't pulled in when I rebased. Just synced the changes by hand and committed the manifest file. Should hopefully fix it.

@bdraco
Copy link
Member

bdraco commented Jul 26, 2020

Looks good

@bdraco bdraco merged commit 36cb818 into home-assistant:dev Jul 26, 2020
@shred86
Copy link
Contributor Author

shred86 commented Jul 31, 2020

@ballob @bdraco Any chance this can be added into a minor release of 0.113? The only reason is bumping the Abodepy version fixes #38136 which unfortunately is causing a lot of issues for Abode users since the Abode servers are having constant issues.

@bdraco
Copy link
Member

bdraco commented Jul 31, 2020

I think its simple enough and there aren't any breaking changes so it should be ok

@bdraco bdraco added this to the 0.113.3 milestone Jul 31, 2020
balloob pushed a commit that referenced this pull request Aug 1, 2020
* Add Abode camera controls

* Add tests for camera turn on and off service

* Bump abodepy version

* Bump abodepy version and updates to reflect changes

* Update manifest
@balloob balloob mentioned this pull request Aug 1, 2020
@shred86 shred86 deleted the abode_camera_control branch November 12, 2020 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants