iot_usbh_modem: fix concurrency issue

Close #295

components/usb/iot_usbh_modem/CHANGELOG.md

@@ -1,5 +1,13 @@
 # ChangeLog
 
+## v0.2.0 - 2023-10-9
+
+* Improve recovery mechanism
+* Add new model AIR780E
+* Fix concurrency issue in AT command mode
+* Fix memory stamp issue in `common_get_operator_after_mode_format`
+
+
 ## v0.1.6 - 2023-04-17
 
 * Fix PPP mode can not exit gracefully

components/usb/iot_usbh_modem/Kconfig

@@ -8,6 +8,8 @@ menu "IoT USB Host Modem"
             bool "ML302-DNLM/CNLM"
         config MODEM_TARGET_AIR724UG_NFM
             bool "AIR724UG-NFM"
+        config MODEM_TARGET_AIR780_E
+            bool "AIR780E"
         config MODEM_TARGET_EC600N_CNLA_N05
             bool "EC600NCNLA-N05"
         config MODEM_TARGET_EC600N_CNLC_N06
@@ -31,6 +33,7 @@ menu "IoT USB Host Modem"
         default "EC600NCNLA-N05" if MODEM_TARGET_EC600N_CNLA_N05
         default "EC600NCNLC-N06" if MODEM_TARGET_EC600N_CNLC_N06
         default "A7600C1" if MODEM_TARGET_A7600C1
+        default "AIR780E" if MODEM_TARGET_AIR780_E
         default "BG95M3" if MODEM_TARGET_BG95_M3
         default "BG96MA" if MODEM_TARGET_BG96_MA
         default "MC610_EU" if MODEM_TARGET_MC610_EU
@@ -52,6 +55,7 @@ menu "IoT USB Host Modem"
             default 0x0f if MODEM_TARGET_EC600N_CNLA_N05
             default 0x0a if MODEM_TARGET_EC600N_CNLC_N06
             default 0x0a if MODEM_TARGET_A7600C1
+            default 0X02 if MODEM_TARGET_AIR780_E
             default 0x03 if MODEM_TARGET_BG95_M3
             default 0x04 if MODEM_TARGET_BG96_MA
             default 0x07 if MODEM_TARGET_MC610_EU
@@ -67,6 +71,7 @@ menu "IoT USB Host Modem"
             default 0x86 if  MODEM_TARGET_EC600N_CNLA_N05
             default 0x81 if  MODEM_TARGET_EC600N_CNLC_N06
             default 0x81 if MODEM_TARGET_A7600C1
+            default 0X84 if MODEM_TARGET_AIR780_E
             default 0x84 if MODEM_TARGET_BG95_M3
             default 0x86 if MODEM_TARGET_BG96_MA
             default 0x87 if MODEM_TARGET_MC610_EU
@@ -125,6 +130,24 @@ menu "IoT USB Host Modem"
             default 1
     endmenu
 
+    menu "Timeout Config"
+        config MODEM_COMMAND_TIMEOUT_DEFAULT
+            int "Timeout (ms) for common AT command"
+            default 2000
+        config MODEM_COMMAND_TIMEOUT_OPERATOR
+            int "Timeout (ms) for getting operator status"
+            default 6000
+        config MODEM_COMMAND_TIMEOUT_RESET
+            int "Timeout (ms) for reset command"
+            default 6000
+        config MODEM_COMMAND_TIMEOUT_MODE_CHANGE
+            int "Timeout (ms) for changing working mode"
+            default 10000
+        config MODEM_COMMAND_TIMEOUT_POWEROFF
+            int "Timeout (ms) for power down"
+            default 1000
+    endmenu
+
     menu "Wi-Fi SoftAP"
 
         config MODEM_WIFI_SSID

components/usb/iot_usbh_modem/idf_component.yml

@@ -1,4 +1,4 @@
-version: "0.1.6"
+version: "0.2.0"
 targets:
   - esp32s2
   - esp32s3

components/usb/iot_usbh_modem/include/usbh_modem_board.h

