Skip to content

Better fix for "ack timeout 4" client disconnects. - #5

Closed
mmakaay wants to merge 2 commits into
OttoWinter:masterfrom
mmakaay:master
Closed

Better fix for "ack timeout 4" client disconnects.#5
mmakaay wants to merge 2 commits into
OttoWinter:masterfrom
mmakaay:master

Conversation

@mmakaay

@mmakaay mmakaay commented May 5, 2021

Copy link
Copy Markdown

After my first attempt at fixing the client disconnects (#4) got merged
into AsyncTCP, it turned out that there was regression for some devices: the connection stability actually went down instead of up.

After a lot of debugging and discussion with @glmnet (some of the results can be found in the above pull request discussion), we came up with an improved fix for the disconnect issues.

Changed:

The code that checks for ACK timeouts has been simplified in such way, that only two timestamps are now used to determine if an ACK timeout has happened: the time of the last sent packet (this was already recorded), and the time of the last received ACK from the client (this has been added).

Using these timestamps, there is no more need for a separate field to keep track if we are waiting for an ACK or not _pcb_busy). Therefore, this field was completely removed from the code.

While I was at it, I renamed a few variables to make the code easier to read and more consistent.

Results:

I connected Home Assistant plus 8 OTA loggers at the same time, using very verbose logging output. This normally was an easy way to trigger the disconnect errors.

It turned out, this solution runs as solid for me, as when disabling the ACK timeout checks completely using AsyncClient::setAckTimeout(0) (meaning: no more disconnects).

After my first attempt at fixing the client disconnects
(#4) got merged
into AsyncTCP, it turned out that there was regression
for some devices: the connection stability actually went
down instead of up.

After a lot of debugging and discussion with @glmnet (some of
the results can be found in the above pull request discussion),
we came up with an improved fix for the disconnect issues.

**Changed:**

The code that checks for ACK timeouts has been simplified in
such way, that only two timestamps are now used to determine if
an ACK timeout has happened: the time of the last sent packet
(this was already recorded), and the time of the last received
ACK from the client (this has been added).

Using these timestamps, there is no more need for a separate field
to keep track if we are waiting for an ACK or not (`_pcb_busy`).
Therefore, this field was completely removed from the code.

While I was at it, I renamed a few variables to make the code
easier to read and more consistent.

**Results:**

I connected Home Assistant plus 8 OTA loggers at the same time,
using very verbose logging output. This normally was an easy way
to trigger the disconnect errors.

It turned out, this solution runs as solid for me, as when disabling
the ACK timeout checks completely
(using `AsyncClient::setAckTimeout(0)`).
@mmakaay

mmakaay commented May 9, 2021

Copy link
Copy Markdown
Author

Because I am gettng quite a few questions about how to apply this fix manually, here a short recipe for getting this going:

  • Inside the directory where you store your YAML configs, create a subdirectory libs
  • Inside that directory, clone my forked repo: https://github.com/mmakaay/AsyncTCP
  • Now edit libs/AsyncTCP/library.json and change the version line from "1.2.1" to "1.1.1" (because ESPHome currently references 1.1.1 as the AsyncTCP version to use, but that library is currently at version 1.2.1)
  • Edit the YAML file, to include lib_extra_dirs: /absolute/path/to/your/esphome/config/libs in your platformio options (see below for a full example)
  • Build and upload the firmware

Extended onfiguration

esphome:
  platformio_options:
    lib_extra_dirs: /absolute/path/to/your/esphome/config/libs

FAQ: I'm using the Home Assistant plugin, how do I do this?

I run HA from a docker container myself, so this is all second hand information, but some pointers that have worked for others:

Install the Samba plugin in Home Assistant, so you can access the Home Assistant config directory via windows file sharing. The ESPHome config directory can be found in the "config" share, in the subdirectory "esphome". The libs subdirectory can be created in that directory.

Instead of cloning the code using git, you can also download the repo as a ZIP file, and unpack it in the target location (config\esphome\libs).

afbeelding

The value to use for lib_extra_dirs is /config/esphome/libs
Use exactly this path, otherwise it will not work.
Also remember to update the library.json file to use version 1.1.1, as mentioned above.

FAQ: How can I be sure the patched AsyncTCP is being used?

I don't know a really good way to check if the build will actually pick up the cloned version of the library. What I normally do (this might seem very hacky for those in the know), is enforce some syntax error in libs/AsyncTCP/src/AsyncTCP.cpp (e.g. add a line saying "lalalalala") and then compile the firmware. When you get an error about this line, you'll know the patched version of the code is in use.

@mmakaay

mmakaay commented May 23, 2021

Copy link
Copy Markdown
Author

I'll close this pull request, since the code was merged in the new esphome/AsyncTCP repo, which is now used in the standard ESPHome releases.

@mmakaay mmakaay closed this May 23, 2021
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

Successfully merging this pull request may close these issues.

1 participant