Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Successive calls to readBytes() trigger a timeout. #4882

Closed
jfjlaros opened this issue Mar 2, 2021 · 14 comments
Closed

Successive calls to readBytes() trigger a timeout. #4882

jfjlaros opened this issue Mar 2, 2021 · 14 comments
Assignees
Labels
Area: BT&Wifi BT & Wifi related issues Status: Solved
Milestone

Comments

@jfjlaros
Copy link

jfjlaros commented Mar 2, 2021

Hardware:

Board: ESP32 Dev Module
Core Installation version: 1.0.4, 1.0.5
IDE name: Arduino IDE
Flash Frequency: n.a.
PSRAM enabled: n.a.
Upload Speed: n.a.
Computer OS: Debian 10

Description:

Sucsessive calls to readBytes() trigger a timeout in de example below. Is there a way to avoid this, or is this perhaps a bug in one of the core libraries?

When calling readBytes() twice in succession, the second call takes a lot longer than the first one. This behaviour seems to be occurring only when using the WiFi interface, both the Bluetooth and the Serial interface are not affected. This behaviour seems to be specific to this setup, other WiFi modules are not affected.

Sketch:

#include <WiFi.h>

#include "config.h" // SSID, PASS and PORT.


/* Timing measurement functions. */
#define SLOTS 100

unsigned long _last[SLOTS];
size_t _index = 0;


void _set(void) {
  _last[_index] = millis();
  _index++;
}

void _delta(char* label) {
  _index--;
  char buf[80];
  sprintf(buf, "%04i %s", millis() - _last[_index], label);
  Serial.println(buf);
}
/* End timing measurement functions. */


WiFiServer server(PORT);


void setup(void) {
  Serial.begin(9600);
  WiFi.begin(SSID, PASS);

  while (WiFi.status() != WL_CONNECTED) {
    delay(500);
  }

  server.begin();
}

void loop(void) {
  WiFiClient client = server.available();

  if (client) {
    while (client.connected()) {
      if (client.available()) {
        uint8_t c;
        _set();
        client.readBytes(&c, sizeof(uint8_t));
        _delta("read 1");

        /* Offending snippet. */
        _set();
        client.readBytes(&c, sizeof(uint8_t));
        _delta("read 2");
        /* End offending snippet. */

        _set();
        client.write(c);
        _delta("write");
      }
    }
    client.stop();
  }
}

Host code:

from serial import serial_for_url
from time import time


# Timing measurement functions.
_last = []


def _ms():
    return int(round(time() * 1000))


def _set():
    _last.append(_ms())


def _delta(label):
    print('{:04d} {}'.format(_ms() - _last.pop(-1), label))
# End timing measurement functions.


def test_raw(calls):
    connection = serial_for_url("socket://192.168.21.53:1025")
    _set()
    for _ in range(calls):
        _set()
        connection.write(b'\x00')
        _delta("write 1")

        # Offending snippet.
        _set()
        connection.write(b'\x00')
        _delta("write 2")
        # End offending snippet.

        _set()
        connection.read()
        _delta("read")
    _delta("total")


test_raw(3);

Debug Messages:

When we run the code as is, we see the following output:

Arduino     | Host
------------+-------------
0000 read 1 | 0000 write 1
0244 read 2 | 0000 write 2
0001 write  | 0254 read
0000 read 1 | 0001 write 1
0248 read 2 | 0000 write 2
0002 write  | 0252 read
0000 read 1 | 0001 write 1
0242 read 2 | 0000 write 2
0001 write  | 0249 read
            | 0757 total

Notice that read 2 takes a long time as seen from the perspective of the Arduino, while read takes a long time as seen from the perspective of the host.

When the offending snippets are commented out, we see the following output:

Arduino     | Host 
------------+-------------
0000 read 1 | 0001 write 1
0001 write  | 0006 read
0000 read 1 | 0000 write 1
0001 write  | 0005 read
0000 read 1 | 0000 write 1
0001 write  | 0004 read
            | 0016 total

Which is expected behaviour.

@felmue
Copy link

felmue commented Mar 5, 2021

Hello @jfjlaros

I am just guessing here, but I think the client.flush(); after the first read removes the data of the second write and then the second read runs into a timeout as nothing is there anymore to read.

From the description of WiFiClient.flush()

flush()

Discard any bytes that have been written to the client but not yet read.

Thanks
Felix

@jfjlaros
Copy link
Author

