search

JeffLIrion / python-androidtv

Communicate with an Android TV or Fire TV device via ADB over a network.
MIT License
165 stars 57 forks source link

Add transport_timeout_s parameter for setup and adb_connect methods #304

Closed ollo69 closed 2 years ago

ollo69 commented 2 years ago

In most case during initial connection to my FireTV devices I receive back the error:

[androidtv.adb_manager.adb_manager_async] Couldn't connect to 192.168.10.18:5555.  TcpTimeoutException: Connecting to 192.168.10.18:5555 timed out (1.0 seconds)

With this PR it will be possible to increase a little bit the transport_timeout used in the connect method that is currently hard coded to 1 second.

JeffLIrion commented 2 years ago

I don't know why this exception is happening. Here's where it is in the code: https://github.com/JeffLIrion/adb_shell/blob/bf4d348e3aa0999b24976de9bac442b0e180a27e/adb_shell/transport/tcp_transport_async.py#L75-L79

I think 1 second should be plenty of time. Have you tested your changes and verified that allowing more time avoids the exception?

ollo69 commented 2 years ago

Error normally occurs only during initial setup. I think that could be related to the hw of the fire TV stick that probably is not so fast. It fails on first connect, second retry that is normally fired after few seconds by HA ends without error. And yes, I tested changes in HA and just increasing the timeout to 2 seconds seems to solve the issue.

JeffLIrion commented 2 years ago

Just to be clear, when you say "initial setup," are you talking about config flow or at every HA restart?

Also, a simpler change would be to increase the hard-coded value from 1.0 to, say, 5.0. Of course, adding a parameter allows for greater flexibility. I think either approach is probably fine.

ollo69 commented 2 years ago

Talking about every HA restart, so when setup is called. I agree with you that increase the hard-coded value is simpler, but I wouldn't change current default value. I also defined a max value (5 sec) to avoid set this timeout to excessive high value.

JeffLIrion commented 2 years ago

It would still be good to get to the bottom of this. I would think that 1 second would be plenty of time and that there must be some other issue. But if changing it to 2 seconds fixed the problem for you, that strongly suggests the problem really was as simple as 1 second not being enough time.

Let me know if you want me to make a new release.

ollo69 commented 2 years ago

It would still be good to get to the bottom of this. I would think that 1 second would be plenty of time and that there must be some other issue. But if changing it to 2 seconds fixed the problem for you, that strongly suggests the problem really was as simple as 1 second not being enough time.

Let me know if you want me to make a new release.

That's the idea, This is not a big or urgent issue, because at the end I just have a couple of Warning message during HA restart, but I think that having a new version to be able to increase the initial timeout to 2 seconds will be better and I don't see any possible side-effect on this.