Skip to content

Add Switcher Runner S11 support#123578

Merged
thecode merged 35 commits into
home-assistant:devfrom
YogevBokobza:switcher-add-s11-support
Sep 20, 2024
Merged

Add Switcher Runner S11 support#123578
thecode merged 35 commits into
home-assistant:devfrom
YogevBokobza:switcher-add-s11-support

Conversation

@YogevBokobza
Copy link
Copy Markdown
Contributor

@YogevBokobza YogevBokobza commented Aug 11, 2024

Breaking change

Proposed change

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

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.

To help with the load of incoming pull requests:

@home-assistant
Copy link
Copy Markdown
Contributor

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

Code owner commands

Code owners of switcher_kis 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 switcher_kis Removes the current integration label and assignees on the pull request, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

@YogevBokobza YogevBokobza changed the title switcher start s11 integration switcher add s11 integration Aug 11, 2024
@YogevBokobza
Copy link
Copy Markdown
Contributor Author

@thecode
Regarding this in-progress PR I have a couple of questions that maybe you can help:

  1. In previous PRs to switcher_kis there was a change of config_flow to this: config_entry_flow.register_discovery_flow(DOMAIN, "Switcher", async_has_devices) I need to revert that to a full setup file right? because right now we going to need auth having CONF_TOKEN and CONF_USERNAME
  2. Please take a look at this, I am trying to use the API call from the aioswithcer library and I am getting that fail. How to fix that?
  3. Regarding this, I don't like passing token as that.. I think after (1) I should get the token from the config_entry right?
  4. Regarding this logic, So in the discovery we found out a device that requires a token. Now it should prompt a form for the user to enter a username(which is email) and token(after he manually goes and fetches it from the Switcher URL), validate it and store the token so when creating each coordinator it will have the token in it.
    You mentioned reauthentication, the documentation is really lacking on this and I could not find an appropriate example in other integration.. Can you assist with that?

@MartinHjelmare MartinHjelmare changed the title switcher add s11 integration Add switcher s11 integration Aug 11, 2024
@thecode
Copy link
Copy Markdown
Member

thecode commented Aug 11, 2024

  1. In previous PRs to switcher_kis there was a change of config_flow to this: config_entry_flow.register_discovery_flow(DOMAIN, "Switcher", async_has_devices) I need to revert that to a full setup file right? because right now we going to need auth having CONF_TOKEN and CONF_USERNAME

You need to implement a full config flow

  1. Please take a look at this, I am trying to use the API call from the aioswithcer library and I am getting that fail. How to fix that?

Unfortunately this was implemented as a blocking call in aioswitcher so you either fix it there (best option would be to use aiohttp) or run it in executor. See Blocking operations with asyncio

  1. Regarding this, I don't like passing token as that.. I think after (1) I should get the token from the config_entry right?

You can access the token from entry.data (entry.data.get(CONF_TOKEN)), no need to pass it to the coordinator

  1. Regarding this logic, So in the discovery we found out a device that requires a token. Now it should prompt a form for the user to enter a username(which is email) and token(after he manually goes and fetches it from the Switcher URL), validate it and store the token so when creating each coordinator it will have the token in it.
    You mentioned reauthentication, the documentation is really lacking on this and I could not find an appropriate example in other integration.. Can you assist with that?

Here is an example for starting a reauth flow:

self.entry.async_start_reauth(self.hass)

You can start the reauth flow when you detect a new device here:


and skip adding the device or generating coordinator for it. After the user will finish the flow the integration will be reloaded and devices will be added again.

@thecode thecode changed the title Add switcher s11 integration Add Switcher Runner S11 support Aug 11, 2024
@YogevBokobza
Copy link
Copy Markdown
Contributor Author

@thecode
Made some progress
(1) Made full config flow (basically revert what it was before the last change of minimize that)
(2) Added CONF_USERNAME and CONF_TOKEN
(3) Added reauth step.. it's kinda buggy right now. Please take a look to see what is going on..
(4) I am skipping creating a coordinator if a token is needed but not provided.
(5) Regarding the validate token function I have no idea how to implement it at the moment..