jfjlaros commented Mar 5, 2021

Hi @felmue thank you for looking into this.

Sorry, the flush() call should not be present in the sketch, I updated the issue.

Without this line, the problem persists. I can also confirm that no data is discarded (with or without the flush() call).

@jfjlaros jfjlaros changed the title Sucsessive calls to readBytes() trigger a timeout. Successive calls to readBytes() trigger a timeout. Mar 8, 2021
@stale
Copy link

stale bot commented Jun 20, 2021

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status: Stale Issue is stale stage (outdated/stuck) label Jun 20, 2021
@jfjlaros
Copy link
Author

Bump for stale bot. This issue is not resolved yet.

@stale
Copy link

stale bot commented Jun 26, 2021

[STALE_CLR] This issue has been removed from the stale queue. Please ensure activity to keep it openin the future.

@stale stale bot removed the Status: Stale Issue is stale stage (outdated/stuck) label Jun 26, 2021
@stale
Copy link

stale bot commented Aug 28, 2021

[STALE_SET] This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Status: Stale Issue is stale stage (outdated/stuck) label Aug 28, 2021
@jfjlaros
Copy link
Author

Bump for stale bot. This issue is not resolved yet.

@stale
Copy link

stale bot commented Aug 28, 2021

[STALE_CLR] This issue has been removed from the stale queue. Please ensure activity to keep it openin the future.

@stale stale bot removed the Status: Stale Issue is stale stage (outdated/stuck) label Aug 28, 2021
@VojtechBartoska
Copy link
Contributor

Hello @jfjlaros , sorry for late reply. Are you able to test your issue on development version 2.0.3-RC1 to check if this is still valid? You can take a look on Docs where is explained how to choose development release version in Arduino IDE.

@VojtechBartoska VojtechBartoska added the Resolution: Awaiting response Waiting for response of author label Mar 30, 2022
@jfjlaros
Copy link
Author

Hi @VojtechBartoska ,

I tested version 2.0.3-RC1, but I see no difference in behaviour.

@VojtechBartoska
Copy link
Contributor

Hi, Thanks for testing. I'm adding it to the roadmap and we will take a look on this more.

@P-R-O-C-H-Y P-R-O-C-H-Y self-assigned this May 30, 2022
@P-R-O-C-H-Y P-R-O-C-H-Y moved this from Todo to In Progress in Arduino ESP32 Core Project Roadmap May 30, 2022
@VojtechBartoska VojtechBartoska added this to the 2.0.4 milestone May 30, 2022
@P-R-O-C-H-Y P-R-O-C-H-Y added Status: In Progress Issue is in progress and removed Status: Test needed Issue needs testing labels May 31, 2022
@P-R-O-C-H-Y
Copy link
Member

P-R-O-C-H-Y commented Jun 6, 2022

Hi, i have simulated your issue between 2 ESP32s. I have used your server code and rewrite the host for ESP32.

        /* Offending snippet. */
        _set();
        client.readBytes(&c, sizeof(uint8_t));
        _delta("read 2");
        /* End offending snippet. */

This part takes 0000-0001 ms, but you can read wrong data. You need to wait till the data are available for every read with
if (client.available()), than you will read right data every time. So there is no timeout issue, but the data to be available for reading (read num. 2) takes sometimes from 250-600ms. Gonna take deeper look if there is something to speed it up.

@P-R-O-C-H-Y
Copy link
Member

@jfjlaros Your issue can be solved by calling client.setNoDelay(true); after successful client connection. Default value is FALSE.

And as I mentioned in comment above, always read only if client.available() is true.

Repository owner moved this from In Progress to Done in Arduino ESP32 Core Project Roadmap Jun 7, 2022
@P-R-O-C-H-Y P-R-O-C-H-Y added Area: BT&Wifi BT & Wifi related issues Status: Solved and removed Status: In Progress Issue is in progress labels Jun 7, 2022
@jfjlaros
Copy link
Author

jfjlaros commented Jun 7, 2022

After some more experimenting, the solution seems to be to set the NODELAY option on the sending side instead of the receiving side in this particular case. Adding the following line to the Python code seems to do the trick:

connection._socket.setsockopt(IPPROTO_TCP, TCP_NODELAY, 1)

I suspect setNoDelay will be needed when the roles are reversed.

Thank you very much for pointing me in the right direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: BT&Wifi BT & Wifi related issues Status: Solved
Projects
Development

No branches or pull requests

4 participants