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 ability to lock to set mode #5924

Merged
merged 1 commit into from
Dec 20, 2023
Merged

Add ability to lock to set mode #5924

merged 1 commit into from
Dec 20, 2023

Conversation

ysmilda
Copy link
Contributor

@ysmilda ysmilda commented Dec 14, 2023

What does this implement/fix?

It allows locking the read out of the HLW8012 component to a single mode (e.g current or voltage). This is useful when you're not interested in one of the two values and want to continuously read back the other.

This allows the change_mode_every parameter to be set to "never" which locks the mode to the set initial_mode.

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#3473

Test Environment

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

Example entry for config.yaml:

With the below config the mode will be locked to VOLTAGE.

# Example config.yaml
  - platform: hlw8012
    change_mode_every: "never"
    initial_mode: VOLTAGE

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:

Copy link
Member

@jesserockz jesserockz left a comment

Choose a reason for hiding this comment

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

This would be better to change change_every to allow never similar to update_intervals do

@ysmilda
Copy link
Contributor Author

ysmilda commented Dec 14, 2023

I like that proposal, but I'm not sure how to implement that. The update_intervals take a time string, this is than checked to be never or parsed to an uint32_t in milliseconds. The change_every already takes an uint32_t and changing that to a string would break everyone's config.

Ideally change_every would take either a string or an int but I'm unsure how that could be implemented.

@jesserockz
Copy link
Member

You can use something like cv.Any(..., "never") to allow the current time and the hardcoded string never. And then convert never into 0 and check for 0 in the cpp code since the minimum is currently 1

@probot-esphome probot-esphome bot removed the core label Dec 14, 2023
@ysmilda
Copy link
Contributor Author

ysmilda commented Dec 14, 2023

Thanks for the insight, I've changed change_every to accept "never" and that locks the mode to the one set in initial_mode.

Copy link
Contributor

@kroimon kroimon left a comment

Choose a reason for hiding this comment

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

LGTM

@kbx81 kbx81 merged commit 23cedda into esphome:dev Dec 20, 2023
49 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2023
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.

None yet

4 participants