WifiS3 - Non blocking Wifi connect #461

k3ldar · 2025-04-19T14:22:20Z

This PR prevents non blocking connection to wifi, so that normal loop processing can continue whilst waiting for the connection to be established.

Also a potential bug fix in Modem.cpp where MODEM_TIMEOUT was used during begin, effectively ignoring what a user may have set the value to.

Appreciate it's a long shot this getting through, but happy to make adjustments if required.

… timeout value for buf_read in modem. Fix bug where timout is reset to the default of 10000ms when calling begin

…ng for connection to wifi

CLAassistant · 2025-04-19T14:22:27Z

All committers have signed the CLA.

Copilot

Pull request overview

This PR introduces non-blocking WiFi connection functionality to the WiFiS3 library, allowing normal loop processing to continue while waiting for a WiFi connection to be established. The implementation adds new connection states (WL_CONNECTING and WL_DISCONNECTING) and a new isConnected() method to poll connection status, while also fixing a timeout handling issue in the modem initialization.

Key changes:

Modified WiFi.begin() to return immediately with WL_CONNECTING status instead of blocking
Added isConnected() method to check connection progress and timeout
Fixed modem timeout handling in Modem.cpp to preserve user-configured timeout values

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
libraries/WiFiS3/src/WiFiTypes.h	Adds two new WiFi status enumerations: `WL_CONNECTING` and `WL_DISCONNECTING`
libraries/WiFiS3/src/WiFiClient.h	Adds getter method `getConnectionTimeout()` for retrieving the client connection timeout
libraries/WiFiS3/src/WiFi.h	Adds `_start_connection_time` member variable and declares `isConnected()` method for non-blocking connection status polling
libraries/WiFiS3/src/WiFi.cpp	Refactors `begin()` to be non-blocking and implements `isConnected()` to track connection state and timeout
libraries/WiFiS3/src/Modem.h	Adds getter methods for timeout values and introduces `_readTimeout` member variable with documentation updates
libraries/WiFiS3/src/Modem.cpp	Fixes timeout handling to preserve user settings during modem initialization and applies `_readTimeout` for read operations
libraries/WiFiS3/examples/WiFiWebClientSSL/WiFiWebClientSSL.ino	Updates example to demonstrate non-blocking WiFi connection with connection attempt limiting
libraries/WiFiS3/examples/WiFiWebClient/WiFiWebClient.ino	Updates example to demonstrate non-blocking WiFi connection with connection attempt limiting

Comments suppressed due to low confidence (1)

libraries/WiFiS3/examples/WiFiWebClientSSL/WiFiWebClientSSL.ino:118

The client is connecting to port 80 (HTTP) instead of port 443 (HTTPS/SSL), which is inconsistent with the WiFiSSLClient usage and the file's purpose. This should be client.connect(server, 443) for SSL connections.

    clientConnected = client.connect(server, 80);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

libraries/WiFiS3/src/WiFi.h

libraries/WiFiS3/src/WiFi.cpp

libraries/WiFiS3/examples/WiFiWebClient/WiFiWebClient.ino

libraries/WiFiS3/examples/WiFiWebClientSSL/WiFiWebClientSSL.ino

libraries/WiFiS3/examples/WiFiWebClient/WiFiWebClient.ino

libraries/WiFiS3/src/WiFi.cpp

libraries/WiFiS3/src/WiFi.h

libraries/WiFiS3/examples/WiFiWebClientSSL/WiFiWebClientSSL.ino

libraries/WiFiS3/src/WiFiClient.h

libraries/WiFiS3/examples/WiFiWebClientSSL/WiFiWebClientSSL.ino

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 16 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-04T14:33:30Z

libraries/WiFiS3/src/WiFi.h

    void _config(IPAddress local_ip, IPAddress gateway, IPAddress subnet, IPAddress dns1, IPAddress dns2);
    void _sortAPlist(uint8_t num);
    unsigned long _timeout;
+    unsigned long _start_connection_time = millis();


Initializing _start_connection_time with millis() in the header file is problematic. In C++, non-static member initializers are evaluated at object construction time, not at compile time. This means millis() will be called when the global WiFi object is constructed (before main() runs), which could be before the Arduino framework is initialized, potentially returning 0 or an undefined value.

The constructor in WiFi.cpp correctly initializes this to 0. Remove the = millis() initializer from the header and rely solely on the constructor initialization.

Suggested change

unsigned long _start_connection_time = millis();

unsigned long _start_connection_time;

Copilot · 2025-12-04T14:33:30Z

libraries/WiFiS3/src/WiFi.cpp

