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.

Sign up for GitHub

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

Jump to bottom

Refactor Freebox : add config flow + temperature sensor + signal dispatch #30334

Merged
merged 45 commits into from
Mar 11, 2020
Merged

Refactor Freebox : add config flow + temperature sensor + signal dispatch #30334

merged 45 commits into from
Mar 11, 2020

Conversation

Quentame
Copy link
Member

@Quentame Quentame commented Dec 31, 2019 •
edited
Loading

Breaking Change:

None in the code.

But need to re-authorize HA to access the Freebox router.

Description:

Adding config flow to Freebox + temperature sensors

Can de done after :

  • add more sensors and services
  • refactor discovery
  • available entity state
  • API local polling

Pull request with documentation for home-assistant.io : home-assistant/home-assistant.io#11712
Pull request with frontend for home-assistant-polymer : home-assistant/frontend#4448

PS : HAPPY NEW YEAR 🎉 🎆 !

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 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. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

If the code does not interact with devices:

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

@project-bot project-bot bot added this to Needs review in Dev Dec 31, 2019
@probot-home-assistant
Copy link

Hey there @SNoof85, mind taking a look at this pull request as its been labeled with a integration (freebox) you are listed as a codeowner for? Thanks!

@Quentame Quentame changed the title Add config flow to Freebox WIP : Add config flow to Freebox Dec 31, 2019
@SNoof85
Copy link
Contributor

SNoof85 commented Jan 1, 2020 •
edited
Loading

That is very nice @Quentame I started something in that way too but you were faster.
Like it !
You may add yourself as codeowner if you want. And a happy new year.

@SNoof85
Copy link
Contributor

SNoof85 commented Jan 2, 2020

Tried it yesterday. Works fine.
I was thinking that adding a step after the form Host/Port to mention that the user has to press the right arrow on the Freebox router to validate the application access would be nice.

@Quentame Quentame force-pushed the freebox/config-flow branch 2 times, most recently from ec30949 to 9a641cc Compare January 5, 2020 15:52
@MartinHjelmare MartinHjelmare moved this from Needs review to Incoming in Dev Jan 7, 2020
Quentame
Quentame commented Jan 9, 2020
View reviewed changes
Copy link
Member Author

@Quentame Quentame left a comment

Choose a reason for hiding this comment

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

Some changes to be done before remove the draft status + update the description

homeassistant/components/freebox/device_tracker.py Outdated Show resolved Hide resolved
homeassistant/components/freebox/config_flow.py Outdated Show resolved Hide resolved
homeassistant/components/freebox/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/freebox/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/freebox/__init__.py Outdated Show resolved Hide resolved
@Quentame Quentame self-assigned this Jan 9, 2020
@Quentame Quentame force-pushed the freebox/config-flow branch 2 times, most recently from 9ebdaa6 to e6c52e1 Compare January 10, 2020 12:37
@Quentame Quentame mentioned this pull request Jan 10, 2020
Merged
@Quentame Quentame force-pushed the freebox/config-flow branch 2 times, most recently from 737e13d to 13b81a3 Compare January 11, 2020 16:58
@Quentame Quentame marked this pull request as ready for review January 11, 2020 16:59
@Quentame Quentame changed the title WIP : Add config flow to Freebox Add config flow to Freebox Jan 11, 2020
@Quentame Quentame mentioned this pull request Jan 11, 2020
Merged
2 tasks
@Quentame Quentame moved this from Incoming to Needs review in Dev Jan 11, 2020
This was referenced Jan 11, 2020
Merged
Merged
@Quentame Quentame removed their assignment Jan 11, 2020
MartinHjelmare
MartinHjelmare approved these changes Mar 11, 2020
View reviewed changes
Copy link
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! Thanks!

Dev automation moved this from Review in progress to Reviewer approved Mar 11, 2020
@MartinHjelmare
Copy link
Member

Can we merge here or do we need to wait for the frontend PR?

@Quentame
Copy link
Member Author

Quentame commented Mar 11, 2020 •
edited
Loading

Don't know the order, the PR is written as "wait for backend" home-assistant/frontend#4448

@Quentame
Copy link
Member Author

Should be front then back, then doc I guess

@MartinHjelmare
Copy link
Member

Ok. Please check with frontend team. I'll leave this here for now. It can be merged when ready.

@Quentame
Copy link
Member Author

Thanks 🙏

@bramkragten
Copy link
Member

Frontend merged

@bramkragten bramkragten merged commit 19be4a5 into home-assistant:dev Mar 11, 2020
Dev automation moved this from Reviewer approved to Done Mar 11, 2020
@Quentame Quentame deleted the freebox/config-flow branch March 11, 2020 21:17
@lock lock bot locked and limited conversation to collaborators Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@SNoof85 SNoof85 SNoof85 approved these changes

@MartinHjelmare MartinHjelmare MartinHjelmare approved these changes

Assignees
No one assigned
Labels
breaking-change cla-signed has-tests integration: freebox
Projects
Dev
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Quentame @SNoof85 @MartinHjelmare @bramkragten @homeassistant