Skip to content
Next Next commit
Prevent blocking whilst waiting for a connection and introduce a read…
… timeout value for buf_read in modem.

Fix bug where timout is reset to the default of 10000ms when calling begin
  • Loading branch information
k3ldar committed Apr 19, 2025
commit 99df70332697256dbc82b690ed477f57e9270efd
24 changes: 13 additions & 11 deletions libraries/WiFiS3/src/Modem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,14 @@ void ModemClass::begin(int badurate, int retry){
_serial->begin(badurate);
string res = "";
_serial->flush();

unsigned long modemTimeout = _timeout;
modem.timeout(500);
while(!beginned && retry > 0) {
beginned = modem.write(string(PROMPT(_SOFTRESETWIFI)),res, "%s" , CMD(_SOFTRESETWIFI));
retry -= 1;
}
modem.timeout(MODEM_TIMEOUT);
modem.timeout(modemTimeout);
}
}

Expand Down Expand Up @@ -186,7 +188,7 @@ ModemClass::ParseResult ModemClass::buf_read(const string &prompt, string &data_
unsigned long start_time = millis();
while(state != at_parse_state_t::Completed) {

if(millis() - start_time > _timeout) {
if(millis() - start_time > _readTimeout) {
res = Timeout;
break;
}
Expand Down Expand Up @@ -293,15 +295,15 @@ ModemClass::ParseResult ModemClass::buf_read(const string &prompt, string &data_
* in data_res contains the length of the next token
*/

if(c == '|') { // sized read, the previous parameter is the length
sized_read_size = atoi(data_res.c_str());
data_res.clear();
if (sized_read_size != 0) {
state = at_parse_state_t::Sized;
} else {
state = at_parse_state_t::Res;
}
} else if(c == '\r') {
if(c == '|') { // sized read, the previous parameter is the length
sized_read_size = atoi(data_res.c_str());
data_res.clear();
if (sized_read_size != 0) {
state = at_parse_state_t::Sized;
} else {
state = at_parse_state_t::Res;
}
} else if(c == '\r') {
state = at_parse_state_t::ResWaitLF;
} else if(c == '\n') {
state = at_parse_state_t::Res;
Expand Down
182 changes: 102 additions & 80 deletions libraries/WiFiS3/src/Modem.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,89 +17,89 @@

/**
* @brief A class that provides methods to interact with a modem.
*
*
* This class is responsible for providing an interface to communicate with
* a modem through serial communication. It includes methods for initialization,
* a modem through serial communication. It includes methods for initialization,
* sending and receiving data, and handling modem configurations.
*/
class ModemClass {

public:
/**
* @brief Constructor for the ModemClass, which initializes the modem with the specified transmit (TX) and receive (RX) pins.
*
* @param Initializes an instance of the ModemClass class with
* specific transmit `tx` and receive `rx` pins for communication.
*/
ModemClass(int tx, int rx);

/**
* @brief Constructor for the ModemClass, which initializes the modem with the specified UART interface.
*
* @param `_serial` is a pointer to the UART object that will be used for communication with the modem.
*/
ModemClass(UART * _serial);

/**
* @brief Destructor for ModemClass.
*/
~ModemClass();


/**
* @brief Initializes the modem communication with a specified baud rate.
*
* @param[in] `badurate` sets the baud rate for the serial connection.
*/
void begin(int badurate = 115200, int retry = 3);

/**
* @brief Ends the modem communication.
*/
void end();


/**
* @brief Sends a formatted command string to a device and stores the response.
*
* This function formats a command string using the provided format and arguments,
* sends it to a device, and waits for a response, which is stored in the `str` string.
*
* @param `cmd` A string representing the command to be sent to the device.
* @param `str` A reference to a string that will hold the device's response.
* @param `fmt` A format string for constructing the command.
*
* @return `true` if the command was successfully sent and a response was received,
* `false` otherwise.
*/
bool write(const std::string &cmd, std::string &str, const char * fmt, ...);

/**
* @brief Used to send a command to the modem without waiting for a response.
*
* @param It takes a command string `cmd`, a string `str` where the response will be stored,
* and a format string `fmt` along with additional arguments.
*/
void write_nowait(const std::string &cmd, std::string &str, const char * fmt, ...);

/**
* @brief Sends binary data directly to the modem without any processing or interpretation.
*
* @param It takes a pointer to the binary `data` and the `size` of the data as arguments.
* Used for sending raw binary commands or data to the modem for operations that
* require direct communication without any additional formatting or parsing.
*/
bool passthrough(const uint8_t *data, size_t size);

/**
* @brief Disables automatic trimming of results for one operation.
*
* This function disables the automatic trimming of results for one operation.
* After it is called, the results will not be trimmed automatically until
* the function is called again.
*/
void avoid_trim_results() {
/* one shot - it works only 1 time the it is necessary to call again this
/**
* @brief Constructor for the ModemClass, which initializes the modem with the specified transmit (TX) and receive (RX) pins.
*
* @param Initializes an instance of the ModemClass class with
* specific transmit `tx` and receive `rx` pins for communication.
*/
ModemClass(int tx, int rx);

/**
* @brief Constructor for the ModemClass, which initializes the modem with the specified UART interface.
*
* @param `_serial` is a pointer to the UART object that will be used for communication with the modem.
*/
ModemClass(UART * _serial);

/**
* @brief Destructor for ModemClass.
*/
~ModemClass();


/**
* @brief Initializes the modem communication with a specified baud rate.
*
* @param[in] `badurate` sets the baud rate for the serial connection.
*/
void begin(int badurate = 115200, int retry = 3);

/**
* @brief Ends the modem communication.
*/
void end();


/**
* @brief Sends a formatted command string to a device and stores the response.
*
* This function formats a command string using the provided format and arguments,
* sends it to a device, and waits for a response, which is stored in the `str` string.
*
* @param `cmd` A string representing the command to be sent to the device.
* @param `str` A reference to a string that will hold the device's response.
* @param `fmt` A format string for constructing the command.
*
* @return `true` if the command was successfully sent and a response was received,
* `false` otherwise.
*/
bool write(const std::string &cmd, std::string &str, const char * fmt, ...);

/**
* @brief Used to send a command to the modem without waiting for a response.
*
* @param It takes a command string `cmd`, a string `str` where the response will be stored,
* and a format string `fmt` along with additional arguments.
*/
void write_nowait(const std::string &cmd, std::string &str, const char * fmt, ...);

/**
* @brief Sends binary data directly to the modem without any processing or interpretation.
*
* @param It takes a pointer to the binary `data` and the `size` of the data as arguments.
* Used for sending raw binary commands or data to the modem for operations that
* require direct communication without any additional formatting or parsing.
*/
bool passthrough(const uint8_t *data, size_t size);

/**
* @brief Disables automatic trimming of results for one operation.
*
* This function disables the automatic trimming of results for one operation.
* After it is called, the results will not be trimmed automatically until
* the function is called again.
*/
void avoid_trim_results() {
/* one shot - it works only 1 time the it is necessary to call again this
funtion */
trim_results = false;
}
Expand All @@ -109,9 +109,9 @@ class ModemClass {
* to be read is considered for processing.
*/
void read_using_size() {
// read_by_size = true; // deprecated
// read_by_size = true; // deprecated
}

bool beginned;

/* Calling this function with no argument will enable debug message to be printed
Expand Down Expand Up @@ -143,11 +143,32 @@ class ModemClass {

/**
* @brief Sets the timeout value for communication operations.
*
*
* @param Can be called with a specified timeout value in milliseconds.
*/
void timeout(size_t timeout_ms) {_timeout = timeout_ms;}

/**
* @brief Gets the timeout value for communication operations.
*
* @return Can be called to get the specified timeout value in milliseconds.
Comment on lines +152 to +154
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.

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

Suggested change
* @brief Gets the timeout value for communication operations.
*
* @return Can be called to get the specified timeout value in milliseconds.
* @brief Gets the timeout value for modem communication operations.
*
* @return The current timeout value (in milliseconds) used for modem communication operations.
Copilot uses AI. Check for mistakes.
*/
unsigned long getTimeout() { return _timeout; }

/**
* @brief Sets the timeout value for reading communication operations.
*
* @param Can be called with a specified read timeout value in milliseconds.
*/
void readTimeout(size_t timeout_ms) {_readTimeout = timeout_ms;}

/**
* @brief Gets the timeout value for reading communication operations.
*
* @return Can be called to get the specified read timeout value in milliseconds.
Comment on lines +166 to +168
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.

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 uses AI. Check for mistakes.
*/
unsigned long getReadTimeout() { return _readTimeout; }

private:
enum ParseResult {
Ok,
Expand All @@ -160,6 +181,7 @@ class ModemClass {
bool delete_serial;
UART * _serial;
unsigned long _timeout;
unsigned long _readTimeout = MODEM_TIMEOUT;
uint8_t tx_buff[MAX_BUFF_SIZE];
bool trim_results;
Stream * _serial_debug;
Expand Down
23 changes: 16 additions & 7 deletions libraries/WiFiS3/src/WiFi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,22 @@ int CWifi::begin(const char* ssid, const char *passphrase) {
}
}

unsigned long start_time = millis();
while(millis() - start_time < _timeout){
if(status() == WL_CONNECTED) {
return WL_CONNECTED;
}
}
return WL_CONNECT_FAILED;
_start_connection_time = millis();
return WL_CONNECTING;
}


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

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

return WL_CONNECT_FAILED;
}

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


/*
* @brief Retrieves the connected state
*
* @return Current connection state of WL_CONNECT_FAILED, WL_CONNECTED or WL_CONNECTING
*/
int isConnected();
Comment on lines +458 to +462
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.

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

/**
Expand Down
11 changes: 10 additions & 1 deletion libraries/WiFiS3/src/WiFiClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,16 @@ class WiFiClient : public Client {
void setConnectionTimeout(int timeout) {
_connectionTimeout = timeout;
}


/**
* @brief Retrieves the connection timeout period
*
* @return Returns the connection timeout in milliseconds
*/
int getConnectionTimeout() {
return _connectionTimeout;
}

/**
* @brief Declares WiFiServer as a friend class.
*
Expand Down
4 changes: 3 additions & 1 deletion libraries/WiFiS3/src/WiFiTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ typedef enum {
WL_DISCONNECTED,
WL_AP_LISTENING,
WL_AP_CONNECTED,
WL_AP_FAILED
WL_AP_FAILED,
WL_CONNECTING,
WL_DISCONNECTING
} wl_status_t;

/* Encryption modes */
Expand Down