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 Configurable Launch Timeout to Spotify App Launch #424

Merged
merged 0 commits into from
May 7, 2024

Conversation

bruvv
Copy link

@bruvv bruvv commented Feb 18, 2024

This Pull Request introduces the ability to configure the timeout for launching the Spotify application within spotcast. The primary motivation behind this enhancement is to provide users with more control over the launch process, particularly in varying network conditions where the default timeout may not be sufficient.

Key Changes:

  • Configurable Timeout: Adds a launch_timeout parameter to the launch_app method, allowing for dynamic adjustment based on user configuration or environmental conditions.
  • Improved Error Handling: Updates the LaunchError exception message to include the launch_timeout value. This enhancement aids in debugging by explicitly stating the timeout duration after which the launch was considered unsuccessful.
  • Service Schema Update: Adjusts the spotcast.start service schema to recognize set_launch_timeout as an optional parameter, enabling users to specify a custom timeout for each launch attempt via service calls.

Motivation and Context:

In some environments, the response time from the Spotify app can exceed the previously hard-coded timeout, leading to premature launch failures. By making the timeout configurable, we empower users to tailor the component's behavior to their specific setup, potentially reducing the occurrence of unnecessary errors and improving the overall reliability of the Spotify launch process.

Testing:

I have done testing for myself and it seems to work, please test this yourself before merging.

@NonaSuomy
Copy link

How do you use this where do you set launch_timeout and what value should you use?

@bruvv
Copy link
Author

bruvv commented Mar 1, 2024

@NonaSuomy its in seconds with a minimum of 10 second and you can configure it via the ui or use it in a service call

@fcusson
Copy link
Collaborator

fcusson commented Mar 1, 2024

@bruvv I had to update som breaking change with pychromecast, can you fix merge conflict. I'll work on integrating your pr

@bruvv
Copy link
Author

bruvv commented Mar 1, 2024

@fcusson sure, i have updated it. let me know if you need anything else. I have not tested this merge now tho so please test it first.

@fcusson
Copy link
Collaborator

fcusson commented Mar 1, 2024

Thank you

@bruvv
Copy link
Author

bruvv commented Mar 12, 2024

@fcusson any update on this?

@fcusson fcusson merged commit 8df59dd into fondberg:master May 7, 2024
24 checks passed
@hmmbob
Copy link
Contributor

hmmbob commented Jul 14, 2024

Is this part of a release already?

@fcusson
Copy link
Collaborator

fcusson commented Jul 14, 2024

Not yet need to draft it and make the release. I was planning on doing that during the weekend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants