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.

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 System Bridge Media Source #72865

Merged
merged 7 commits into from
Aug 16, 2022

Conversation

timmo001
Copy link
Contributor

@timmo001 timmo001 commented Jun 1, 2022

Proposed change

Adds media source to System Bridge integration.

Type of change

  • New feature (which adds functionality to an existing integration)

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
  • 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.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • 🥈 Silver

To help with the load of incoming pull requests:

@timmo001
Copy link
Contributor Author

timmo001 commented Jun 1, 2022

Python cache having issues. Will rebase later

@timmo001 timmo001 force-pushed the system-bridge-mediasource branch 2 times, most recently from 96d2c8b to fd8b508 Compare June 2, 2022 11:53
@timmo001 timmo001 force-pushed the system-bridge-mediasource branch from fd8b508 to 34ac1a0 Compare June 9, 2022 00:02
@timmo001 timmo001 marked this pull request as draft June 9, 2022 00:02
@timmo001 timmo001 marked this pull request as ready for review June 9, 2022 00:05
@timmo001 timmo001 force-pushed the system-bridge-mediasource branch from 1a50f09 to d9d4d84 Compare June 11, 2022 20:26
@frenck frenck added the smash Indicator this PR is close to finish for merging or closing label Jun 22, 2022
@frenck frenck removed the smash Indicator this PR is close to finish for merging or closing label Jun 28, 2022
@timmo001 timmo001 force-pushed the system-bridge-mediasource branch from 925882e to f662ec0 Compare July 1, 2022 09:45
@timmo001 timmo001 requested a review from balloob July 1, 2022 10:26
@timmo001 timmo001 force-pushed the system-bridge-mediasource branch from 973972a to d6eb26a Compare July 6, 2022 17:07
@timmo001
Copy link
Contributor Author

timmo001 commented Jul 6, 2022

Updated package to use futures as suggested

https://github.com/timmo001/system-bridge/releases/tag/3.3.1

@timmo001 timmo001 force-pushed the system-bridge-mediasource branch 2 times, most recently from 2aeefcd to 2e41384 Compare July 16, 2022 21:23
@timmo001 timmo001 force-pushed the system-bridge-mediasource branch from 2e41384 to 38348ca Compare August 15, 2022 13:40
@timmo001 timmo001 force-pushed the system-bridge-mediasource branch from 2c2850d to aa65303 Compare August 15, 2022 15:46
@balloob balloob merged commit 3cb062d into home-assistant:dev Aug 16, 2022
@timmo001 timmo001 deleted the system-bridge-mediasource branch August 16, 2022 16:49
@github-actions github-actions bot locked and limited conversation to collaborators Aug 17, 2022
entry: ConfigEntry,
) -> str:
"""Build base url for System Bridge media."""
return f"http://{entry.data[CONF_HOST]}:{entry.data[CONF_PORT]}/api/media/file/data?apiKey={entry.data[CONF_API_KEY]}"
Copy link
Member

Choose a reason for hiding this comment

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

Please split long strings around 88 characters per line.

domain=DOMAIN,
identifier=f"{path}/{media_file.name}{ext}",
media_class=MEDIA_CLASS_DIRECTORY
if media_file.is_directory or media_file.mime_type is None
Copy link
Member

Choose a reason for hiding this comment

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

A ternary operator expression inside an instantiating multi-line call like this is really hard to read. Please rewrite it.

"""Build individual media item."""
ext = (
f"~~{media_file.mime_type}"
if media_file.is_file and media_file.mime_type 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.

Please don't use ternary operator expressions that span more than one line.

@frenck frenck added the noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear) label Aug 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
by-code-owner cla-signed integration: system_bridge new-feature noteworthy Marks a PR as noteworthy and should be in the release notes (in case it normally would not appear)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants