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 new number entity integration #42735

Merged
merged 23 commits into from
Dec 2, 2020
Merged

Conversation

Shulyaka
Copy link
Contributor

@Shulyaka Shulyaka commented Nov 1, 2020

Breaking change

N/A

Proposed change

Add new integration for an analog switch, a new entity type that other platforms may use to get a value from user.
We already have a number of integrations that allow the user to set a certain value within a range, such as light, climate, humidifier, input_number, and media_player. However we lack the generic integration that other integrations or platforms could use for a value input, along with sensor for value output, binary_sensor for boolean output, and switch for boolean input.
This PR adds this support. It would allow to create a separate entity to control a volume for something that is not a media player (i.e. an alarm clock), or to control volume of more than 1 channel for a media player that needs it (multichannel or equalizer may be?), or to control the brightness of indicator LEDs of an air purifier device, or to set PWM level for a ZHA XBee device, all without complicated setup that involved manual input_numbers, automations, and services, or using entities that not exactly fit the actual device.
Other examples of the possible usage include:

  • rpi_gpio and arduino PWM output
  • Transmission or rtorrent download speed control
  • Modbus write register representation
  • Other "protocol" level integrations

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:

# Example configuration.yaml

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:

@probot-home-assistant
Copy link

Hey there @home-assistant/core, mind taking a look at this pull request as its been labeled with an integration (demo) you are listed as a codeowner for? Thanks!
(message by CodeOwnersMention)

@kukulich
Copy link
Contributor

kukulich commented Nov 2, 2020

Would not be better to modify input_* integrations so inputs can be added via other integrations?

https://community.home-assistant.io/t/allow-to-connect-input-number-input-select-input-boolean-etc-to-integrations/219550

@Shulyaka
Copy link
Contributor Author

Shulyaka commented Nov 2, 2020

No. I already tried that in the past. Please see home-assistant/architecture#328

@kukulich
Copy link
Contributor

kukulich commented Nov 2, 2020

No. I already tried that in the past. Please see home-assistant/architecture#328

Ok :(

Btw I don't know better name but I'm not sure if analog_switch is right. It can be used for digital devices as well. And it looks it's not "switch". Yeah, the best name would be input_number 😄

Copy link
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

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

This PR adds a new entity component.

New entity components need to be discussed using an architecture issue first. Please open an architecture issue.

Dev automation moved this from Needs review to Review in progress Nov 2, 2020
@Shulyaka
Copy link
Contributor Author

Shulyaka commented Nov 2, 2020

Done: home-assistant/architecture#453

@Shulyaka
Copy link
Contributor Author

Shulyaka commented Nov 2, 2020

Will update the PR today according to the architecture issues comments.

@Shulyaka Shulyaka changed the title Add new analog_switch entity integration Add new number entity integration Nov 2, 2020
@Shulyaka
Copy link
Contributor Author

Shulyaka commented Nov 2, 2020

Who has the power to fix the "integration: analog_switch" label of this PR?

@Shulyaka
Copy link
Contributor Author

How do I fix this mypy error? If I satisfy its requirements to define the return type for async_setup_entry and async_unload_entry as bool, it starts complaining that the functions can return Any. And I don't understand how other integrations pass this requirement...

@JeffLIrion
Copy link
Contributor

How do I fix this mypy error? If I satisfy its requirements to define the return type for async_setup_entry and async_unload_entry as bool, it starts complaining that the functions can return Any. And I don't understand how other integrations pass this requirement...

Probably like this:

# mypy: allow-untyped-defs, no-check-untyped-defs

@Shulyaka
Copy link
Contributor Author

Shulyaka commented Nov 15, 2020

Probably like this:

# mypy: allow-untyped-defs, no-check-untyped-defs

Already tried this: f1f6617

Co-authored-by: Franck Nijhof <frenck@frenck.nl>
Co-authored-by: Franck Nijhof <frenck@frenck.nl>
@Shulyaka
Copy link
Contributor Author

It works! Thanks @frenck!

Dev automation moved this from Review in progress to Reviewer approved Nov 16, 2020
Copy link
Member

@frenck frenck left a comment

Choose a reason for hiding this comment

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

👍 Looks good.

I would like to see a second opinion before merging though.

@frenck frenck added the second-opinion-wanted Add this label when a reviewer needs a second opinion from another member. label Nov 16, 2020
@project-bot project-bot bot moved this from Reviewer approved to Second opinion wanted in Dev Nov 16, 2020
@Jc2k Jc2k mentioned this pull request Nov 23, 2020
21 tasks
@cgarwood cgarwood added this to Incoming in New Integrations via automation Nov 25, 2020
Dev automation moved this from Second opinion wanted to Reviewer approved Dec 2, 2020
New Integrations automation moved this from Incoming to Reviewer approved Dec 2, 2020
@balloob balloob merged commit f744f7c into home-assistant:dev Dec 2, 2020
Dev automation moved this from Reviewer approved to Done Dec 2, 2020
New Integrations automation moved this from Reviewer approved to Done Dec 2, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Dec 3, 2020
@Shulyaka Shulyaka deleted the analog_switch branch June 8, 2021 13:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Dev
  
Done
Development

Successfully merging this pull request may close these issues.

Add new Number entity component
6 participants