Add Config Flow to Obihai #88627

ejpenney · 2023-02-22T18:46:59Z

Breaking change

N/A

Proposed change

Adds config_flow to the Obihai integration, deprecates YAML configuration.

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: N/A
This PR is related to issue: N/A
Link to documentation pull request: Add Config Flow to Obihai home-assistant.io#26354

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:

Documentation added/updated for [www.home-assistant.io(http://www.home-assistant.io][docs-repository/)]

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:

I have reviewed two other open pull requests in this repository.

home-assistant · 2023-02-22T18:47:06Z

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

Code owner commands

Code owners of obihai can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign obihai Removes the current integration label and assignees on the pull request, add the integration domain after the command.

dshokouhi · 2023-02-22T18:55:15Z

This great! Please also consider adding yourself to the codeowners for this integration as I have not done any updates to this in a very long time 😬

ejpenney · 2023-02-22T19:01:20Z

Ugh, bit by pre-commit. Running it offline now, another commit coming momentarily...

ejpenney · 2023-02-22T19:16:02Z

I don't know what the heck is going on. These tests are passing for me offline.

ejpenney · 2023-02-22T19:55:57Z

@dshokouhi, looks like I got the pre-commit working. Added myself as code owner. Let me know if you think anything else needs done!

epenet

Please split into smaller pr

one for code owners
one for reboot service
one for ConfigFlow (without dhcp)
one follow up for dhcp

home-assistant · 2023-02-22T20:20:19Z

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.

.coveragerc

homeassistant/components/obihai/__init__.py

homeassistant/components/obihai/config_flow.py

homeassistant/components/obihai/obihai_api.py

epenet

Also please ensure that you have 100% test coverage on config flow.

homeassistant/components/obihai/sensor.py

homeassistant/components/obihai/config_flow.py

homeassistant/components/obihai/strings.json

ejpenney · 2023-02-22T23:51:07Z

@epenet, I believe I have resolved (or otherwise addressed) all your concerns. When this gets submitted I'll submit a few other PRs. The other config_flow changes might take a bit since they don't have tests...

Let me know if it's looking good.

homeassistant/components/obihai/config_flow.py

homeassistant/components/obihai/const.py

homeassistant/components/obihai/obihai_api.py

homeassistant/components/obihai/strings.json

homeassistant/components/obihai/sensor.py

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>

homeassistant/components/obihai/strings.json

homeassistant/components/obihai/config_flow.py

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>

epenet

LGTM 👍

MartinHjelmare

Please address the comments in a new PR. Thanks!

MartinHjelmare · 2023-02-27T16:38:30Z

homeassistant/components/obihai/config_flow.py

+    async def async_step_import(self, config: dict[str, Any]) -> FlowResult:
+        """Handle a flow initialized by importing a config."""
+        self._async_abort_entries_match({CONF_HOST: config[CONF_HOST]})
+        return self.async_create_entry(


We should test the connection to the device with the config before creating the config entry, as we do in the user step. Abort if it doesn't work.

Would you have objections to just using LOGGER to drop a warning in system log if YAML import failed? I suppose we could create an "issue", but that seems overkill IMHO.

Strike that, I'm just going to use async_abort and hand it cannot_connect.

MartinHjelmare · 2023-02-27T16:40:13Z

homeassistant/components/obihai/sensor.py

+        "manual_migration",
+        breaks_in_ha_version="2023.6.0",
+        is_fixable=False,
+        severity=issue_registry.IssueSeverity.ERROR,


Please use warning severity.

MartinHjelmare · 2023-02-27T16:44:50Z

homeassistant/components/obihai/sensor.py

@@ -148,6 +157,10 @@ def icon(self):

    def update(self) -> None:
        """Update the sensor."""
+        if not self._pyobihai.check_account():


What does False mean here? If we can't connect we should set the available property to False in the entity. If credentials are bad we should start a reauth flow.

https://developers.home-assistant.io/docs/config_entries_config_flow_handler#reauthentication

The available property will return False if self._state is None which is what we've done here. I'll take a look at the re-auth feedback, though I think that should be a PR in-and-of-itself as it would require new classes and tests.

Setting the native value to None means that the state is unknown. This is different than state unavailable. It would be better not to couple these things to avoid confusion when reading the code.

MartinHjelmare · 2023-02-27T16:45:18Z

homeassistant/components/obihai/strings.json

+  },
+  "issues": {
+    "manual_migration": {
+      "title": "Manual migration required for Obihai",


What manual migration is required?

Can you be more specific about what's confusing here? This is C+P from other components that require migration from YAML. This is just a title. The description is... more descriptive of the process. Would you like the title to describe how to migrate?

The title is incorrect. There is no manual migration required (it has already migrated). It should state the YAML configuration is deprecated and should be removed.

home-assistant bot added cla-signed config-flow This integration migrates to the UI by adding a config flow deprecation Indicates a breaking change to happen in the future integration: obihai new-feature labels Feb 22, 2023

home-assistant bot assigned dshokouhi Feb 22, 2023

home-assistant bot added the Quality Scale: No score label Feb 22, 2023

ejpenney mentioned this pull request Feb 22, 2023

Obihai: Add Config Flow home-assistant/home-assistant.io#26339

Closed

8 tasks

ejpenney force-pushed the ejpenney_obihai_enhancements branch from c3f2619 to 1c92605 Compare February 22, 2023 19:11

epenet requested changes Feb 22, 2023

View reviewed changes

home-assistant bot marked this pull request as draft February 22, 2023 20:20

ejpenney force-pushed the ejpenney_obihai_enhancements branch from eb6f859 to 0620082 Compare February 22, 2023 21:02

ejpenney changed the title ~~Obihai: Add Config Flow and reboot service~~ Obihai: Add Config Flow Feb 22, 2023

ejpenney marked this pull request as ready for review February 22, 2023 21:09

home-assistant bot requested a review from epenet February 22, 2023 21:09

epenet requested changes Feb 22, 2023

View reviewed changes

home-assistant bot marked this pull request as draft February 22, 2023 21:14

epenet requested changes Feb 22, 2023

View reviewed changes

epenet reviewed Feb 22, 2023

View reviewed changes

homeassistant/components/obihai/sensor.py Outdated Show resolved Hide resolved

homeassistant/components/obihai/config_flow.py Outdated Show resolved Hide resolved

homeassistant/components/obihai/strings.json Outdated Show resolved Hide resolved

ejpenney marked this pull request as ready for review February 22, 2023 23:45

home-assistant bot requested a review from epenet February 22, 2023 23:45

epenet requested changes Feb 23, 2023

View reviewed changes

home-assistant bot marked this pull request as draft February 23, 2023 10:33

ejpenney and others added 13 commits February 27, 2023 07:25

Update homeassistant/components/obihai/config_flow.py

6a0e939

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>

Update homeassistant/components/obihai/config_flow.py

e97f68f

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>

Update homeassistant/components/obihai/config_flow.py

167b622

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>

Cleanup

35e36c8

Update homeassistant/components/obihai/__init__.py

fdb5971

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>

PR feedback

eed7e76

Backout today's changes: Fix mypy error

4eca21b

Update tests/components/obihai/test_config_flow.py

646f2e8

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>

Update tests/components/obihai/test_config_flow.py

96549b4

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>

Update tests/components/obihai/test_config_flow.py

b4248af

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>

Update tests/components/obihai/test_config_flow.py

f1c97d0

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>

Don't plan ahead

83602a3

PR feedback

5e72f24

ejpenney force-pushed the ejpenney_obihai_enhancements branch from 8763c3f to 5e72f24 Compare February 27, 2023 07:25

ejpenney marked this pull request as ready for review February 27, 2023 07:26

home-assistant bot requested a review from epenet February 27, 2023 07:26

epenet reviewed Feb 27, 2023

View reviewed changes

homeassistant/components/obihai/strings.json Outdated Show resolved Hide resolved

epenet requested changes Feb 27, 2023

View reviewed changes

homeassistant/components/obihai/config_flow.py Outdated Show resolved Hide resolved

home-assistant bot marked this pull request as draft February 27, 2023 08:10

ejpenney and others added 2 commits February 27, 2023 07:26

Update homeassistant/components/obihai/config_flow.py

9a5704d

Co-authored-by: epenet <6771947+epenet@users.noreply.github.com>

Cleanup strings

2bfafe3

ejpenney marked this pull request as ready for review February 27, 2023 15:29

home-assistant bot requested a review from epenet February 27, 2023 15:30

epenet approved these changes Feb 27, 2023

View reviewed changes

epenet merged commit 0e8d28d into home-assistant:dev Feb 27, 2023

MartinHjelmare reviewed Feb 27, 2023

View reviewed changes

ejpenney mentioned this pull request Feb 27, 2023

Obihai config flow fixes #88853

Merged

19 tasks

github-actions bot locked and limited conversation to collaborators Feb 28, 2023

ejpenney deleted the ejpenney_obihai_enhancements branch April 3, 2023 19:54

ejpenney restored the ejpenney_obihai_enhancements branch April 6, 2023 18:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Config Flow to Obihai #88627

Add Config Flow to Obihai #88627

ejpenney commented Feb 22, 2023 •

edited

Loading

home-assistant bot commented Feb 22, 2023

dshokouhi commented Feb 22, 2023

ejpenney commented Feb 22, 2023

ejpenney commented Feb 22, 2023

ejpenney commented Feb 22, 2023

epenet left a comment

home-assistant bot commented Feb 22, 2023

epenet left a comment

ejpenney commented Feb 22, 2023 •

edited

Loading

epenet left a comment

MartinHjelmare left a comment

MartinHjelmare Feb 27, 2023

ejpenney Feb 27, 2023

ejpenney Feb 27, 2023

MartinHjelmare Feb 27, 2023

MartinHjelmare Feb 27, 2023

ejpenney Feb 27, 2023 •

edited

Loading

MartinHjelmare Feb 27, 2023

MartinHjelmare Feb 27, 2023

ejpenney Feb 27, 2023

frenck Feb 27, 2023

Add Config Flow to Obihai #88627

Add Config Flow to Obihai #88627

Conversation

ejpenney commented Feb 22, 2023 • edited Loading

Breaking change

Proposed change

Type of change

Additional information

Checklist

home-assistant bot commented Feb 22, 2023

dshokouhi commented Feb 22, 2023

ejpenney commented Feb 22, 2023

ejpenney commented Feb 22, 2023

ejpenney commented Feb 22, 2023

epenet left a comment

Choose a reason for hiding this comment

home-assistant bot commented Feb 22, 2023

epenet left a comment

Choose a reason for hiding this comment

ejpenney commented Feb 22, 2023 • edited Loading

epenet left a comment

Choose a reason for hiding this comment

MartinHjelmare left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ejpenney Feb 27, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ejpenney commented Feb 22, 2023 •

edited

Loading

ejpenney commented Feb 22, 2023 •

edited

Loading

ejpenney Feb 27, 2023 •

edited

Loading