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

Add config flow to frontier_silicon #64365

Merged
merged 37 commits into from
Mar 10, 2023
Merged

Add config flow to frontier_silicon #64365

Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
427a711
Add config_flow to frontier_silicon
wlcrs Jun 29, 2022
1436787
Add missing translation file
wlcrs Jun 29, 2022
7fc104b
Delay unique_id validation until radio_id can be determined
wlcrs Jun 29, 2022
2c3456a
Fix tests
wlcrs Jun 29, 2022
f7bcbff
Improve tests
wlcrs Jun 29, 2022
65be2b6
Use FlowResultType
wlcrs Jun 29, 2022
1bae242
Bump afsapi to 0.2.6
wlcrs Jul 12, 2022
482f0e2
Fix requirements_test_all.txt
wlcrs Jul 26, 2022
44c1a28
Stash ssdp, reauth and unignore flows for now
wlcrs Aug 24, 2022
1fa59e2
Re-introduce SSDP flow
wlcrs Aug 25, 2022
5170de2
hassfest changes
wlcrs Aug 25, 2022
4a17ac8
Merge branch 'dev' into fsapi-0.2.0-update
wlcrs Oct 13, 2022
d4ca822
Merge branch 'dev' into fsapi-0.2.0-update
wlcrs Oct 14, 2022
e527195
Merge remote-tracking branch 'upstream/dev' into fsapi-0.2.0-update
wlcrs Oct 18, 2022
ddf694e
Address review comments
wlcrs Oct 18, 2022
f269111
Small style update
wlcrs Oct 18, 2022
577a6f3
Fix tests
wlcrs Oct 19, 2022
808217b
Merge branch 'dev' into fsapi-0.2.0-update
wlcrs Nov 6, 2022
447edf9
Update integrations.json
wlcrs Nov 6, 2022
12ef02b
Merge branch 'dev' into fsapi-0.2.0-update
wlcrs Nov 6, 2022
956d8b0
Merge branch 'dev' into fsapi-0.2.0-update
wlcrs Dec 16, 2022
62caf1e
Merge branch 'dev' into fsapi-0.2.0-update
wlcrs Mar 2, 2023
842f7e2
fix order in manifest.json
wlcrs Mar 2, 2023
1b8ef30
fix black errors
wlcrs Mar 2, 2023
eb5abb8
Apply suggestions from code review
wlcrs Mar 2, 2023
e087f21
Address review comments
wlcrs Mar 2, 2023
43c715e
fix black errors
wlcrs Mar 2, 2023
da0a7e1
Use async_setup_platform instead of async_setup
wlcrs Mar 6, 2023
e215fdd
Address review comments on tests
wlcrs Mar 6, 2023
1e69338
parameterize tests
wlcrs Mar 6, 2023
6fcb5c5
Remove discovery component changes from this PR
wlcrs Mar 6, 2023
c956220
Address review comments
wlcrs Mar 6, 2023
e174b53
Apply suggestions from code review
wlcrs Mar 6, 2023
382ed4f
Add extra asserts to tests
wlcrs Mar 6, 2023
df07d73
Restructure _async_step_device_config_if_needed
wlcrs Mar 6, 2023
06090ad
Add return statement
wlcrs Mar 6, 2023
08896fd
Update homeassistant/components/frontier_silicon/media_player.py
wlcrs Mar 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add return statement
  • Loading branch information
@wlcrs
wlcrs committed Mar 6, 2023
commit 06090adf49f28e025ee87b6c47a95d7875e7d37d
7 changes: 5 additions & 2 deletions homeassistant/components/frontier_silicon/media_player.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,10 @@ async def async_setup_platform(
discovery_info: DiscoveryInfoType | None = None,
) -> None:
"""Set up the Frontier Silicon platform.

YAML is deprecated, and imported automatically.
SSDP discovery is temporarily retained - to be refactor subsequently."""
SSDP discovery is temporarily retained - to be refactor subsequently.
"""
if discovery_info is not None:
Copy link
Member

Choose a reason for hiding this comment

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

You should only do the import in the platform setup and add the media player entities only from the setup_entry method. Right now you add the entities twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In which case are the entities loaded twice? I fail to see what problem you are mentioning?

In case of an SSDP discovery: they are added here in async_setup_platform and not in setup_entry.
In case of an import of the old YAML: the entity is migrated here but not added. It is subsequently added from setup_entry.
In case of an entity added via the config flow: only added via setup_entry.

Copy link
Member

@Kane610 Kane610 Mar 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

In which case are the entities loaded twice? I fail to see what problem you are mentioning?

In case of an SSDP discovery: they are added here in async_setup_platform and not in setup_entry. In case of an import of the old YAML: the entity is migrated here but not added. It is subsequently added from setup_entry. In case of an entity added via the config flow: only added via setup_entry.

Aah sorry, I apparently forgot the nomenclature for the old way of loading entities. Still good I mentioned it as you should also move the SSDP discovery from the old discovery way to the config flow, there shouldn't be any in-between steps, its either old platform or new way. Can't say that I have any other comments apart from this though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah - that was a recommendation that I made, to reduce the size of the PR.
I felt it was easier to review without SSDP, and then migrate SSDP as a follow-up PR.

Copy link
Member

Choose a reason for hiding this comment

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

Ok. And this was the future config entry PR mentioned? If that is the case I can be ok with that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I'll be creating a PR for the following branch from the moment this one is merged: wlcrs/home-assistant-core@fsapi-0.2.0-update...wlcrs:home-assistant-core:frontier_silicon_ssdp

webfsapi_url = await AFSAPI.get_webfsapi_endpoint(
discovery_info["ssdp_description"]
Expand All @@ -67,6 +68,8 @@ async def async_setup_platform(
True,
)

return

ir.async_create_issue(
hass,
DOMAIN,
Expand Down