@@ -34,7 +34,7 @@ typedef enum {
         .rx_buffer_size = 1024*15,   \
         .tx_buffer_size = 1024*15,   \
         .line_buffer_size = 1600,    \
-        .event_task_priority = CONFIG_USBH_TASK_BASE_PRIORITY + 3,\
+        .event_task_priority = CONFIG_USBH_TASK_BASE_PRIORITY + 1,\
         .event_task_stack_size = 3072\
     }
 

components/usb/iot_usbh_modem/private_include/esp_modem.h

@@ -180,13 +180,6 @@ typedef struct esp_modem_dce_config_s {
  */
 esp_modem_dte_t *esp_modem_dte_new(const esp_modem_dte_config_t *config);
 
-/**
- * @brief Create and initialize Modem DCE object
- *
- * @param config configuration of ESP Modem DTE object
- * @return esp_modem_dce_t* Modem DCE object
- */
-esp_modem_dce_t *esp_modem_dce_new(esp_modem_dce_config_t *config);
 
 /**
  * @brief Initialize the DCE object that has already been created
@@ -240,9 +233,6 @@ esp_err_t esp_modem_set_event_handler(esp_modem_dte_t *dte, esp_event_handler_t
 esp_err_t esp_modem_remove_event_handler(esp_modem_dte_t *dte, esp_event_handler_t handler);
 
 esp_err_t esp_modem_post_event(esp_modem_dte_t *dte, int32_t event_id, void* event_data, size_t event_data_size, TickType_t ticks_to_wait);
-/**
- * @}
- */
 
 
 /**

components/usb/iot_usbh_modem/private_include/esp_modem_dce_common_commands.h

@@ -10,6 +10,7 @@ extern "C" {
 #endif
 
 #include "esp_modem_dce.h"
+#include "sdkconfig.h"
 
 /**
 * @brief Result Code from DCE
@@ -28,11 +29,11 @@ extern "C" {
  * @brief Specific Timeout Constraint, Unit: millisecond
  *
  */
-#define MODEM_COMMAND_TIMEOUT_DEFAULT (2000)      /*!< Default timeout value for most commands */
-#define MODEM_COMMAND_TIMEOUT_OPERATOR (60000)   /*!< Timeout value for getting operator status */
-#define MODEM_COMMAND_TIMEOUT_RESET (60000)   /*!< Timeout value for reset command */
-#define MODEM_COMMAND_TIMEOUT_MODE_CHANGE (30000) /*!< Timeout value for changing working mode */
-#define MODEM_COMMAND_TIMEOUT_POWEROFF (1000)    /*!< Timeout value for power down */
+#define MODEM_COMMAND_TIMEOUT_DEFAULT     CONFIG_MODEM_COMMAND_TIMEOUT_DEFAULT
+#define MODEM_COMMAND_TIMEOUT_OPERATOR    CONFIG_MODEM_COMMAND_TIMEOUT_OPERATOR
+#define MODEM_COMMAND_TIMEOUT_RESET       CONFIG_MODEM_COMMAND_TIMEOUT_RESET
+#define MODEM_COMMAND_TIMEOUT_MODE_CHANGE CONFIG_MODEM_COMMAND_TIMEOUT_MODE_CHANGE
+#define MODEM_COMMAND_TIMEOUT_POWEROFF    CONFIG_MODEM_COMMAND_TIMEOUT_POWEROFF
 
 /**
 * @brief Strip the tailed "\r\n"

components/usb/iot_usbh_modem/private_include/esp_modem_dte.h

@@ -12,6 +12,8 @@ extern "C" {
 #include "esp_types.h"
 #include "esp_err.h"
 #include "esp_event.h"
+#include "freertos/FreeRTOS.h"
+#include "freertos/semphr.h"
 
 /**
  * @brief Working mode of Modem
@@ -31,7 +33,8 @@ typedef enum {
 struct esp_modem_dte {
     esp_modem_flow_ctrl_t flow_ctrl;                                                    /*!< Flow control of DTE */
     esp_modem_dce_t *dce;                                                               /*!< DCE which connected to the DTE */
-    struct esp_modem_netif_driver_s *netif_adapter;
+    struct esp_modem_netif_driver_s *netif_adapter;                                     /*!< Netif adapter */
+    SemaphoreHandle_t send_cmd_lock;                                                    /*!< Mutex for send command */
     esp_err_t (*send_cmd)(esp_modem_dte_t *dte, const char *command, uint32_t timeout); /*!< Send command to DCE */
     int (*send_data)(esp_modem_dte_t *dte, const char *data, uint32_t length);          /*!< Send data to DCE */
     esp_err_t (*send_wait)(esp_modem_dte_t *dte, const char *data, uint32_t length,

components/usb/iot_usbh_modem/src/esp_modem.c

@@ -145,16 +145,6 @@ esp_err_t esp_modem_default_attach(esp_modem_dte_t *dte, esp_modem_dce_t *dce, e
     return ESP_FAIL;
 }
 
-esp_modem_dce_t *esp_modem_dce_new(esp_modem_dce_config_t *config)
-{
-    ESP_MODEM_ERR_CHECK(config, "failed to init with zero configuration", err);
-    esp_modem_dce_t *dce = calloc(1, sizeof(esp_modem_dce_t));
-    ESP_MODEM_ERR_CHECK(dce, "calloc of esp_modem_dce_t failed", err);
-    ESP_MODEM_ERR_CHECK(esp_modem_dce_init(dce, config) == ESP_OK, "esp_modem_dce_init has failed", err);
-    return dce;
-err:
-    return NULL;
-}
 
 esp_err_t esp_modem_dce_init(esp_modem_dce_t *dce, esp_modem_dce_config_t *config)
 {

components/usb/iot_usbh_modem/src/esp_modem_dce.c

@@ -4,6 +4,8 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 #include <string.h>
+#include "freertos/FreeRTOS.h"
+#include "freertos/semphr.h"
 #include "esp_modem_dce.h"
 #include "esp_modem_dce_command_lib.h"
 #include "esp_modem_dce_common_commands.h"
@@ -15,17 +17,21 @@ static const char *TAG = "esp_modem_dce";
 esp_err_t esp_modem_dce_generic_command(esp_modem_dce_t *dce, const char * command, uint32_t timeout, esp_modem_dce_handle_line_t handle_line, void *ctx)
 {
     esp_modem_dte_t *dte = dce->dte;
-    ESP_LOGD(TAG, "%s(%d): Sending command:%s\n", __func__, __LINE__, command );
+    ESP_LOGD(TAG, "%s(%d): Sending command:%s\n", __func__, __LINE__, command);
+    xSemaphoreTake(dte->send_cmd_lock, portMAX_DELAY);
     dce->handle_line = handle_line;
     dce->handle_line_ctx = ctx;
     if (dte->send_cmd(dte, command, timeout) != ESP_OK) {
+        xSemaphoreGive(dte->send_cmd_lock);
         ESP_LOGW(TAG, "%s(%d): Command:%s response timeout", __func__, __LINE__, command );
         return ESP_ERR_TIMEOUT;
     }
     if (dce->state == ESP_MODEM_STATE_FAIL) {
+        xSemaphoreGive(dte->send_cmd_lock);
         ESP_LOGW(TAG, "%s(%d): Command:%s\n...failed", __func__, __LINE__, command );
         return ESP_FAIL;
     }
+    xSemaphoreGive(dte->send_cmd_lock);
     ESP_LOGD(TAG, "%s(%d): Command:%s\n succeeded", __func__, __LINE__, command );
     return ESP_OK;
 }
@@ -175,7 +181,6 @@ esp_err_t esp_modem_dce_default_start_up(esp_modem_dce_t *dce)
 {
     ESP_MODEM_ERR_CHECK(dce->sync(dce, NULL, NULL) == ESP_OK, "sending sync failed", err);
     ESP_MODEM_ERR_CHECK(dce->set_echo(dce, (void*)false, NULL) == ESP_OK, "set_echo failed", err);
-    ESP_MODEM_ERR_CHECK(dce->store_profile(dce, NULL, NULL) == ESP_OK, "store_profile failed", err);
     return ESP_OK;
     err:
     return ESP_FAIL;

components/usb/iot_usbh_modem/src/esp_modem_dce_command_lib.c

@@ -90,8 +90,6 @@ static esp_err_t esp_modem_dce_init_command_list(esp_modem_dce_t *dce, size_t co
     return ESP_OK;
 }
 
-
-
 esp_err_t esp_modem_set_default_command_list(esp_modem_dce_t *dce)
 {
     esp_err_t err = esp_modem_dce_init_command_list(dce, sizeof(s_command_list) / sizeof(cmd_item_t), s_command_list);

components/usb/iot_usbh_modem/src/esp_modem_dce_common_commands.c

@@ -23,21 +23,27 @@ static inline esp_err_t generic_command_default_handle(esp_modem_dce_t *dce, con
 static esp_err_t common_handle_string(esp_modem_dce_t *dce, const char *line)
 {
     esp_err_t err = ESP_FAIL;
-    if (strstr(line, MODEM_RESULT_CODE_SUCCESS)) {
-        err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_SUCCESS);
-    } else if (strstr(line, MODEM_RESULT_CODE_ERROR)) {
+    if (strstr(line, MODEM_RESULT_CODE_ERROR)) {
         err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_FAIL);
+        return err;
     }
-    common_string_t *result_str = dce->handle_line_ctx;
-    assert(result_str->command != NULL && result_str->string != NULL && result_str->len != 0);
-    char *result = strstr(line, "+");
-    if (result) {
-        int len = snprintf(result_str->string, result_str->len, "%s", result);
-        if (len > 2) {
-            /* Strip "\r\n" */
-            strip_cr_lf_tail(result_str->string, len);
-            err = ESP_OK;
+
+    if (strstr(line, "+")) {
+        common_string_t *result_str = dce->handle_line_ctx;
+        assert(result_str->command != NULL && result_str->string != NULL && result_str->len != 0);
+        char *result = strstr(line, "+");
+        if (result) {
+            int len = snprintf(result_str->string, result_str->len, "%s", result);
+            if (len > 2) {
+                /* Strip "\r\n" */
+                strip_cr_lf_tail(result_str->string, len);
+            }
         }
+        err = ESP_OK;
+    }
+
+    if (strstr(line, MODEM_RESULT_CODE_SUCCESS)) {
+        err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_SUCCESS);
     }
     return err;
 }
@@ -48,17 +54,21 @@ static esp_err_t common_handle_string(esp_modem_dce_t *dce, const char *line)
 static esp_err_t esp_modem_dce_common_handle_cbc(esp_modem_dce_t *dce, const char *line)
 {
     esp_err_t err = ESP_FAIL;
-    if (strstr(line, MODEM_RESULT_CODE_SUCCESS)) {
-        err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_SUCCESS);
-    } else if (strstr(line, MODEM_RESULT_CODE_ERROR)) {
+    if (strstr(line, MODEM_RESULT_CODE_ERROR)) {
         err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_FAIL);
+        return err;
     }
+
     if (strstr(line, "+CBC")) {
         esp_modem_dce_cbc_ctx_t *cbc = dce->handle_line_ctx;
         /* +CBC: <bcs>,<bcl>,<voltage> */
         sscanf(strstr(line, "+CBC"), "%*s%d,%d,%d", &cbc->bcs, &cbc->bcl, &cbc->battery_status);
         err = ESP_OK;
     }
+
+    if (strstr(line, MODEM_RESULT_CODE_SUCCESS)) {
+        err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_SUCCESS);
+    }
     return err;
 }
 /**
@@ -67,18 +77,22 @@ static esp_err_t esp_modem_dce_common_handle_cbc(esp_modem_dce_t *dce, const cha
 static esp_err_t esp_modem_dce_common_handle_csq(esp_modem_dce_t *dce, const char *line)
 {
     esp_err_t err = ESP_FAIL;
-    if (strstr(line, MODEM_RESULT_CODE_SUCCESS)) {
-        err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_SUCCESS);
-    } else if (strstr(line, MODEM_RESULT_CODE_ERROR)) {
+    if (strstr(line, MODEM_RESULT_CODE_ERROR)) {
         err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_FAIL);
+        return err;
     }
+
     if (strstr(line, "+CSQ")) {
         /* store value of rssi and ber */
         esp_modem_dce_csq_ctx_t *csq = dce->handle_line_ctx;
         /* +CSQ: <rssi>,<ber> */
         sscanf(strstr(line, "+CSQ"), "%*s%d,%d", &csq->rssi, &csq->ber);
         err = ESP_OK;
     }
+
+    if (strstr(line, MODEM_RESULT_CODE_SUCCESS)) {
+        err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_SUCCESS);
+    }
     return err;
 }
 
@@ -90,7 +104,9 @@ static esp_err_t esp_modem_dce_handle_power_down(esp_modem_dce_t *dce, const cha
     esp_err_t err = ESP_FAIL;
     if (strstr(line, MODEM_RESULT_CODE_SUCCESS)) {
         err = ESP_OK;
-    } else if (strstr(line, "POWERED DOWN")) {
+    }
+
+    if (strstr(line, "POWERED DOWN")) {
         err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_SUCCESS);
     }
     return err;
@@ -129,11 +145,11 @@ static esp_err_t esp_modem_dce_handle_atd_ppp(esp_modem_dce_t *dce, const char *
 static esp_err_t esp_modem_dce_handle_read_pin(esp_modem_dce_t *dce, const char *line)
 {
     esp_err_t err = ESP_FAIL;
-    if (strstr(line, MODEM_RESULT_CODE_SUCCESS)) {
-        err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_SUCCESS);
-    } else if (strstr(line, MODEM_RESULT_CODE_ERROR)) {
+    if (strstr(line, MODEM_RESULT_CODE_ERROR)) {
         err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_FAIL);
+        return err;
     }
+
     if (strstr(line, "READY")) {
         int *ready = (int*)dce->handle_line_ctx;
         *ready = true;
@@ -143,6 +159,10 @@ static esp_err_t esp_modem_dce_handle_read_pin(esp_modem_dce_t *dce, const char
         *ready = false;
         err = ESP_OK;
     }
+
+    if (strstr(line, MODEM_RESULT_CODE_SUCCESS)) {
+        err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_SUCCESS);
+    } 
     return err;
 }
 
@@ -151,8 +171,7 @@ static esp_err_t esp_modem_dce_handle_reset(esp_modem_dce_t *dce, const char *li
     esp_err_t err = ESP_OK;
     if (strstr(line, MODEM_RESULT_CODE_SUCCESS)) {
         err = ESP_OK;
-    } else
-    if (strstr(line, "PB DONE")) {
+    } else if (strstr(line, "PB DONE")) {
         err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_SUCCESS);
     } else if (strstr(line, MODEM_RESULT_CODE_ERROR)) {
         err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_FAIL);
@@ -179,11 +198,11 @@ esp_err_t esp_modem_dce_set_echo(esp_modem_dce_t *dce, void *param, void *result
 static esp_err_t common_get_operator_after_mode_format(esp_modem_dce_t *dce, const char *line)
 {
     esp_err_t err = ESP_FAIL;
-    if (strstr(line, MODEM_RESULT_CODE_SUCCESS)) {
-        err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_SUCCESS);
-    } else if (strstr(line, MODEM_RESULT_CODE_ERROR)) {
+    if (strstr(line, MODEM_RESULT_CODE_ERROR)) {
         err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_FAIL);
+        return err;
     }
+
     if (strstr(line, "+COPS")) {
         common_string_t *result_str = dce->handle_line_ctx;
         assert(result_str->string != NULL && result_str->len != 0);
@@ -192,11 +211,11 @@ static esp_err_t common_get_operator_after_mode_format(esp_modem_dce_t *dce, con
         char *line_copy = strdup(strstr(line, "+COPS"));
         /* +COPS: <mode>[, <format>[, <oper>]] */
         char *str_ptr = NULL;
-        char *p[3];
+        char *p[6] = {0};
         uint8_t i = 0;
         /* strtok will broke string by replacing delimiter with '\0' */
         p[i] = strtok_r(line_copy, ",", &str_ptr);
-        while (p[i]) {
+        while (p[i] && i < 5) {
             p[++i] = strtok_r(NULL, ",", &str_ptr);
         }
         if (i >= 3) {
@@ -208,6 +227,11 @@ static esp_err_t common_get_operator_after_mode_format(esp_modem_dce_t *dce, con
             }
         }
         free(line_copy);
+        err = ESP_OK;
+    }
+
+    if (strstr(line, MODEM_RESULT_CODE_SUCCESS)) {
+        err = esp_modem_process_command_done(dce, ESP_MODEM_STATE_SUCCESS);
     }
     return err;
 }