Skip to content
This repository has been archived by the owner on May 28, 2024. It is now read-only.

Thoughts about socket connection life cycle #7

Open
bernimoses opened this issue Aug 25, 2020 · 5 comments
Open

Thoughts about socket connection life cycle #7

bernimoses opened this issue Aug 25, 2020 · 5 comments

Comments

@bernimoses
Copy link
Contributor

bernimoses commented Aug 25, 2020

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
Copy link

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

@bernimoses
Copy link
Contributor Author

bernimoses commented Oct 15, 2020

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
Copy link

amonhk commented Oct 16, 2020

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
Copy link
Contributor Author

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
Copy link

amonhk commented Oct 16, 2020

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants