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

Improve RemoteEntity class #40605

Merged
merged 2 commits into from
Oct 18, 2020
Merged

Conversation

felipediel
Copy link
Contributor

@felipediel felipediel commented Sep 26, 2020

Proposed change

This PR is based on this discussion. We are improving the RemoteEntity. These are the proposed changes:

  • Add the command_type option to remote.learn_command.
  • Create remote.delete_command service.

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

Example entry for configuration.yaml:

Learn RF command

# Example configuration.yaml
script:
  learn_car_lock:
    sequence:
      - service: remote.learn_command
        data:
          entity_id: remote.garage
          device: car
          command: unlock
          command_type: rf

Delete a command

# Example configuration.yaml
script:
  delete_car_lock:
    sequence:
      - service: remote.delete_command
        data:
          entity_id: remote.garage
          device: car
          command: unlock

Delete a list of commands

# Example configuration.yaml
script:
  delete_tv_commands:
    sequence:
      - service: remote.delete_command
        data:
          entity_id: remote.bedroom
          device: television
          command:
            - power
            - volume up
            - volume down
            - channel up
            - channel down

Additional information

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

To help with the load of incoming pull requests:

@springstan springstan changed the title Improve RemoteEntity Improve RemoteEntity class Sep 26, 2020
@bdraco
Copy link
Member

bdraco commented Sep 26, 2020

It appears the discussion is still ongoing so this isn't mergeable yet. I'm going to mark this as a draft and it can be marked ready for review when the discussion is completed in the architecture issue.

@felipediel
Copy link
Contributor Author

I think everyone agreed. There's not much to overthink about this, we covered everything in the issue. So I am reopening the PR. If anyone has a better idea, please join the discussion and I will mark this as a draft.

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
Copy link
Member

bdraco commented Oct 13, 2020

I'm relatively sure this isn't a breaking change because alternative was left alone.

@haglen
Copy link

haglen commented Oct 18, 2020

This is great! My RM4 Pro works just fine with the new integration but cant learn the RF commands and it looks like this will fix the issue. Sorry for my ignorance, but how do I know what release this fix will be in?

@bdraco
Copy link
Member

bdraco commented Oct 18, 2020

Seems like we have consensus or at least lack of objection here so I'll merge this.

@bdraco bdraco merged commit d96a174 into home-assistant:dev Oct 18, 2020
@felipediel
Copy link
Contributor Author

Thank you!

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.

Support learning different command types with remote
5 participants