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

[homeassistant] Native number entity import and control #6455

Merged
merged 21 commits into from
Aug 14, 2024

Conversation

landonr
Copy link
Contributor

@landonr landonr commented Mar 31, 2024

What does this implement/fix?

I'm adding the number component which I tested using a graphical display menu config

Home Assistant Component

  • next_api_publish_: prevents publishing the next value to the API, used to prevent publishing duplicate values back to the api when the entity changes from the api
  • can_update_from_api: updates from the api are ignored for a second after the device publishes to the API, this is used so the updates don't recursively continue
  • ignore_api_updates_with_seconds: ignore updates before publishing to update can_update_from_api

all of this is used in combination to keep the device's entity in sync with the API as close as possible. This works for most devices like switches, numbers, covers, but it can be a bit buggy for more complicated entities like Lights.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#3942

Test Environment

  • ESP32
  • ESP32 IDF
  • ESP8266
  • RP2040
  • BK72xx
  • RTL87xx

Example entry for config.yaml:

number:
  - platform: homeassistant
    id: my_number
    name: "My Number"
    entity_id: "number.bedroom_bass"
    on_value:
      then:
        lambda: 'ESP_LOGI("number", "value: %f", x);'

graphical_display_menu:
  id: my_graphical_display_menu
...
  items:
    - type: number
      text: 'My Number'
      format: '%.2f'
      number: my_number

Checklist:

  • The code change is tested and works locally.
  • Tests have been added to verify that the new code works (under tests/ folder).

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

@probot-esphome
Copy link

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

@probot-esphome
Copy link

Hey there @landonr,
Thanks for submitting this pull request! Can you add yourself as a codeowner for this integration? This way we can notify you if a bug report for this integration is reported.
In __init__.py of the integration, please add:

CODEOWNERS = ["@landonr"]

And run script/build_codeowners.py

(message by NeedsCodeownersLabel)

@codecov-commenter
Copy link

codecov-commenter commented Mar 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 53.94%. Comparing base (4d8b5ed) to head (8b5cda2).
Report is 1122 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #6455      +/-   ##
==========================================
+ Coverage   53.70%   53.94%   +0.23%     
==========================================
  Files          50       50              
  Lines        9408     9655     +247     
  Branches     1654     1705      +51     
==========================================
+ Hits         5053     5208     +155     
- Misses       4056     4124      +68     
- Partials      299      323      +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nagyrobi
Copy link
Member

I think that the attributes like min_value, max_value and step should be automatically retrieved from HA, and not re-configured here. Someone may configure them differently on the sides which will likely cause malfunctions.

@landonr
Copy link
Contributor Author

landonr commented Jun 12, 2024

I think that the attributes like min_value, max_value and step should be automatically retrieved from HA, and not re-configured here. Someone may configure them differently on the sides which will likely cause malfunctions.

I think I agree, I'll make the changes and see how it works. thanks!

@landonr
Copy link
Contributor Author

landonr commented Jun 14, 2024

I think that the attributes like min_value, max_value and step should be automatically retrieved from HA, and not re-configured here. Someone may configure them differently on the sides which will likely cause malfunctions.

works great, nice suggestion thanks

@landonr
Copy link
Contributor Author

landonr commented Jul 1, 2024

@nagyrobi can you take another look

@landonr landonr changed the title Home Assisstant Number which can read and write to API Home Assistant Number which can read and write to API Jul 2, 2024
@jesserockz
Copy link
Member

jesserockz commented Aug 13, 2024

I removed all of the timeout stuff and also added a simple value == state check in the state callback to not do anything if it is the same state.

We can add more protections back in later if needed, but in my basic number testing, it was fine.

@jesserockz jesserockz changed the title Home Assistant Number which can read and write to API [homeassistant] Native number entity import and control Aug 13, 2024
@jesserockz jesserockz marked this pull request as ready for review August 13, 2024 20:59
@probot-esphome
Copy link

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

jesserockz
jesserockz previously approved these changes Aug 13, 2024
@jesserockz jesserockz merged commit a5fdcb3 into esphome:dev Aug 14, 2024
2 checks passed
@jesserockz jesserockz mentioned this pull request Aug 14, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 16, 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.

5 participants