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 Sky remote integration #124507

Merged
merged 20 commits into from
Nov 13, 2024
Merged

Add Sky remote integration #124507

merged 20 commits into from
Nov 13, 2024

Conversation

dunnmj
Copy link
Contributor

@dunnmj dunnmj commented Aug 23, 2024

Proposed change

Adds the ability to send commands to a networked skyHD or skyq box.

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)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

Link to library: https://pypi.org/project/skyboxremote/
Link to brand PR: home-assistant/brands#6023

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
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.

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

To help with the load of incoming pull requests:

Copy link

@home-assistant home-assistant bot left a comment

Choose a reason for hiding this comment

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

Hi @dunnmj

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@home-assistant
Copy link

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

@home-assistant home-assistant bot marked this pull request as draft August 23, 2024 19:24
@dunnmj dunnmj marked this pull request as ready for review August 25, 2024 18:28
Copy link
Member

@joostlek joostlek left a comment

Choose a reason for hiding this comment

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

Hello 👋🏻,

New integrations have to ue both a config flow and have that config flow 100% unit tested before being reviewed to be added to core. Please add the config flow.

@home-assistant home-assistant bot marked this pull request as draft August 26, 2024 11:50
@dunnmj dunnmj marked this pull request as ready for review August 31, 2024 22:04
@home-assistant home-assistant bot requested a review from joostlek August 31, 2024 22:04
homeassistant/components/sky_remote/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/sky_remote/config_flow.py Outdated Show resolved Hide resolved
homeassistant/components/sky_remote/const.py Outdated Show resolved Hide resolved
homeassistant/components/sky_remote/config_flow.py Outdated Show resolved Hide resolved
def send_command(self, command: Iterable[str], **kwargs: Any) -> None:
"""Send a list of commands to the device."""
try:
self._remote.send_keys(command)
Copy link
Member

Choose a reason for hiding this comment

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

What can an user send with this integration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Commands that can be sent are listed in the docs for the integration - https://github.com/home-assistant/home-assistant.io/pull/34419/files

@home-assistant home-assistant bot marked this pull request as draft September 3, 2024 11:57
@dunnmj
Copy link
Contributor Author

dunnmj commented Sep 3, 2024

@joostlek Would you like all these commits squashed?

homeassistant/components/sky_remote/manifest.json Outdated Show resolved Hide resolved
homeassistant/components/sky_remote/manifest.json Outdated Show resolved Hide resolved
homeassistant/components/sky_remote/config_flow.py Outdated Show resolved Hide resolved
homeassistant/components/sky_remote/config_flow.py Outdated Show resolved Hide resolved
homeassistant/components/sky_remote/strings.json Outdated Show resolved Hide resolved
@autinerd
Copy link
Contributor

autinerd commented Sep 7, 2024

@joostlek Would you like all these commits squashed?

No need to squash, it will be squashed on merge.

@dunnmj
Copy link
Contributor Author

dunnmj commented Oct 8, 2024

Please also add a link to your library and to the documentation pull request in the PR description.

Added link to library
Documentation PR already linked

@dunnmj dunnmj marked this pull request as ready for review October 8, 2024 21:20
@home-assistant home-assistant bot requested review from joostlek and autinerd October 8, 2024 21:20
homeassistant/components/sky_remote/config_flow.py Outdated Show resolved Hide resolved
homeassistant/components/sky_remote/config_flow.py Outdated Show resolved Hide resolved
tests/components/sky_remote/conftest.py Outdated Show resolved Hide resolved
tests/components/sky_remote/conftest.py Outdated Show resolved Hide resolved
tests/components/sky_remote/conftest.py Outdated Show resolved Hide resolved
tests/components/sky_remote/test_init.py Outdated Show resolved Hide resolved
tests/components/sky_remote/test_init.py Outdated Show resolved Hide resolved
tests/components/sky_remote/test_remote.py Outdated Show resolved Hide resolved
@home-assistant home-assistant bot marked this pull request as draft October 25, 2024 13:39
@joostlek
Copy link
Member

Also, the brands PR is missing

@dunnmj dunnmj force-pushed the sky_remote branch 3 times, most recently from 56ca901 to 3d46050 Compare October 26, 2024 22:03
@dunnmj dunnmj marked this pull request as ready for review October 26, 2024 22:41
@home-assistant home-assistant bot requested a review from joostlek October 26, 2024 22:41
@joostlek
Copy link
Member

What's the difference between sky_hub and this? Are they the same device?

@dunnmj
Copy link
Contributor Author

dunnmj commented Oct 30, 2024

What's the difference between sky_hub and this? Are they the same device?

Both are from the company Sky.
The Sky Hub is a broadband router, that integration allows presence info by looking at devices connected to it's wifi network.

This is a remote that allows control of Sky+HD and SkyQ satellite tv receivers over network.

As both are made by sky they use the same brand images.

@joostlek
Copy link
Member

If both @dunnmj and @saty9 can send over their discord username (if you have one) that'd be great :)

@saty9
Copy link
Contributor

saty9 commented Nov 10, 2024

im also saty9 on discord

@dunnmj
Copy link
Contributor Author

dunnmj commented Nov 10, 2024

If both @dunnmj and @saty9 can send over their discord username (if you have one) that'd be great :)

Hi

I'm thew_mat on discord :)

@joostlek joostlek merged commit 72b976f into home-assistant:dev Nov 13, 2024
65 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Nov 14, 2024
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