Wait for cast device to boot before failing #366
Conversation
|
Just to add an input if needed ; i manually implemented the edits from this pull and it works just fine now. Whatever may be the state of my cast devices, they wake up just fine to play music. @Ionshard thank you |
|
I can also confirm that this PR works great and completely fixes #334. Thank you. |
|
I have implemented the edits and the error is gone! Thank you! |
|
Hi @Ionshard, I finally have time to review PR. I'm taking care of your PR next. I made some changes to the code when implementing two (2) other PRs you currently have two (2) merge conflicts with the master branch. Can you resolve them? I will be reviewing the rest of the code while you do that. |
| @@ -41,7 +41,7 @@ | |||
| WS_TYPE_SPOTCAST_PLAYER, | |||
| WS_TYPE_SPOTCAST_PLAYLISTS, | |||
| ) | |||
| from .helpers import async_wrap, get_cast_devices, get_spotify_devices | |||
| from .helpers import async_wrap, get_cast_devices, get_spotify_devices, get_spotify_media_player | |||
There was a problem hiding this comment.
please see merge conflicts, due to the line getting long, I moved to a parentheses block import for the helper methods
There was a problem hiding this comment.
I have fixed the merge conflict here.
| def get_spotify_devices(hass, spotify_user_id): | ||
| def get_spotify_media_player(hass: ha_core.HomeAssistant, spotify_user_id: str) -> SpotifyMediaPlayer: | ||
| """ | ||
| Get the spotify media player entity from hass. |
There was a problem hiding this comment.
I'm trying to clean up code documentation and move it toward the PEP8 format. could you rewrite this comment section using that format?
There was a problem hiding this comment.
I am not a Python developer so I am not 100% certain on the PEP8 format. I looked it up and I think all I really needed to do was ensure the docstring was all on one line for a single line doc string correct?
There was a problem hiding this comment.
No problem, I actually just learned that I am not using the PEP8 version of docstring (because there are none... my bad) but the google convention. So basically, it's:
"""Returns the sum of two decimal numbers in binary digits.
Args:
a (int): A decimal integer
b (int): Another decimal integer
Returns:
bool: Binary string of the sum of a and b
"""Anyway, no need to rewrite, I'm going to review the code, mostly I needed the merge conflict to be resolved in order to integrate your change.
|
Sorry for the delay, I am currently on vacation. It may take me a couple days to be able to make the requested changes. I will update the PR when I return. |
|
I manually copied the code and seems to be working without errors. Thank you both for your time with this. |
|
Thank you for your patience. I have returned from vacation and have addressed the comments. |
I was running into the issues described in #334 and I saw the #339 PR which looked promising but has a bunch of syntax errors and appears to be abandoned.
This PR takes the implementation from that PR and brings it up to date with the latest commits and actually compiles.