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

client.connected() bug #4728

Closed
6 tasks done
aerlon opened this issue May 14, 2018 · 2 comments
Closed
6 tasks done

client.connected() bug #4728

aerlon opened this issue May 14, 2018 · 2 comments
Assignees

Comments

@aerlon
Copy link
Contributor

aerlon commented May 14, 2018

Basic Infos

  • This issue complies with the issue POLICY doc.
  • I have read the documentation at readthedocs and the issue is not addressed there.
  • I have tested that the issue is present in current master branch (aka latest git).
  • I have searched the issue tracker for a similar issue.
  • If there is a stack dump, I have decoded it.
  • I have filled out all fields below.

Platform

  • Hardware: ESP-12

  • Core Version:
    SDK:2.2.1(cfd48f3)/Core:win-2.5.0-dev/lwIP:2.0.3(STABLE-2_0_3_RELEASE/glue:arduino-2.4.1-7-g2b827f8)
    SDK:2.2.1(cfd48f3)/Core:win-2.5.0-dev/lwIP:1.4.0rc2
    SDK:2.2.1(cfd48f3)/Core:win-2.4.1/lwIP:2.0.3(STABLE-2_0_3_RELEASE/glue:arduino-2.4.1)
    SDK:2.2.1(cfd48f3)/Core:win-2.4.1/lwIP:1.4.0rc2

  • Development Env: Arduino IDE

  • Operating System: Windows

Settings in IDE

  • Module: NodeMCU 1.0
  • Flash Mode: dio
  • Flash Size: 4MB
  • lwip Variant: v2 Lower Memory OR v1.4
  • Reset Method: nodemcu
  • Flash Frequency: 40Mhz
  • CPU Frequency: 80Mhz
  • Upload Using: SERIAL
  • Upload Speed: 115200

Problem Description

In the latest master branch, the station in the file ClientConnectedBug.ino (download below) seems to randomly loose connection to the AP server while waiting for curr_client.available() to indicate a server response (WiFiClient class is used to create a TCP connection to the server, so curr_client is the server here). This was not the case in Arduino core for ESP8266 version 2.4.1 and 2.3.0. The problem seems to be that !curr_client.connected() at line 50 in ClientConnectedBug.ino now sometimes evaluates to true, whereas before it did not.

Instructions for reproducing bug:

  1. Download and unzip the .ino files in ClientConnectedBug.zip
  2. Make sure you are using Arduino core for ESP8266 version 2.4.1.
  3. Use lwIP2 or lwIP1.4.
  4. Add AP1.ino to one ESP8266 and ClientConnectedBug.ino to another.
  5. Observe serial output from ClientConnectedBug.ino.
  6. Update Arduino core for ESP8266 to latest master branch.
  7. Restart IDE and re-upload ClientConnectedBug.ino.
  8. Observe serial output from ClientConnectedBug.ino.
@d-a-v
Copy link
Collaborator

d-a-v commented May 17, 2018

This behaviour (sometimes yes, sometimes not) is expected.
With this added in your sketch,

    if (curr_client.available())
    {
      Serial.printf("Still, we have %d bytes to read.\r\n", curr_client.available());
    }
    return false;

I get this message always displayed

22:37:39.777 -> connecting to 192.168.4.1
22:37:39.810 -> 
22:37:39.810 -> We did not run out of waiting time.
22:37:39.810 -> We are not disconnected from the network.
22:37:39.810 -> We have curr_client.available().
22:37:39.810 -> But the client is no longer connected.
22:37:39.810 -> Still, we have 22 bytes to read.
22:37:39.843 -> Still, we can read the data the server sent to the client: 
22:37:39.843 -> 21Response #378from AP1
22:37:39.843 -> 
22:37:39.843 -> closing connection
22:37:39.843 -> 
22:37:39.843 -> wait 2 sec...

It is expected because the server sends its data, then flush() meaning it ensures that the data are received by its client, then immediately closes. By the time the client gets the data, the server may already have closed the connection, so technically the client is not anymore connected.

And yes, this behavior was not encountered with previous versions.
That's because of #4626 which changes connected() to the way described above.

You should test available() to know whether data is to be read, and connected() to know whether peer is still connected (= we can send data to server). The connection is to be discarded when (available() || connected()) is false.

@devyte
Copy link
Collaborator

devyte commented May 18, 2018

