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

picture_settings only contains the last set values #45

Open
nisargjhaveri opened this issue Dec 3, 2021 · 7 comments
Open

picture_settings only contains the last set values #45

nisargjhaveri opened this issue Dec 3, 2021 · 7 comments

Comments

@nisargjhaveri
Copy link

I was trying to integrate the new picture_settings and ability to set brightness, contrast, etc in home assistant and this is what I observed.

Initially picture_settings contains all four brightness, contrast, backlight and color properties. But once I called set_current_picture_settings with a dict containing only backlight, the picture_settings now only contains the backlight property.

Is this expected? How to ensure that all the properties are correctly reported even after setting one of the properties optionally?

@chros73
Copy link

chros73 commented Dec 3, 2021

Hi! Can you write down the exact steps (methods) to reproduce it, along with the expected result? I can take a look at it then.

@nisargjhaveri
Copy link
Author

Here is the minimal script that I tried with the output.

import asyncio
from aiopylgtv import WebOsClient

async def on_state_change():
    print("State changed:", client.picture_settings)

async def runloop():
    global client
    client = await WebOsClient.create('<host>', '<key file>')

    await client.register_state_update_callback(on_state_change)

    await client.connect()

    print("Updating backlight")
    picture_settings = {'backlight': 100}
    await client.set_current_picture_settings(picture_settings)
    
    await asyncio.sleep(5)
    print("After sleep:", client.picture_settings)

    await client.disconnect()

asyncio.get_event_loop().run_until_complete(runloop())

Output:

State changed: {'contrast': 100, 'backlight': 50, 'brightness': 50, 'color': 70}
Updating backlight
State changed: {'backlight': 100}
After sleep: {'backlight': 100}
State changed: None

Expected:
After calling set_current_picture_settings, it should report all the properties as before.

Actual:
After calling set_current_picture_settings, it reports only the properties that were updated.

@chros73
Copy link

chros73 commented Dec 3, 2021

I've fixed it in my fork, no release yet, so you have to install it from source.
(I also added the ability to specify certain static states and state updates.)

What happened was: the server only sent back the modified value(s), and it has overwritten the original ones.
Solution: merge the sent value(s) with original.

I'm not sure whether other state updates are also affected or not, you can see the full list here.

@bendavid
Copy link
Owner

bendavid commented Dec 3, 2021

I can imagine that what's happening is that the subscription is modified underneath, so I would assume that the other values will not update in this condition.

Not sure what the proper solution is, other than always calling the get api again with the full set of parameters.

@chros73
Copy link

chros73 commented Dec 3, 2021

This should work fine, since we update it with values from the server, so I don't think another call is needed.
Not sure about the rest.

@chros73
Copy link

chros73 commented Dec 4, 2021

I released it (along with couple of other changes).

@nisargjhaveri
Copy link
Author

@bendavid Any update on this? I guess merging the results with an earlier reaponse might work on client side as well, but it'd be much nicer if the case was already handled here the right way. (I'm too new to this to know if there's a better way as well)

If we know the correct solution I might also be able to help of needed. Or if some more investigations I could do.

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

No branches or pull requests

3 participants