From 5c49f161fb1e23b544e3a0591772812fd085aac3 Mon Sep 17 00:00:00 2001 From: David Alonso de la Torre Date: Sat, 30 Dec 2023 11:47:20 +0100 Subject: [PATCH 1/3] Increase AT timeout to 10s in AT_CellularSMS::send_sms For some devices sending can be slow (as an example see SIM800, it can be up to 60s), command is being run properly but default timeout is returning an invalid error. See https://www.elecrow.com/wiki/images/2/20/SIM800_Series_AT_Command_Manual_V1.09.pdf --- .../cellular/source/framework/AT/AT_CellularSMS.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp b/connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp index 79ebc908733..8820c731c31 100644 --- a/connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp +++ b/connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp @@ -420,6 +420,7 @@ nsapi_size_or_error_t AT_CellularSMS::send_sms(const char *phone_number, const c } _at.lock(); + _at.set_at_timeout(10s); int write_size = 0; @@ -437,6 +438,7 @@ nsapi_size_or_error_t AT_CellularSMS::send_sms(const char *phone_number, const c // sending can be cancelled by giving character (IRA 27). _at.cmd_start(ESC); _at.cmd_stop(); + _at.restore_at_timeout(); _at.unlock(); return write_size; } @@ -482,6 +484,7 @@ nsapi_size_or_error_t AT_CellularSMS::send_sms(const char *phone_number, const c pdu_str = create_pdu(phone_number, message + i * concatenated_sms_length, pdu_len, sms_count, i + 1, header_len); if (!pdu_str) { + _at.restore_at_timeout(); _at.unlock(); return NSAPI_ERROR_NO_MEMORY; } @@ -509,6 +512,7 @@ nsapi_size_or_error_t AT_CellularSMS::send_sms(const char *phone_number, const c // sending can be cancelled by giving character (IRA 27). _at.cmd_start(ESC); _at.cmd_stop(); + _at.restore_at_timeout(); _at.unlock(); delete [] pdu_str; return msg_write_len; @@ -523,6 +527,7 @@ nsapi_size_or_error_t AT_CellularSMS::send_sms(const char *phone_number, const c delete [] pdu_str; remaining_len -= concatenated_sms_length; if (_at.get_last_error() != NSAPI_ERROR_OK) { + _at.restore_at_timeout(); return _at.unlock_return_error(); } } @@ -530,6 +535,7 @@ nsapi_size_or_error_t AT_CellularSMS::send_sms(const char *phone_number, const c _sms_message_ref_number++; nsapi_error_t ret = _at.get_last_error(); + _at.restore_at_timeout(); _at.unlock(); return (ret == NSAPI_ERROR_OK) ? msg_len : ret; From d676084600bb9df3496cef59e9e2a51296217b76 Mon Sep 17 00:00:00 2001 From: David Alonso de la Torre Date: Sat, 30 Dec 2023 11:47:56 +0100 Subject: [PATCH 2/3] Increase AT timeout to 10s in AT_CellularSMS::get_sms When SMS list is big and baudrate is not fast enough, with default timeout we can suffer from timeout error while getting a sms because method is parsing the full list and this takes long. --- connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp b/connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp index 8820c731c31..949bf17cfb7 100644 --- a/connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp +++ b/connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp @@ -703,6 +703,7 @@ nsapi_size_or_error_t AT_CellularSMS::get_sms(char *buf, uint16_t len, char *pho } _at.lock(); + _at.set_at_timeout(10s); nsapi_size_or_error_t err = list_messages(); if (err == NSAPI_ERROR_OK) { @@ -716,6 +717,7 @@ nsapi_size_or_error_t AT_CellularSMS::get_sms(char *buf, uint16_t len, char *pho *buf_size = info->msg_size; } free_linked_list(); + _at.restore_at_timeout(); _at.unlock(); return NSAPI_ERROR_PARAMETER; } @@ -740,6 +742,7 @@ nsapi_size_or_error_t AT_CellularSMS::get_sms(char *buf, uint16_t len, char *pho free_linked_list(); + _at.restore_at_timeout(); _at.unlock(); // update error only when there really was an error, otherwise we return the length From 9e7e22dd2c3392c2fee68b5ad9bae2b1ce59f3a6 Mon Sep 17 00:00:00 2001 From: David Alonso de la Torre Date: Sat, 30 Dec 2023 11:52:12 +0100 Subject: [PATCH 3/3] Fix AT_CellularSMS::list_messages breaking in text mode when CRLF is contained in SMS payload text When parsing SMS, it can happen that we receive CRLF in the SMS payload (happened to me when receiving provider texts). As an example, we can receive: """ Hello World! """ With previous implementation, second consume_to_stop_tag was stopping in and rest of the code was failing for obvious reasons. With this commit we consume the full payload as bytes. --- .../cellular/framework/API/ATHandler.h | 13 ++++++ .../source/framework/AT/AT_CellularSMS.cpp | 15 ++++++- .../source/framework/device/ATHandler.cpp | 40 +++++++++++++++++++ .../UNITTESTS/doubles/ATHandler_stub.cpp | 9 +++++ .../AT/at_cellularsms/at_cellularsmstest.cpp | 12 +++++- 5 files changed, 85 insertions(+), 4 deletions(-) diff --git a/connectivity/cellular/include/cellular/framework/API/ATHandler.h b/connectivity/cellular/include/cellular/framework/API/ATHandler.h index da2a6ea124a..4c70dc2fa7a 100644 --- a/connectivity/cellular/include/cellular/framework/API/ATHandler.h +++ b/connectivity/cellular/include/cellular/framework/API/ATHandler.h @@ -318,6 +318,13 @@ class ATHandler { */ void skip_param(ssize_t len, uint32_t count); + /** Consumes the given length from the reading buffer even if the stop tag has been found + * + * @param len length to be consumed from reading buffer + * @param count number of parameters to be skipped + */ + void skip_param_bytes(ssize_t len, uint32_t count); + /** Reads given number of bytes from receiving buffer without checking any subparameter delimiters, such as comma. * * @param buf output buffer for the read @@ -408,6 +415,12 @@ class ATHandler { */ bool consume_to_stop_tag(); + /** Consumes the received content until current stop tag is found even if stop tag has been found previously + * + * @return true if stop tag is found, false otherwise + */ + bool consume_to_stop_tag_even_found(); + /** Return the last 3GPP error code. * @return last 3GPP error code */ diff --git a/connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp b/connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp index 949bf17cfb7..c7ad6a537b3 100644 --- a/connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp +++ b/connectivity/cellular/source/framework/AT/AT_CellularSMS.cpp @@ -1036,6 +1036,7 @@ nsapi_error_t AT_CellularSMS::list_messages() int index = 0; int length = 0; char *pdu = NULL; + char buffer[32]; // 32 > SMS_STATUS_SIZE, SMS_MAX_PHONE_NUMBER_SIZE, SMS_MAX_TIME_STAMP_SIZE _at.resp_start("+CMGL:"); while (_at.info_resp()) { @@ -1058,8 +1059,18 @@ nsapi_error_t AT_CellularSMS::list_messages() // +CMGL: ,,,[],[][,,][ // +CMGL: ,,,[],[][,,][...]] index = _at.read_int(); - (void)_at.consume_to_stop_tag(); // consume until - (void)_at.consume_to_stop_tag(); // consume until + _at.read_string(buffer, SMS_STATUS_SIZE); + _at.read_string(buffer, SMS_MAX_PHONE_NUMBER_SIZE); + _at.skip_param(); // + _at.read_string(buffer, SMS_MAX_TIME_STAMP_SIZE); + _at.read_int(); + int size = _at.read_int(); // length + _at.consume_to_stop_tag(); // consume until end of header + if (size > 0) { + // we can not use skip param because we already consumed stop tag + _at.skip_param_bytes(size, 1); + } + _at.consume_to_stop_tag_even_found(); // consume until -> data } if (index >= 0) { diff --git a/connectivity/cellular/source/framework/device/ATHandler.cpp b/connectivity/cellular/source/framework/device/ATHandler.cpp index 034d586a142..7388bd4eb4a 100644 --- a/connectivity/cellular/source/framework/device/ATHandler.cpp +++ b/connectivity/cellular/source/framework/device/ATHandler.cpp @@ -484,6 +484,26 @@ void ATHandler::skip_param(ssize_t len, uint32_t count) return; } +void ATHandler::skip_param_bytes(ssize_t len, uint32_t count) +{ + if (!ok_to_proceed()) { + return; + } + + for (uint32_t i = 0; i < count; i++) { + ssize_t read_len = 0; + while (read_len < len) { + int c = get_char(); + if (c == -1) { + set_error(NSAPI_ERROR_DEVICE_ERROR); + return; + } + read_len++; + } + } + return; +} + ssize_t ATHandler::read_bytes(uint8_t *buf, size_t len) { if (!ok_to_proceed()) { @@ -1093,6 +1113,26 @@ bool ATHandler::consume_to_stop_tag() return false; } + +bool ATHandler::consume_to_stop_tag_even_found() +{ + if (_error_found) { + return true; + } + + if (!_is_fh_usable) { + _last_err = NSAPI_ERROR_BUSY; + return true; + } + + if (consume_to_tag((const char *)_stop_tag->tag, true)) { + return true; + } + + clear_error(); + return false; +} + // consume by size needed? void ATHandler::resp_stop() diff --git a/connectivity/cellular/tests/UNITTESTS/doubles/ATHandler_stub.cpp b/connectivity/cellular/tests/UNITTESTS/doubles/ATHandler_stub.cpp index c5b9c7ce673..1f11e035085 100644 --- a/connectivity/cellular/tests/UNITTESTS/doubles/ATHandler_stub.cpp +++ b/connectivity/cellular/tests/UNITTESTS/doubles/ATHandler_stub.cpp @@ -201,6 +201,10 @@ void ATHandler::skip_param(ssize_t len, uint32_t count) { } +void ATHandler::skip_param_bytes(ssize_t len, uint32_t count) +{ +} + ssize_t ATHandler::read_bytes(uint8_t *buf, size_t len) { return ATHandler_stub::ssize_value; @@ -301,6 +305,11 @@ bool ATHandler::consume_to_stop_tag() return ATHandler_stub::bool_value; } +bool ATHandler::consume_to_stop_tag_even_found() +{ + return ATHandler_stub::bool_value; +} + void ATHandler::resp_stop() { if (ATHandler_stub::resp_stop_success_count > 0) { diff --git a/connectivity/cellular/tests/UNITTESTS/framework/AT/at_cellularsms/at_cellularsmstest.cpp b/connectivity/cellular/tests/UNITTESTS/framework/AT/at_cellularsms/at_cellularsmstest.cpp index 4f4b1d9bc02..87c56bd197c 100644 --- a/connectivity/cellular/tests/UNITTESTS/framework/AT/at_cellularsms/at_cellularsmstest.cpp +++ b/connectivity/cellular/tests/UNITTESTS/framework/AT/at_cellularsms/at_cellularsmstest.cpp @@ -155,8 +155,16 @@ TEST_F(TestAT_CellularSMS, test_AT_CellularSMS_get_sms) ATHandler_stub::resp_info_false_counter = 1; ATHandler_stub::resp_info_true_counter2 = 2; ATHandler_stub::int_value = 11; - ATHandler_stub::read_string_index = 4; - ATHandler_stub::read_string_table[4] = ""; + ATHandler_stub::read_string_index = (3 * 2) + (2 * 2); // 3 read_string in list_messages + 2 read_string in read_sms_from_index + ATHandler_stub::read_string_table[10] = ""; + // list_messages + ATHandler_stub::read_string_table[9] = "1"; // status + ATHandler_stub::read_string_table[8] = "+00611223344"; // phone + ATHandler_stub::read_string_table[7] = "24/12/12,11:15:00+04"; // timestamp + ATHandler_stub::read_string_table[6] = "1"; // status + ATHandler_stub::read_string_table[5] = "+00611223344"; // phone + ATHandler_stub::read_string_table[4] = "24/12/12,11:15:00+04"; // timestamp + // read_sms_from_index ATHandler_stub::read_string_table[3] = "REC READ"; ATHandler_stub::read_string_table[2] = "09/01/12,11:15:00+04"; ATHandler_stub::read_string_table[1] = "REC READ";