Skip to content

Plex discovery on demand#35303

Merged
cgarwood merged 3 commits intohome-assistant:devfrom
jjlawren:plex_discovery_on_demand
May 13, 2020
Merged

Plex discovery on demand#35303
cgarwood merged 3 commits intohome-assistant:devfrom
jjlawren:plex_discovery_on_demand

Conversation

@jjlawren
Copy link
Copy Markdown
Contributor

@jjlawren jjlawren commented May 6, 2020

Proposed change

Plex does not support the standard protocols for discovery (SSDP, zeroconf) and the legacy netdisco discovery didn't provide a unique ID. The underlying library now supports GDM discovery which can be triggered by a call to the integration.

This PR doesn't add any code that's directly called yet, but sets up the ability to trigger discovery on demand during onboarding.

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)
  • 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 #
  • This PR is related to issue:
  • Link to documentation pull request:

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:

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.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

for server_data in gdm.entries:
await hass.config_entries.flow.async_init(
DOMAIN,
context={"source": config_entries.SOURCE_DISCOVERY},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should use another source. This source is already reserved for the discovery integration.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should I create a new one? Any suggestions in mind? Maybe "discovery requested" or "onboarding"?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

integration_discovery maybe? Or self discovery

Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

This looks good 👍

@jjlawren jjlawren force-pushed the plex_discovery_on_demand branch from fbc4884 to 8b726b5 Compare May 9, 2020 04:49
@jjlawren jjlawren marked this pull request as ready for review May 9, 2020 05:04
@cgarwood cgarwood merged commit c12f8be into home-assistant:dev May 13, 2020
}


async def async_discover(hass):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What is calling this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Nothing yet (besides the tests). The plan discussed is to offer integration-handled discovery as part of onboarding.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is there an architecture issue where this is described?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, this is already described pretty well in home-assistant/architecture#172.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think those are just ideas. The api is not described from what I read.

@lock lock Bot locked and limited conversation to collaborators May 20, 2020
@jjlawren jjlawren deleted the plex_discovery_on_demand branch May 31, 2022 17:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants