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

fix: Give correct error messages with Dashcast #133

Merged
merged 3 commits into from
Oct 12, 2018
Merged

Conversation

theychx
Copy link
Collaborator

@theychx theychx commented Oct 11, 2018

Because of my earlier "fix", the wrong CastController was being returned if the Dashcast app was running on the cc. Resulting in irrelevant error messages when issuing a control command (which the dashcast CastController does not support).

The CI failure is still because of the DailyMotion extractor, which was fixed in the latest release of youtube-dl. Any ideas? Or should we just update requirements?

..the default castcontroller was being returned when Dashcast was active.
@skorokithakis
Copy link
Owner

Is there a downside to just updating requirements?

@theychx
Copy link
Collaborator Author

theychx commented Oct 11, 2018

Is there a downside to just updating requirements?

Nope, I was just wondering about the intricacies of package versions used in CI builds.
I'll make a seperate PR with the updated setup.py.

@skorokithakis
Copy link
Owner

We should update whenever we can, as long as it doesn't break anything. Unfortunately catt is almost impossible to automatically test...

@theychx
Copy link
Collaborator Author

theychx commented Oct 11, 2018

Indeed. Are you ok with this PR?

@skorokithakis
Copy link
Owner

Yep, looks great!

theychx added 2 commits October 12, 2018 08:44
..in order to make CI pass
..by replacing dailymotion test with twitch test.
@theychx
Copy link
Collaborator Author

theychx commented Oct 12, 2018

Merging is blocked for me when CI has failed, so I took the liberty of updating requirements here. For unknown reasons the dailymotion test still fails, so I gave up and replaced this test with a twitch one.

@skorokithakis skorokithakis merged commit 138f580 into master Oct 12, 2018
@skorokithakis
Copy link
Owner

Looks great, thank you!

@skorokithakis skorokithakis deleted the dashcast-fix2 branch October 12, 2018 10:38
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.

2 participants