search

merdok / homebridge-webos-tv

Homebridge plugin for LG webOS TVs
MIT License
664 stars 89 forks source link

Added state option for picture mode buttons #505

Closed tcarlson25 closed 1 year ago

tcarlson25 commented 1 year ago

This is a solution related to this issue 497.

Problem: Picture mode buttons do not have a state like sound mode buttons do. It is understood that this is because the TV does not give the active picture mode in the response, so nothing can be parsed.

Solution A solution implemented in this pull request involves adding another parameter called statefulPictureMode. When set to false, all picture mode buttons behave like normal (which is keeping them all disabled even after a new mode is set). However, when set to true, the plugin uses a global variable to keep track of the active picture mode state. When the plugin first starts up, no buttons will be enabled because no picture mode is set to active. After a picture mode is selected, the variable is updated after updating the TV mode, and the state then acts just like it does in sound modes, except the state is retrieved from a global variable instead of the TV.

While this isn't perfect, I think it is a pretty good solution, rather than not having any state for picture modes. When the bridge restarts, all buttons will be disabled. However, when a picture mode is selected, the picture mode button states perform perfectly (just like sound modes).

merdok commented 1 year ago

Thanks for that. I would need to think about it. The thing is, it will solve your issue but introduce more code which needs to be maintained, so i ask myself in that regards, how many other people would actually use that also?

tcarlson25 commented 1 year ago

It really isn't completely "new code" though. I pretty much copied the exact configuration that is done for sound mode, and applied it for picture mode - using the statefulService instead of statelessService.

The only thing "new" really is the configurable property. If that is the concern, it is also an option to remove the configurable property and have it always use the global property for picture mode state. In my opinion, it is better that way anyway rather than just having no state and disabling all picture mode buttons. I can't see why someone would want to have the new statefulPictureMode disabled in the first place. So that is an option too - removing the property and keeping picture modes stateful with the global property. Thoughts on this approach?

merdok commented 1 year ago

Well, the media pause or screen on state are also not reported by the tv and i also use an internal variable to keep track of that. So i would say if you could adjust the code in LgTvController to match isScreenOn and mediaPaused instead of let CURRENT_PICTURE_MODE = '' And lets remove the statefulPictureMode flag and always use that, then i guess we could merge it.

tcarlson25 commented 1 year ago

Please take a look. I just made the updates.

merdok commented 1 year ago

Thanks, the initial value for pictureMode i guess would be better null