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

Sony Bravia XE add #1286

Merged
merged 1 commit into from May 31, 2017
Merged

Sony Bravia XE add #1286

merged 1 commit into from May 31, 2017

Conversation

@SubJunk
Copy link
Member

SubJunk commented May 25, 2017

What's chromecast_extension?

@Sami32
Copy link
Contributor Author

Sami32 commented May 25, 2017

Since many users reported that they have connection issue without this option, and that they ask and search where is their correct ums.conf file to modify, i thought it was much easier for users to set it directly in their renderer, as it will work same as if that was done in ums.conf anyway.

Because when i read that this user make 3H, i suspect that some other users just give up ?

@Nadahar
Copy link
Contributor

Nadahar commented May 25, 2017

We shouldn't "mix the cards" by trying to patch a bug by making another one in my opinion. The Cromecast implementation is bugged for several reasons, and unless someone that actually has a Chromecast for testing cleans up this code we should disable Chromecast by default. I don't even know if the current implementation does much good for those that have one, and this buggy implementation certainly can cause issues for those who don't. Disabling it in the renderer configuration is definitly wrong, as the problem isn't related to that particular renderer.

It won't work as intended either. UMS doesn't read that particular renderer's configuration when determining if ChromecastMgr should be started. It's done in PMS.init() using the "global" configuration.

@Sami32
Copy link
Contributor Author

Sami32 commented May 26, 2017

Corrected.

@Nadahar I understood that you said me that the parameters should be handled in whatever configuration file they was, so after all, it's seem that it's not all the parameters ?

@Nadahar
Copy link
Contributor

Nadahar commented May 26, 2017

@Sami32 No, it's more complicated than that. All parameters are "supported" in the renderer configuration file as well, but you have to think of how this works. It only applies to parameters that's relevant for a particular renderer. The chromecast "service" starts up without being related to any particular renderer. Even if you turn it off in that particular renderer configuration, it's not turned off in all the other renderer configurations. How should UMS know to listen to that particular renderer configuration file?

@Sami32
Copy link
Contributor Author

Sami32 commented May 26, 2017

@Nadahar So it was fine, as i intended to disable only for this kind of renderers having issue with it activated, avoiding the total desactivation for the other Chromacast able renderers that users can have and work fine (I've serious doubt about "Chromecast fine" ;))

@Nadahar
Copy link
Contributor

Nadahar commented May 26, 2017

@Sami32 No, as I tried to explain, it doesn't work like that. UMS either runs the chromecast service or not. That's not "tied" to a renderer, so setting that in a renderer configuration will have no effect at all.

I think we should disable it in UMS.conf by default because it is so buggy and will probably create problems not only for this renderer.

@Sami32
Copy link
Contributor Author

Sami32 commented May 26, 2017

@Nadahar I see, so i agree that it should disable by default, as it's not the first user on the forum having issue with their Chromecast renderer, and it did take me some time to found why they cannot make UMS working with it.

@Sami32 Sami32 merged commit 7050313 into master May 31, 2017
@Sami32 Sami32 deleted the Sami32-patch-3 branch May 31, 2017 17:29
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.

None yet

3 participants