Please revisit the changes and let me know what you think and how to improve.

Comment thread homeassistant/components/switcher_kis/config_flow.py Outdated
Comment thread homeassistant/components/switcher_kis/config_flow.py Outdated
Comment thread homeassistant/components/switcher_kis/config_flow.py Outdated
Comment thread homeassistant/components/switcher_kis/const.py Outdated
Comment thread homeassistant/components/switcher_kis/cover.py Outdated
Comment thread homeassistant/components/switcher_kis/strings.json Outdated
@YogevBokobza YogevBokobza force-pushed the switcher-add-s11-support branch 2 times, most recently from dc518df to e227b50 Compare August 12, 2024 21:19
@YogevBokobza YogevBokobza requested a review from thecode August 13, 2024 21:03
@YogevBokobza

This comment was marked as outdated.

@YogevBokobza
Copy link
Copy Markdown
Contributor Author

I am quite stuck here.. I don't know if I am doing things right and having trouble with the tests and the coverage..

Comment thread homeassistant/components/switcher_kis/config_flow.py Outdated
Comment thread homeassistant/components/switcher_kis/config_flow.py Outdated
Comment thread homeassistant/components/switcher_kis/const.py Outdated
@YogevBokobza
Copy link
Copy Markdown
Contributor Author

@thecode I will be happy if you can assist here and guide me on how to progress..

Comment thread homeassistant/components/switcher_kis/utils.py Outdated
Comment thread tests/components/switcher_kis/__init__.py Outdated
Comment thread homeassistant/components/switcher_kis/cover.py Outdated
Comment thread homeassistant/components/switcher_kis/cover.py Outdated
Comment thread homeassistant/components/switcher_kis/__init__.py Outdated
Comment thread homeassistant/components/switcher_kis/config_flow.py Outdated
@YogevBokobza YogevBokobza force-pushed the switcher-add-s11-support branch from 053cb4c to 201425c Compare September 20, 2024 18:46
@YogevBokobza
Copy link
Copy Markdown
Contributor Author

Thanks @YogevBokobza 👍

Feel free to make a PR to add yourself as a code-owner to get notified on issues and PRs.

Thanks :)
How to make that PR? what file need to be change?
Who can merge this PR?

@thecode thecode added the noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) label Sep 20, 2024
@thecode thecode merged commit 3e1da87 into home-assistant:dev Sep 20, 2024
JakeMartin-ICL pushed a commit to JakeMartin-ICL/home-assistant that referenced this pull request Sep 21, 2024
* switcher start s11 integration

* switcher linting

* switcher starting reauth logic

* switcher fix linting

* switcher fix linting

* switcher remove get_circuit_number

* switcher adding support for validate token

* switcher fix initial auth for new devices and fix strings

* switcher fix linting

* switcher fix utils

* Revert "switcher fix utils"

This reverts commit b162a94.

* switcher revert and test

* switcher fix validate logic and strings

* switcher add tests to improve coverage

* switcher adding tests

* switcher adding test

* switcher revert back things

* switcher fix based on requested changes

* switcher tests fixes

* switcher fix based on requested changes

* switcher remove single_instance_allowed code and added tests

* Update config_flow.py

* switcher fix comment

* switcher fix tests

* switcher lint

* switcehr fix based on requested changes

* switche fix lint

* switcher small rename fix

* switcher fix based on requested changes

* switcher fix based on requested changes

* switcher fix based on requested changes

* Update tests/components/switcher_kis/test_config_flow.py

Co-authored-by: Shay Levy <levyshay1@gmail.com>

* Update tests/components/switcher_kis/test_config_flow.py

Co-authored-by: Shay Levy <levyshay1@gmail.com>

* Update tests/components/switcher_kis/test_config_flow.py

Co-authored-by: Shay Levy <levyshay1@gmail.com>

* Update tests/components/switcher_kis/test_config_flow.py

---------

Co-authored-by: Shay Levy <levyshay1@gmail.com>
@github-actions github-actions Bot locked and limited conversation to collaborators Sep 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed code-owner-approved has-tests integration: switcher_kis new-feature noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) Quality Scale: platinum

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants