Error: Broken pipe about samsungctl HOT 9 CLOSED

@Ape I looked into websockets and played around with it today. Although the library is great its APIs are asyncio/coroutine-based. This means to use it samsungctl will need to be asyncio-based as well. I think it might make sense to use it when samsungctl wants to expose async API but at the moment it might not be feasible as it impacts not only websocket implementation but legacy implementation as well.

Because of this, I think it is more reasonable to pursue WebSocketApp approach and keeping the API as-is.

from samsungctl.

I'm seeing this issue as well in Home Assistant, with websocket method. I'm getting BrokenPipe error when issuing new command after a brief idle period.

I'm curious if this issue has anything to do with websocket-client implementation, in that the usage pattern used by samsungctl is designed for short-live/one off send-receive but not for long-live connection? The reason I'm asking is that on websocket-client's README it shows a different usage pattern for long-live connection using callbacks.

from samsungctl.

It looks like the problem is with websocket-client in that websocket created with websocket-client.create_connection() does not seem to work reliably with long-live connection. After leaving the connection idle for a brief period of time (I can consistently reproduce with a wait time between 30 seconds to 1 minute on KS8000), there will be no response to the next issued command, and the following command will cause broken pipe error.

If I switched the implementation to WebSocketApp which according to the doc is meant to be used with long-live connection, the problem goes away. I tested this with samsungctl's interactive mode as well as with Home Assistant's samsungtv component.

The implication of using WebSocketApp is that it requires spawning up a separate thread just for itself.
Another possible option, although with more ramification, is switching to a different websocket library such as websockets which provide simplified APIs based on asyncio.

@Ape I'm happy to help get this issue fixed but would love to hear what's your thought is on the direction that you prefer.

from samsungctl.

I have changed the port number from 55000 to 8001 and that did solve it for me. Because of your rely @ejel I enabled it again and found that the port number needs to be changed.

from samsungctl.

@ejel I would be happy to accept a pull request with a new websocket implementation if it fixes these issues. Either WebSocketApp or websockets would be good. I might prefer websockets.

from samsungctl.

@Vulpecula-nl Can you share your full command that makes it work? If it's a websocket Samsung TV it should only work with port 8001 and not at all with port 55000 as far as I understand.

from samsungctl.

@ejel What do you want to know? I'm using Domoticz and what I did there was changing the port number from 55000 to 8001.

from samsungctl.

@Ape Turns out that there is no need to switch to WebSocketApps. The current WebSocket class is capable of long-live connection as long as there is a designated thread to keep the underlying socket alive. I posted a PR #88 with the change.

A caveat with this change is that a thread is created even for one-off / short-lived connection. If this is not desirable, one solution is to add a new config option to specify if the connection is intended to be long-lived, and only create a thread in such case. Let me know what you think.

from samsungctl.

@Vulpecula-nl You mentioned in the original post that you reproduced the issue using the interactive mode. Can you share your full command that reproduces the issue and one that fixes the issue?

from samsungctl.