Skip to content
Prev Previous commit
Next Next commit
Fixes from review
  • Loading branch information
k3ldar committed Dec 3, 2025
commit a2603888e5ed0b8207edc728dad4121b9aa88ef1
3 changes: 2 additions & 1 deletion libraries/WiFiS3/examples/WiFiWebClient/WiFiWebClient.ino
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ void setup() {
}

// 3 second wait for connection
client.setConnectionTimeout(3000);
client.setTimeout(3000);
}

void connectToWifi() {
Expand Down Expand Up @@ -149,6 +149,7 @@ void loop() {
Serial.println();
Serial.println("disconnecting from server.");
client.stop();
status = WL_CONNECTED;
}
}
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ void setup() {
}

// 3 second wait for connection
modem.readTimeout(3000);
modem.setTimeout(3000);
}

void connectToWifi() {
Expand Down Expand Up @@ -115,7 +115,7 @@ void loop() {
printWifiStatus();

Serial.println("\nStarting connection to server...");
clientConnected = client.connect(server, 80);
clientConnected = client.connect(server, 443);

if (clientConnected) {
connectionCount++;
Expand Down
2 changes: 1 addition & 1 deletion libraries/WiFiS3/src/Modem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ void ModemClass::begin(int badurate, int retry){
string res = "";
_serial->flush();

unsigned long modemTimeout = _timeout;
unsigned long modemTimeout = _timeout;
modem.timeout(500);
while(!beginned && retry > 0) {
beginned = modem.write(string(PROMPT(_SOFTRESETWIFI)),res, "%s" , CMD(_SOFTRESETWIFI));
Expand Down
26 changes: 16 additions & 10 deletions libraries/WiFiS3/src/WiFi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
using namespace std;

/* -------------------------------------------------------------------------- */
CWifi::CWifi() : _timeout(10000){
CWifi::CWifi() : _timeout(10000), _start_connection_time(0){
}
/* -------------------------------------------------------------------------- */

Expand Down Expand Up @@ -66,15 +66,21 @@ int CWifi::begin(const char* ssid, const char *passphrase) {

int CWifi::isConnected()
{
if (status() == WL_CONNECTED)
return WL_CONNECTED;

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

return WL_CONNECT_FAILED;
uint8_t current_status = status();

if (current_status == WL_CONNECTED) {
return WL_CONNECTED;
}

if (current_status == WL_CONNECT_FAILED) {
return WL_CONNECT_FAILED;
}

if (millis() - _start_connection_time < _timeout) {
Copy link

Copilot AI Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
return WL_CONNECTING;
}

return WL_CONNECT_FAILED;
}

/* passphrase is needed so a default one will be set */
Expand Down
6 changes: 3 additions & 3 deletions libraries/WiFiS3/src/WiFi.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class CWifi {
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;
unsigned long _start_connection_time;
CAccessPoint access_points[WIFI_MAX_SSID_COUNT];
uint8_t _apsFound = 0;
std::string ssid;
Expand Down Expand Up @@ -449,12 +449,12 @@ class CWifi {

/**
* @brief Sets the timeout value for the WiFi connection.
*
*
* @param `timeout` The timeout value in milliseconds.
*/
void setTimeout(unsigned long timeout);

/*
/**
* @brief Retrieves the connected state
*
* @return Current connection state of WL_CONNECT_FAILED, WL_CONNECTED or WL_CONNECTING
Expand Down
2 changes: 1 addition & 1 deletion libraries/WiFiS3/src/WiFiClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ class WiFiClient : public Client {
* @return Returns the connection timeout in milliseconds
*/
int getConnectionTimeout() {
return _connectionTimeout;
return _connectionTimeout;
}

/**
Expand Down
Loading