google / python-temescal

Python control for LG speaker systems
Apache License 2.0
22 stars 17 forks source link

Thoughts about socket connection life cycle #7

Open bernimoses opened 4 years ago

bernimoses commented 4 years ago

This are just some thoughts about the socket connection life cycle. It would be nice to get some feedback. ;)

Currently every time the connection gets closed (ir remote, tcp timeout) we try to reopen it immediately. This causes (at least for me) to wake up the soundbar from standby. I found that if i turn the soundbar off via the ir remote it closes the tcp connection which in turn would immediately turn it back on (as of version 0.2). Edit: Not opening a connection turns it back on but sending data. This is just from my observations and i have to confirm if this is always the case.

For my use case (homeassistant, hass) this is even more pronounced since the current hass component uses polling. Since a already open socket would update the components state itself, i tried to disable the polling. But this caused (as of version 0.1) the connection to drop completely which makes the hass entity kinda like a "zombie process". I'm not going into more detail here since this is mostly hass related and there need to be changes made to the hass component to implement proper uuid handling.

Ok, now my proposal. Instead of reopening the connection after receiving the null byte we close the client socket. Edit: It seems that this part is not needed since only sending data would turn the soundbar back on. If any library method is called afterwards we reopen the socket connection (if it is closed). This means that we don't get status updates if the soundbar is turned on (wake up from standby) from outside (ir remote, other socket connection) but in turn we allow it to go to sleep.

For the hass part this would mean we use the zeroconf announce (the soundbar sends on broadcast/start) to create the entity and reopen the socket on the same event to get the current status. Maybe zeroconf would also be interesting to implement here in case we get a announce for "wake up from standby" (didn't check yet).

Is there a part of this you would consider adding to this library in one or another (pull request) way?

lasconic commented 4 years ago

Did you check if you get a "wake up from standby" via zeroconf ?

bernimoses commented 4 years ago

As far as i tested it does not. Only after it was completely powered off or if something on the network actively searches for it. But for standby this is not really relevant anymore since the new socket implementation (version > 0.2) has fixed the connection problems for me.

I deactivated the polling in hass (via custom_component) and now it does not wake up and does not loose the connection. So if there is no interest in adding zeroconf (i'm not sure how to implement it since from my observations the published UUID changes regularly) to reestablish the socket after a disconnect (network down, power off, ...) i would say this discussion can be closed.

amonhk commented 4 years ago

I deactivated the polling in hass (via custom_component) and now it does not wake up

good, what have you changed to disable polling?

I have modified this code line to have a unique_id that does not change.

bernimoses commented 4 years ago

good, what have you changed to disable polling?

Just add this here.

...
    @property
    def should_poll(self):
        """No polling needed."""
        return False
...

I have modified this code line to have a unique_id that does not change.

Yeah added something there too. Would be interested what you used as uuid, because i wanted to make a pull request with some of my changes. But i think we should take this conversation somewhere else since this is not library specific.

amonhk commented 4 years ago

Thank you! it seems that by disabling polling everything works... to continue the discussion there is this open issue in home-assistant