+      return WL_CONNECT_FAILED;
+   }
+
+   if (millis() - _start_connection_time < _timeout) {


Potential integer overflow issue with timeout check. When _start_connection_time is 0 (as initialized in the constructor before begin() is called), this check will evaluate to true if less than _timeout milliseconds have passed since the device started (millis() < _timeout). This could cause isConnected() to incorrectly return WL_CONNECTING even when no connection attempt has been made.

Consider checking if _start_connection_time is non-zero before performing the timeout check, or resetting it to 0 when a connection succeeds or fails.

Suggested change

if (millis() - _start_connection_time < _timeout) {

// Only check timeout if a connection attempt has started

if (_start_connection_time != 0 && (millis() - _start_connection_time < _timeout)) {

Copilot · 2025-12-04T14:33:30Z

libraries/WiFiS3/examples/WiFiWebClient/WiFiWebClient.ino

+      if (clientConnected) {
+        // if the server's disconnected, stop the client:
+        if (!client.connected()) {
+          Serial.println();
+          Serial.println("disconnecting from server.");
+          client.stop();
+		      status = WL_IDLE_STATUS;
+        }


The condition if (clientConnected) on line 146 is redundant. At this point, we're already inside the if (clientConnected) block from line 132. This condition will always be true here. This redundant check should be removed, and the code inside should be dedented accordingly.

Suggested change

if (clientConnected) {

// if the server's disconnected, stop the client:

if (!client.connected()) {

Serial.println();

Serial.println("disconnecting from server.");

client.stop();

status = WL_IDLE_STATUS;

}

// if the server's disconnected, stop the client:

if (!client.connected()) {

Serial.println();

Serial.println("disconnecting from server.");

client.stop();

status = WL_IDLE_STATUS;

Copilot · 2025-12-04T14:33:31Z

libraries/WiFiS3/src/WiFi.h

+	 * @brief Retrieves the connected state
+	 * 
+	 * @return Current connection state of WL_CONNECT_FAILED, WL_CONNECTED or WL_CONNECTING
+	 */
+    int isConnected();


The method name isConnected() is ambiguous and potentially confusing. It suggests it returns a boolean (is/isn't connected), but it actually returns multiple connection states (WL_CONNECTED, WL_CONNECT_FAILED, WL_CONNECTING). This differs from the existing status() method only in that it also handles the timeout logic. Consider renaming to something more descriptive like getConnectionStatus() or checkConnectionState() to better reflect that it returns various states, not just a boolean.

Suggested change

* @brief Retrieves the connected state

*

* @return Current connection state of WL_CONNECT_FAILED, WL_CONNECTED or WL_CONNECTING

*/

int isConnected();

* @brief Retrieves the current connection status.

*

* @return Current connection state: WL_CONNECT_FAILED, WL_CONNECTED, or WL_CONNECTING.

*/

int getConnectionStatus();

Copilot · 2025-12-04T14:33:31Z

libraries/WiFiS3/examples/WiFiWebClientSSL/WiFiWebClientSSL.ino

+        delay(200);
+        return; 
+    }
+


The WL_CONNECT_FAILED state is not handled. If the WiFi connection fails (either immediately or after timeout), the code doesn't reset the status to WL_IDLE_STATUS to allow retry. Consider adding a check: if (status == WL_CONNECT_FAILED) { Serial.println("WiFi connection failed!"); status = WL_IDLE_STATUS; return; } before the WL_CONNECTED check.

Suggested change

if (status == WL_CONNECT_FAILED) {

Serial.println("WiFi connection failed!");

status = WL_IDLE_STATUS;

return;

}

Copilot · 2025-12-04T14:33:33Z

libraries/WiFiS3/src/Modem.h

+   * @brief Gets the timeout value for reading communication operations.
+   *
+   * @return Can be called to get the specified read timeout value in milliseconds.


The documentation says "Can be called to get the specified read timeout value" but doesn't document what this timeout is used for or how it differs from the general timeout. Consider adding more context like "Gets the timeout value used specifically for read operations during modem communication."

Suggested change

* @brief Gets the timeout value for reading communication operations.

*

* @return Can be called to get the specified read timeout value in milliseconds.

* @brief Gets the timeout value used specifically for read operations during modem communication.

*

* This timeout determines how long the modem will wait for data to be read from the communication interface

* before timing out. It is distinct from the general communication timeout, which applies to overall operations.

* @return The specified read timeout value in milliseconds.

Copilot · 2025-12-04T14:33:33Z

libraries/WiFiS3/examples/WiFiWebClientSSL/WiFiWebClientSSL.ino

 /* -------------------------------------------------------------------------- */
  //Initialize serial and wait for port to open:
-  Serial.begin(115200);
+  Serial.begin(9600);


[nitpick] The serial baud rate has been changed from 115200 to 9600. This change appears unrelated to the non-blocking WiFi connection feature and slows down serial communication significantly. Unless there's a specific reason for this change, consider reverting to 115200 to maintain consistency with other examples and provide faster serial debugging.

Suggested change

Serial.begin(9600);

Serial.begin(115200);

Copilot · 2025-12-04T14:33:34Z

libraries/WiFiS3/examples/WiFiWebClient/WiFiWebClient.ino

+    Serial.println("Connecting to wifi");
+    delay(200);
+  }
+


The WL_CONNECT_FAILED state is not handled. If the WiFi connection fails (either immediately or after timeout), the code doesn't reset the status to WL_IDLE_STATUS to allow retry. Consider adding a check: if (status == WL_CONNECT_FAILED) { Serial.println("WiFi connection failed!"); status = WL_IDLE_STATUS; return; } before the WL_CONNECTED check.

Suggested change

// Handle WiFi connection failure

if (status == WL_CONNECT_FAILED) {

Serial.println("WiFi connection failed!");

status = WL_IDLE_STATUS;

return;

}

Copilot · 2025-12-04T14:33:34Z

libraries/WiFiS3/examples/WiFiWebClient/WiFiWebClient.ino

+          client.stop();
+		      status = WL_IDLE_STATUS;
+        }
+      }


When the server connection fails, the status is not reset. This means the code will stay in the WL_CONNECTED block and keep trying to connect to the server on every loop iteration. While the if (clientConnected) block won't execute, the loop will still continuously attempt to connect without the proper state management. Consider adding an else clause that handles the connection failure and potentially resets the status or adds a delay.

Suggested change

}

}

} else {

// Handle server connection failure

Serial.println("Failed to connect to server.");

status = WL_IDLE_STATUS;

delay(1000); // Add a delay to avoid rapid retries

Copilot · 2025-12-04T14:33:34Z

libraries/WiFiS3/examples/WiFiWebClientSSL/WiFiWebClientSSL.ino

+
+        Serial.println("\nStarting connection to server...");
+
+        if (client.connect(server, 443)) {


WiFiSSLClient connects to server over TLS without any server certificate validation or hostname verification. An on-path attacker (e.g., rogue AP on local network) can perform a MITM and intercept/modify traffic because the client does not verify the peer; fix by loading and enforcing a trusted CA or server certificate/fingerprint before client.connect, e.g., by calling the appropriate API to set a root CA or pinned certificate and validating the hostname.

k3ldar added 2 commits April 19, 2025 13:24

Prevent blocking whilst waiting for a connection and introduce a read…

99df703

… timeout value for buf_read in modem. Fix bug where timout is reset to the default of 10000ms when calling begin

Update client samples to allow normal processing in loop whilst waiti…

7350bbb

…ng for connection to wifi

This comment was marked as resolved.

Sign in to view

k3ldar changed the title ~~Non blocking Wifi connect~~ WifiS3 - Non blocking Wifi connect Apr 20, 2025

per1234 added the topic: code Related to content of the project itself label Sep 22, 2025

k3ldar marked this pull request as draft December 3, 2025 17:22

k3ldar marked this pull request as ready for review December 3, 2025 17:23

Copilot AI review requested due to automatic review settings December 3, 2025 17:23

Copilot started reviewing on behalf of k3ldar December 3, 2025 17:23 View session

Copilot finished reviewing on behalf of k3ldar December 3, 2025 17:26

Copilot AI reviewed Dec 3, 2025

View reviewed changes

k3ldar added 4 commits December 3, 2025 18:59

Fixes from review

a260388

Merge branch 'arduino:main' into main

2d6b21e

Update examples and minor tweaks

61b798f

Merge branch 'main' into main

b233fe0

k3ldar requested a review from Copilot December 4, 2025 14:26

Copilot started reviewing on behalf of k3ldar December 4, 2025 14:27 View session

k3ldar closed this Dec 4, 2025

Copilot finished reviewing on behalf of k3ldar December 4, 2025 14:30

Copilot AI reviewed Dec 4, 2025

View reviewed changes

	unsigned long _start_connection_time = millis();
	unsigned long _start_connection_time;

	if (millis() - _start_connection_time < _timeout) {
	// Only check timeout if a connection attempt has started
	if (_start_connection_time != 0 && (millis() - _start_connection_time < _timeout)) {

+    if (status == WL_CONNECT_FAILED) {
+        Serial.println("WiFi connection failed!");
+        status = WL_IDLE_STATUS;
+        return;
+    }

-   * @brief Gets the timeout value for reading communication operations.
-   *
-   * @return Can be called to get the specified read timeout value in milliseconds.
+   * @brief Gets the timeout value used specifically for read operations during modem communication.
+   *
+   * This timeout determines how long the modem will wait for data to be read from the communication interface
+   * before timing out. It is distinct from the general communication timeout, which applies to overall operations.
+   * @return The specified read timeout value in milliseconds.

-      }
+      }
+    } else {
+      // Handle server connection failure
+      Serial.println("Failed to connect to server.");
+      status = WL_IDLE_STATUS;
+      delay(1000); // Add a delay to avoid rapid retries


		Serial.println("\nStarting connection to server...");

		if (client.connect(server, 443)) {

Uh oh!

WifiS3 - Non blocking Wifi connect #461

WifiS3 - Non blocking Wifi connect #461

Uh oh!

Conversation

k3ldar commented Apr 19, 2025

Uh oh!

CLAassistant commented Apr 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

CLAassistant commented Apr 19, 2025 •

edited

Loading