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

Add UniFi Protect media_player #62895

Merged
merged 6 commits into from
Dec 29, 2021
Merged

Add UniFi Protect media_player #62895

merged 6 commits into from
Dec 29, 2021

Conversation

AngellusMortis
Copy link
Contributor

@AngellusMortis AngellusMortis commented Dec 27, 2021

Proposed change

Second PR for migrating HACS integration for UniFi Protect. Adds the media_player platforms.

Previous PRs:

Following the advice of @bdraco, these PRs will be broken up into smaller PRs to make them easier to review and merge:

  • Initial - config flow, data service + camera platform
  • One per entity platform
  • One for remaining services

pyunifiprotect changes: https://github.com/briis/pyunifiprotect/compare/v1.4.4..v1.4.7

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: TBA (waiting for first docs PR to be merged first)

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.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

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

To help with the load of incoming pull requests:

@probot-home-assistant
Copy link

Hey there @briis, @bdraco, mind taking a look at this pull request as it has been labeled with an integration (unifiprotect) you are listed as a code owner for? Thanks!
(message by CodeOwnersMention)

@epenet
Copy link
Contributor

epenet commented Dec 27, 2021

Can you add just one platform at a time (either button OR media_player)?

@AngellusMortis

This comment was marked as abuse.

@AngellusMortis AngellusMortis changed the title UniFi Protect: Adds button and media_player platforms UniFi Protect: Adds media_player platform Dec 27, 2021
@epenet
Copy link
Contributor

epenet commented Dec 27, 2021

You should really also move the changes to the camera tests to another PR if you can - though I am not sure if you can do so without creating a conflict.

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 comments above

@MartinHjelmare MartinHjelmare changed the title UniFi Protect: Adds media_player platform Add UniFi Protect media_player Dec 28, 2021
@bdraco
Copy link
Member

bdraco commented Dec 29, 2021

When you bump a dep in a PR it’s required to add a link to the change log of the dep

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.

Thanks 👍

@bdraco bdraco merged commit 490d76e into home-assistant:dev Dec 29, 2021
@bdraco bdraco deleted the ufp-part-2 branch December 29, 2021 04:36
@bdraco
Copy link
Member

bdraco commented Dec 29, 2021

While home-assistant/home-assistant.io#20851 LGTM, I'm keeping track of which platforms need to be added since I want to give other reviews a chance to respond before merging.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 30, 2021
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.

5 participants