Skip to content

Add Ring config flow#30564

Merged
balloob merged 6 commits into
devfrom
ring-config-flow
Jan 10, 2020
Merged

Add Ring config flow#30564
balloob merged 6 commits into
devfrom
ring-config-flow

Conversation

@balloob
Copy link
Copy Markdown
Member

@balloob balloob commented Jan 7, 2020 •
edited by frenck
Loading

Breaking Change

Ring is now configurable via a config entry. This means it will no longer store the username and password and you're able to configure accounts that use 2FA. It's no longer possible to set scan interval, monitored conditions or ffmpeg arguments. The defaults are used.

Description:

This adds config flow support to Ring.

Benefits:

  • No longer need to store username/password in config. Once logged in we just store the tokens.
  • Support two-factor auth

CC @tchellomello

resolves #25088

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@tchellomello
Copy link
Copy Markdown
Contributor

tchellomello commented Jan 7, 2020

@balloob this is awesome!! I'm going to test it tonight!

MartinHjelmare
MartinHjelmare requested changes Jan 7, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Also remove the stale CONF_SCAN_INTERVAL option from the config schema.

Comment thread homeassistant/components/ring/config_flow.py Outdated
Comment thread tests/components/ring/test_config_flow.py Outdated
@tchellomello
Copy link
Copy Markdown
Contributor

tchellomello commented Jan 8, 2020

@balloob I was able to get it working.

Basically one a fresh new installation, I went to Configure -> Integrations -> Ring.

Added my username and password there and the component got added correctly, however, no image was displayed so far.

Then I modified the configuration.yaml including the ring component to the camera platform, I was able to see the video.

image

Tomorrow I'll enable the 2FA to test as well. So far, so good :) 👍

@balloob
Copy link
Copy Markdown
Member Author

balloob commented Jan 8, 2020

Yeah I realized I need to forward the config entry. Haven't done that yet. I'll clean it up. Still going to add it to the beta

@balloob
Copy link
Copy Markdown
Member Author

balloob commented Jan 8, 2020

Addressed comments and converted the platforms to use config entries too. Now it's able to delete the entry without restart too.

Just adding device info would make this 🔥 but I don't have any Ring devices to work with. Just using an empty account. That's for someone else.

@balloob balloob added this to the 0.104.0 milestone Jan 8, 2020
@balloob balloob mentioned this pull request Jan 8, 2020
Merged
@dshokouhi
Copy link
Copy Markdown
Member

dshokouhi commented Jan 8, 2020

This should help to close #25088 :)

MartinHjelmare
MartinHjelmare reviewed Jan 9, 2020
View reviewed changes
Comment thread homeassistant/components/ring/camera.py Outdated
Comment thread homeassistant/components/ring/camera.py
Comment thread homeassistant/components/ring/__init__.py
@jcollie jcollie mentioned this pull request Jan 9, 2020
Closed
MartinHjelmare
MartinHjelmare approved these changes Jan 10, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Looks good!

MartinHjelmare
MartinHjelmare requested changes Jan 10, 2020
View reviewed changes
Comment thread homeassistant/components/ring/__init__.py Outdated
@balloob balloob merged commit 3f29c23 into dev Jan 10, 2020
@delete-merged-branch delete-merged-branch Bot deleted the ring-config-flow branch January 10, 2020 20:35
balloob added a commit that referenced this pull request Jan 10, 2020
@balloob
Add Ring config flow (#30564)
0b3b589 
* Add Ring config flow

* Address comments + migrate platforms to config entry

* Migrate camera too

* Address comments

* Fix order config flows

* setup -> async_setup
@robfish1956
Copy link
Copy Markdown

robfish1956 commented Jan 11, 2020 •
edited
Loading

I tried this using docker image: homeassistant/home-assistant:0.104.0b1
and the Ring integration is available but when I submit my username and password I get
"Unexpected Error"
I have not enabled 2FA on my ring account

@Scope666
Copy link
Copy Markdown

Scope666 commented Jan 11, 2020

I also have the EXACT same error as @robfish1956

@balloob
Copy link
Copy Markdown
Member Author

balloob commented Jan 11, 2020

The ring integration has been blocked again because the library was not fixed properly last time, still causing unnecessary token refreshes on EACH request. That needs to be fixed and this will work again.

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Jan 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@MartinHjelmare MartinHjelmare MartinHjelmare approved these changes

Assignees

No one assigned

Labels

breaking-change cherry-picked cla-signed has-tests integration: ring

Projects

None yet

Milestone

0.104.0

Development

Successfully merging this pull request may close these issues.

Ring seems to need 2-factor now

7 participants

@balloob @tchellomello @dshokouhi @robfish1956 @Scope666 @MartinHjelmare @homeassistant