@aerlon I am not familiar with our mesh lib, except for glancing at the code in passing, but it is possible that the code could need updatiing due to the previous comment. In that case, please continue discussion in the mesh update pr.
Closing this one.

@devyte devyte closed this as completed May 18, 2018
devyte pushed a commit that referenced this issue Aug 1, 2018
* Make mesh network actually usable. Make mesh network use static IP during initial connection to speed up connection time. Add separate handlers for requests and responses. Add network password. Provide more detailed code example. Add optional verbose mode. Improve comments. Add readme file.

* Fix compiler warnings. Fix code style of HelloMesh.ino to avoid upsetting Travis.

* Remove stray spaces.

* Make mesh network WiFi password settable via the ESP8266WiFiMesh constructor. Make use of static IP optional by moving static IP initialization code to setStaticIP method. Increase scanning interval from one to two seconds in the HelloMesh.ino example to increase chances of successful connections. Update comments. Update README.rst.

* Increase specificity in the conditions of the waitForClientTransmission method (renamed from waitForClient) to avoid issues related to #4626 , #4728 and #4754 in the future.

* Improve most parts of the library to achieve better performance and greatly increase flexibility.

Changes:
* Make WiFi-connection related variables static to allow for the use of multiple ESP8266WiFiMesh instances on a single node (useful e.g. when communicating with several different mesh networks).
* Make it possible to choose AP port, which is helpful when using multiple ESP8266WiFiMesh AP:s on a single node.
* Add user-customizable network filter.
* Make activation of own AP optional for each mesh node.
* Add ways to change mesh network name and node id for existing ESP8266WiFiMesh instances.
* Add verboseModePrint method to clean up the code.
* Add reactivation of static IP after successful data transfers to speed up re-connection attempts.
* Add empty_IP constant which can be used to check if static IP is disabled for a ESP8266WiFiMesh instance.
* Remove the WiFiClient _client class variable in ESP8266WiFiMesh since there is no need to save _client in the class instance.
* Add transmission status as a return value from attemptTransmission.
* Pass calling ESP8266WiFiMesh instance pointer to callback functions to allow for greater range of actions in callbacks.
* Make transmission message a class variable to allow it to be stored in the class and accessed from callbacks.
* Add getters for mesh name and node id to ESP8266WiFiMesh.
* Add getter and setter for networkFilter to ESP8266WiFiMesh.
* Increase range of available node_id:s by changing the type to String and adding functions to convert between String and uint64_t using a customizable radix between 2 and 36.
* Make it possible to connect to several nodes during each attemptTransmission call.
* Add static connection_queue and latest_transmission_outcomes vectors to the ESP8266WiFiMesh class, a NetworkInfo class and a TransmissionResult class to aid in bookkeeping when connecting to several AP:s during one attemptTransmission call.
* Make wifi_channel and BSSID optional when connecting to an AP (though excluding them will slow down the connection process).
* Add optional scan and static ip optimizations available in Arduino core for ESP8266 version 2.4.2.
* Add functions to check lwIP version in order to enable WiFi optimizations only available with lwIP2.
* Add concluding_disconnect, initial_disconnect and no_scan options to the attemptTransmission method.
* Update documentation.

* Improve README.rst formatting.

* Further improve README.rst.

* Even further improve README.rst.

* Make source code comments Doxygen compatible. Improve README file and change its file format to .md.

* Add temporary compatibility layer to ensure backwards compatibility with the old mesh network library API until the next major core release (2.5.0).

* Polish documentation slightly.

* Add scan_all_wifi_channels option to attemptTransmission method.

* - Add getter and setter for the WiFi channel of a ESP8266WiFiMesh instance.
- Separate methods for changing mesh name and node id from AP control methods.
- Add methods getAPController and isAPController to better handle situations when multiple ESP8266WiFiMesh instances take turns to be in control of the AP.
- Create separate UtilityMethods.cpp file for utility methods.
- Improve code efficiency and robustness, e.g. by passing arguments by reference instead of by value for non-POD types and employing typedefs.
- Update README.md.

* Make the code more stylish.

* Update README.md with the new ESP8266WiFiMesh constructor documentation.

* Make attemptScan method in CompatibilityLayer use reference as argument.

* Make it possible to use const String as argument to attemptScan.

* - Make code use camelCase instead of snake_case.
- Improve documentation.

* Rename Uint64ToString to uint64ToString and StringToUint64 to stringToUint64, since they are methods.
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

No branches or pull requests

3 participants