Conversation
…lso setting friendly names
…ys and appslist.json file
External mediaartwork
zehnm
left a comment
zehnm
left a comment
There was a problem hiding this comment.
I'm sorry for the delayed review.
This is just an initial quick code review and I'll start testing this afternoon.
The simple-justwatch-python-api library needs to be replaced or removed, otherwise we can't include the integration in our firmware due to licensing concerns.
GPLv3 grants the user the right to modify and replace the software, which get's complicated in our embedded firmware with a read-only filesystem. We do have the "custom integration installation" option, but that doesn't technically modify the existing software.
That's why I prefer to play it safe.
| sanitize-filename~=1.2.0 | ||
| async_timeout~=5.0.1 | ||
| adb_shell==0.4.4 | ||
| simple-justwatch-python-api>=0.16 No newline at end of file |
There was a problem hiding this comment.
The https://github.com/Electronic-Mango/simple-justwatch-python-api library is licensed under GPLv3. Unfortunately we cannot include this in our device firmware.
Please either remove it or replace it with another library licensed under BSD, MIT, Apache-2.0 or MPL-2.0
| def _get_config_root() -> Path: | ||
| config_home = Path(os.environ.get("UC_CONFIG_HOME", "./config")) | ||
| config_home.mkdir(parents=True, exist_ok=True) | ||
| return config_home | ||
|
|
||
|
|
||
| def _get_data_root() -> Path: |
There was a problem hiding this comment.
Exported functions should not be prefixed with _, they are intended for internal use.
_single_leading_underscore: weak "internal use" indicator. E.g. from M import * does not import objects whose name starts with an underscore.
| # async def encode_icon_to_data_uri(icon_name: str) -> str: | ||
| # """ | ||
| # Encode an image from a local file path or remote URL. | ||
| # | ||
| # Returns a base64-encoded PNG data URI. | ||
| # """ | ||
| # if isinstance(icon_name, MediaImage): | ||
| # icon_name = icon_name.url | ||
| # | ||
| # if isinstance(icon_name, str) and icon_name.startswith("data:image"): | ||
| # _LOG.debug("Icon is already a data URI") | ||
| # return icon_name | ||
| # | ||
| # _LOG.debug("Encoding icon to data URI: %s", icon_name) | ||
| # try: | ||
| # if _is_url(icon_name): | ||
| # async with httpx.AsyncClient() as client: | ||
| # response = await client.get(icon_name, timeout=10) | ||
| # response.raise_for_status() | ||
| # img_bytes = BytesIO(response.content) | ||
| # | ||
| # def encode_image() -> str: | ||
| # img = Image.open(img_bytes) | ||
| # img = img.convert("RGBA") | ||
| # buffer = BytesIO() | ||
| # img.save(buffer, format="PNG") | ||
| # encoded = base64.b64encode(buffer.getvalue()).decode("utf-8") | ||
| # return f"data:image/png;base64,{encoded}" | ||
| # | ||
| # return await asyncio.to_thread(encode_image) | ||
| # | ||
| # def load_and_encode() -> str: | ||
| # icon_path = _get_icon_path(icon_name) | ||
| # if not icon_path.exists(): | ||
| # raise FileNotFoundError(f"Icon not found: {icon_name}") | ||
| # with open(icon_path, "rb") as f: | ||
| # img = Image.open(f) | ||
| # img.load() | ||
| # img = img.convert("RGBA") | ||
| # buffer = BytesIO() | ||
| # img.save(buffer, format="PNG") | ||
| # encoded = base64.b64encode(buffer.getvalue()).decode("utf-8") | ||
| # return f"data:image/png;base64,{encoded}" | ||
| # | ||
| # return await asyncio.to_thread(load_and_encode) | ||
| # | ||
| # except Exception as e: | ||
| # _LOG.warning("Failed to encode icon to base64 for %s: %s", icon_name, e) | ||
| # return "" |
There was a problem hiding this comment.
Please remove commented out code blocks, unless they are used "real soon" and with a comment about the intention.
| adb_cert_path = Path(os.environ.get("UC_CONFIG_HOME", "./config")) / "certs" / f"adb_{choice}" | ||
| adb_cert_path_pub = Path(os.environ.get("UC_CONFIG_HOME", "./config")) / "certs" / f"adb_{choice}.pub" | ||
| if adb_cert_path.exists(): | ||
| adb_cert_path.unlink() | ||
| if adb_cert_path_pub.exists(): | ||
| adb_cert_path_pub.unlink() | ||
| appslist_path = Path(os.environ.get("UC_CONFIG_HOME", "./config")) / f"appslist_{choice}.json" |
There was a problem hiding this comment.
Please use the already imported get_config_root() function to get the configuration path to avoid duplicating logic.
| from adb_shell.auth.keygen import keygen | ||
| from adb_shell.auth.sign_pythonrsa import PythonRSASigner | ||
|
|
||
| ADB_CERTS_DIR = Path(os.environ.get("UC_CONFIG_HOME", "./config")) / "certs" |
There was a problem hiding this comment.
Please use the defined get_config_root() function in config.py to get the configuration path to avoid duplicating logic.
|
Sure. Was using this package for the purposes of sourcing media artwork for idenfied media playing when chromecast is enabled, specifically media that wasn't YouTube or natively provided via Chromecast. Do you have any suggested alternatives? There appears to be https://github.com/dawoudt/JustWatchAPI which is MIT and provides similar methods so would be an easy swap out. I haven't had an opportunity to do a clean up so there's some approaches that need to be refactored. But due to other commitments I haven't had time to review and make further contributions. I might have some time later next week. But any edits are welcome. |
|
No worries, I can push some linting fixes and disable the package. |
Run isort, black etc and a few manual fixes. There are still a few Pylint issues left that need to be addressed after testing.
2 participants
Known issue