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 the 10 minutes timeout issue #410

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

swiergot
Copy link

I have forked DashCast to implement a fix for the timeout problem:

https://github.com/swiergot/dashcast

It is registered at Google under ID 5C517DAC.

This pull request a) switches to this new DashCast version and b) adds a new option to cast_site to enable the fix (for backward compatibility the old behaviour remains the default).

Please note that due to security constraints the new method requires the casted website to be served over HTTPS.

@skorokithakis
Copy link
Owner

Thank you! I'm a bit wary of relying on a third party for part of CATT's functionality, would it be possible to add that to CATT?

AFAIK we already have a custom receiver that's registered, it's on this branch:

https://github.com/skorokithakis/catt/blob/feat/custom-receiver/catt_receiver/receiver.html

@@ -26,6 +25,9 @@
VALID_STATE_EVENTS = ["UNKNOWN", "IDLE", "BUFFERING", "PLAYING", "PAUSED"]
CLOUD_APP_ID = "38579375"

DASHCAST_NAMESPACE = "urn:x-cast:es.offd.dashcast"
DASHCAST_APP_ID = "5C517DAC"
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
DASHCAST_APP_ID = "5C517DAC"
DASHCAST_APP_ID = "1B3FF1BC"

@skorokithakis
Copy link
Owner

I've deployed your fork to our custom receiver, would it be possible to deploy your code upstream for inclusion?

@theychx
Copy link
Collaborator

theychx commented Jul 31, 2022

@swiergot how is your approach different from the iframe approach already present in dashcast?
As I recall, using an iframe means a lot of sites won't work.

@skorokithakis
Copy link
Owner

@theychx I'm glad you showed up, I was out of my depth here :P

@swiergot
Copy link
Author

I've deployed your fork to our custom receiver, would it be possible to deploy your code upstream for inclusion?

Where exactly did you deploy it? The link you gave returns 404.

What upstream do you have in mind? https://github.com/stestagg/dashcast?

@swiergot how is your approach different from the iframe approach already present in dashcast? As I recall, using an iframe means a lot of sites won't work.

So now the options are:

  1. DashCast in window.location mode (old or new)

    • works with all websites,
    • times out after 10 minutes (unless the target website is a receiver itself - that's what ha-catt-fix does).
  2. Old DashCast in iframe mode

    • requires the target URL to be HTTPS,
    • doesn't work with websites that block iframe,
    • times out after 10 minutes.
  3. New DashCast in iframe mode

    • requires the target URL to be HTTPS,
    • doesn't work with websites that block iframe,
    • does NOT time out after 10 minutes.

The third, new option will be a huge improvement for many people.

@skorokithakis
Copy link
Owner

Where exactly did you deploy it? The link you gave returns 404.

It's at https://catt.stavros.io/, the link 404s because I moved the file when I added your fork to it.

What upstream do you have in mind? https://github.com/stestagg/dashcast?

Yes, though I now see it hasn't been updated in 6 years, so it'd probably be pointless.

The third, new option will be a huge improvement for many people.

I agree that having the option is worthwhile, even if it doesn't work for everyone. Since it's an option, they can choose "works with every site for 10 minutes" or "works with some sites for however long".

@theychx
Copy link
Collaborator

theychx commented Jul 31, 2022

@swiergot Ok, thanks for clarifying.

@skorokithakis
Copy link
Owner

@theychx does this look good to you?

@swiergot
Copy link
Author

It's at https://catt.stavros.io/, the link 404s because I moved the file when I added your fork to it.

Ok, that looks fine.

Yes, though I now see it hasn't been updated in 6 years, so it'd probably be pointless.

Well, I've done it anyway, won't hurt (stestagg/dashcast#21).

I agree that having the option is worthwhile, even if it doesn't work for everyone. Since it's an option, they can choose "works with every site for 10 minutes" or "works with some sites for however long".

It will work for people casting Home Assistant and similar.

Although after playing with it today I can see some weird Home Assistant UI issues (eg. mini-graph-card not showing the graph). I did not observe that with the ha-catt-fix method.

"-i",
"--iframe",
is_flag=True,
help="Load website into an iframe to avoid timeout (requires HTTPS).",
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
help="Load website into an iframe to avoid timeout (requires HTTPS).",
help="Load website into an iframe to avoid timeout (requires HTTPS, and the site must allow loading into an iframe).",

@theychx
Copy link
Collaborator

theychx commented Jul 31, 2022

@skorokithakis I'm not much of a javascript expert, so cant give any immediate critique.

@swiergot Using cast_site repeatedly doesn't appear to be working
(in either mode using https://rtl.de and https://bbc.co.uk)

@swiergot
Copy link
Author

@swiergot Using cast_site repeatedly doesn't appear to be working

Thanks, I will be on vacation for the next two weeks, will look into it afterwards.

@skorokithakis
Copy link
Owner

Thanks, enjoy your vacation!

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.

3 participants