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 preview to Threshold config & option flow #111043

Closed
wants to merge 47 commits into from

Conversation

jpbede
Copy link
Member

@jpbede jpbede commented Feb 20, 2024

Proposed change

Add the preview function to the Threshold helper config flow and option flow.

Context: The use of the upper, lower or both values together and their effects on the binary sensor are sometimes somewhat confusing (#110904) for users. With the preview, a user would see the result of their changes directly.

Frontend PR: home-assistant/frontend#19845

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

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

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:

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.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

@gjohansson-ST
Copy link
Member

I think perhaps add a printscreen to the PR description to visualize how it will look 👍

@jpbede
Copy link
Member Author

jpbede commented Feb 21, 2024

I think perhaps add a printscreen to the PR description to visualize how it will look 👍

Done 🙂

@frenck frenck added smash Indicator this PR is close to finish for merging or closing awaiting-frontend and removed awaiting-frontend labels Mar 13, 2024
Copy link
Member

@gjohansson-ST gjohansson-ST left a comment

Choose a reason for hiding this comment

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

Conflicts needs to be addressed

@home-assistant home-assistant bot marked this pull request as draft March 15, 2024 22:15
@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.

@jpbede jpbede force-pushed the threshold-config-flow-preview branch from c873458 to 3b5fccd Compare March 16, 2024 17:31
@jpbede jpbede marked this pull request as ready for review March 16, 2024 17:31
@home-assistant home-assistant bot requested a review from gjohansson-ST March 16, 2024 17:31
Comment on lines 286 to 292
# guard against the case where the thresholds are not set
if not hasattr(self, "_threshold_lower") and not hasattr(
self, "_threshold_upper"
):
self._state_position = POSITION_UNKNOWN
self._state = None
return
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this case added? If it's only needed for preview, it should be possible to handle it in async_start_preview, if it's a bug fix it should be moved to a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, that's just the case for preview. I moved this to async_start_preview. See the new PR as I screwed up the rebase

@home-assistant home-assistant bot marked this pull request as draft May 8, 2024 15:04
pantonvich and others added 5 commits May 8, 2024 17:19
…nt#110475)

* Update sensor.py for Hourly Rain Rates mm and in

hourly_rain_rate from integration ecowitt has state class total_increasing, but its state is not strictly increasing

* Update sensor.py format
* Add `open` state to LockEntity

* Add tests

* Fixes

* Fix tests

* strings and icons

* Adjust demo open lock

* Fix lock and tests

* fix import

* Fix strings

* mute ruff

* Change sequence

* Sequence2

* Group on states

* Fix ruff

* Fix tests

* Add more test cases

* Sorting
…sistant#114599)

* fix nibe_heatpump climate for models without cooling support

* add test for set temperature with no cooling support

* fixup use self._coil_setpoint_cool None

* fixup add new test to explicitly test unsupported cooling
autinerd and others added 26 commits May 8, 2024 23:54
* Add better testing to vacuum platform

* remove state strings

* some of the MR comments

* move MockVacuum

* remove manifest extra

* fix linting

* fix other linting

* Fix create entity calls

* Format

* remove create_entity

* change to match notify

---------

Co-authored-by: Martin Hjelmare <[email protected]>
…ant#117095)

Avoid the gather call when there are no loaded config entries
* Catch AuthException in Husqvarna Automower

* don't use getattr

* raise ConfigEntryAuthFailed
…#117171)

Fix ClimateService to rise ServiceValidationError for stack free logs
…sistant#109043)

* Update device info when name changes

* Entities now report themselves as being unavailable when the MotionMount is disconnected

* Don't update device_info when name changes

* Use `device_entry` property to update device name

* Assert device is available

Co-authored-by: Erik Montnemery <[email protected]>

* Add missing import

---------

Co-authored-by: Erik Montnemery <[email protected]>
* Add standard deviation calculation to group

* Add missing bits

---------

Co-authored-by: G Johansson <[email protected]>
…e-assistant-core into threshold-config-flow-preview
@jpbede
Copy link
Member Author

jpbede commented May 10, 2024

Sry everyone, I screwed up the rebase

@github-actions github-actions bot locked and limited conversation to collaborators May 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.