Skip to content

Commit

Permalink
Examples/pppos_client: Remove uart pattern detection
Browse files Browse the repository at this point in the history 
Due to these problems
* possible race condition, two events competing
* pattern detection work a little different for different IDF versions
* switching between pattern detection and data reading is complicated
* using just data events is easier for porting to a different communication peripheral
These changes were made:
* Removed the UART pattern detection entirely
* Used manual pattern detection after reading plain data from the UART

Closes #7176
  • Loading branch information
@david-cermak
david-cermak committed Sep 21, 2021
1 parent 29fac4f commit a90817c
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 143 deletions.
9 changes: 5 additions & 4 deletions examples/protocols/pppos_client/components/modem/include/esp_modem.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,13 @@ typedef struct {
int cts_io_num; /*!< CTS Pin Number */
int rx_buffer_size; /*!< UART RX Buffer Size */
int tx_buffer_size; /*!< UART TX Buffer Size */
int pattern_queue_size; /*!< UART Pattern Queue Size */
int event_queue_size; /*!< UART Event Queue Size */
uint32_t event_task_stack_size; /*!< UART Event Task Stack size */
int event_task_priority; /*!< UART Event Task Priority */
int line_buffer_size; /*!< Line buffer size for command mode */
union {
int dte_buffer_size; /*!< Internal buffer size */
int line_buffer_size; /*!< Compatible option for the internal buffer size */
};
} esp_modem_dte_config_t;

/**
Expand All @@ -87,11 +89,10 @@ typedef esp_err_t (*esp_modem_on_receive)(void *buffer, size_t len, void *contex
.cts_io_num = 23, \
.rx_buffer_size = 1024, \
.tx_buffer_size = 512, \
.pattern_queue_size = 20, \
.event_queue_size = 30, \
.event_task_stack_size = 2048, \
.event_task_priority = 5, \
.line_buffer_size = 512 \
.dte_buffer_size = 512 \
}

/**
Expand Down
182 changes: 52 additions & 130 deletions examples/protocols/pppos_client/components/modem/src/esp_modem.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,13 @@

#define ESP_MODEM_EVENT_QUEUE_SIZE (16)

#define MIN_PATTERN_INTERVAL (9)
#define MIN_POST_IDLE (0)
#define MIN_PRE_IDLE (0)
/**
* @brief This sets the threshold for receiving data events when UART RX buffer reaches
* this level. Decreasing the number causes more events and lowers changes of UART overflows,
* but more allocations in lwIP. You can increase this number if you're using slower baudrates
* or having the UART ISR in IRAM.
*/
#define ESP_MODEM_UART_RX_FULL_THRESHOLD (64)

/**
* @brief Macro defined for error checking
Expand Down Expand Up @@ -59,8 +63,8 @@ typedef struct {
modem_dte_t parent; /*!< DTE interface that should extend */
esp_modem_on_receive receive_cb; /*!< ptr to data reception */
void *receive_cb_ctx; /*!< ptr to rx fn context data */
int line_buffer_size; /*!< line buffer size in commnad mode */
int pattern_queue_size; /*!< UART pattern queue size */
int buffer_size; /*!< internal buffer size */
int consumed; /*!< index to the consumed buffer pointer */
} esp_modem_dte_t;

/**
Expand Down Expand Up @@ -95,22 +99,31 @@ esp_err_t esp_modem_set_rx_cb(modem_dte_t *dte, esp_modem_on_receive receive_cb,
* - ESP_OK on success
* - ESP_FAIL on error
*/
static esp_err_t esp_dte_handle_line(esp_modem_dte_t *esp_dte)
static esp_err_t esp_dte_handle_line(esp_modem_dte_t *esp_dte, char * line, size_t len, char separator)
{
esp_err_t err = ESP_FAIL;
modem_dce_t *dce = esp_dte->parent.dce;
MODEM_CHECK(dce, "DTE has not yet bind with DCE", err);
const char *line = (const char *)(esp_dte->buffer);
size_t len = strlen(line);
/* Skip pure "\r\n" lines */
if (len > 2 && !is_only_cr_lf(line, len)) {
if (dce->handle_line == NULL) {
/* Received an asynchronous line, but no handler waiting this this */
ESP_LOGD(MODEM_TAG, "No handler for line: %s", line);
err = ESP_OK; /* Not an error, just propagate the line to user handler */
goto post_event_unknown;
}
if (separator != '\n' && dce->handle_line) {
/* If waiting for a specific separator, just pass the entire string */
MODEM_CHECK(dce->handle_line(dce, line) == ESP_OK, "handle line failed", post_event_unknown);
return ESP_OK;
}
/* Tokenize the data to call handlers separately for each *line* */
char *str_ptr = NULL;
char *p = strtok_r(line, "\n", &str_ptr);
while (p) {
if (len > 2 && !is_only_cr_lf(p, strlen(p))) {
ESP_LOGD(MODEM_TAG, "Handling line: >>%s\n<<", p);
if (dce->handle_line == NULL) {
/* Received an asynchronous line, but no handler waiting this this */
ESP_LOGD(MODEM_TAG, "No handler for line: %s", p);
err = ESP_OK; /* Not an error, just propagate the line to user handler */
goto post_event_unknown;
}
MODEM_CHECK(dce->handle_line(dce, p) == ESP_OK, "handle line failed", post_event_unknown);
}
p = strtok_r(NULL, "\n", &str_ptr);
}
return ESP_OK;
post_event_unknown:
Expand All @@ -121,54 +134,6 @@ static esp_err_t esp_dte_handle_line(esp_modem_dte_t *esp_dte)
return err;
}

/**
* @brief Handle when a pattern has been detected by UART
*
* @param esp_dte ESP32 Modem DTE object
*/
static void esp_handle_uart_pattern(esp_modem_dte_t *esp_dte)
{
int pos = uart_pattern_pop_pos(esp_dte->uart_port);
int read_len = 0;

if (esp_dte->parent.dce->mode == MODEM_PPP_MODE) {
ESP_LOGD(MODEM_TAG, "Pattern event in PPP mode ignored");
// Ignore potential pattern detection events in PPP mode
// Note 1: the interrupt is disabled, but some events might still be pending
// Note 2: checking the mode *after* uart_pattern_pop_pos() to consume the event
return;
}

if (pos != -1) {
if (pos < esp_dte->line_buffer_size - 1) {
/* read one line(include '\n') */
read_len = pos + 1;
} else {
ESP_LOGW(MODEM_TAG, "ESP Modem Line buffer too small");
read_len = esp_dte->line_buffer_size - 1;
}
read_len = uart_read_bytes(esp_dte->uart_port, esp_dte->buffer, read_len, pdMS_TO_TICKS(100));
if (read_len) {
/* make sure the line is a standard string */
esp_dte->buffer[read_len] = '\0';
/* Send new line to handle */
esp_dte_handle_line(esp_dte);
} else {
ESP_LOGE(MODEM_TAG, "uart read bytes failed");
}
} else {
size_t length = 0;
uart_get_buffered_data_len(esp_dte->uart_port, &length);
if (length) {
ESP_LOGD(MODEM_TAG, "Pattern not found in the pattern queue, uart data length = %d", length);
length = MIN(esp_dte->line_buffer_size-1, length);
length = uart_read_bytes(esp_dte->uart_port, esp_dte->buffer, length, portMAX_DELAY);
ESP_LOG_BUFFER_HEXDUMP("esp-modem-pattern: debug_data", esp_dte->buffer, length, ESP_LOG_DEBUG);
}
uart_flush(esp_dte->uart_port);
}
}

/**
* @brief Handle when new data received by UART
*
Expand All @@ -178,40 +143,28 @@ static void esp_handle_uart_data(esp_modem_dte_t *esp_dte)
{
size_t length = 0;
uart_get_buffered_data_len(esp_dte->uart_port, &length);
ESP_LOGV(MODEM_TAG, "uart_get_buffered_data_len()=%d", length);
if (esp_dte->parent.dce->mode != MODEM_PPP_MODE && length) {
// Check if matches the pattern to process the data as pattern
int pos = uart_pattern_get_pos(esp_dte->uart_port);
if (pos > -1) {
esp_handle_uart_pattern(esp_dte);
return;
}
// Read the data and process it using `handle_line` logic
length = MIN(esp_dte->line_buffer_size-1, length);
length = uart_read_bytes(esp_dte->uart_port, esp_dte->buffer, length, portMAX_DELAY);
esp_dte->buffer[length] = '\0';
if (strchr((char*)esp_dte->buffer, '\n') == NULL) {
size_t max = esp_dte->line_buffer_size-1;
size_t bytes;
// if pattern not found in the data,
// continue reading as long as the modem is in MODEM_STATE_PROCESSING, checking for the pattern
while (length < max && esp_dte->buffer[length-1] != '\n' &&
esp_dte->parent.dce->state == MODEM_STATE_PROCESSING) {
bytes = uart_read_bytes(esp_dte->uart_port,
esp_dte->buffer + length, 1, pdMS_TO_TICKS(100));
length += bytes;
ESP_LOGV("esp-modem: debug_data", "Continuous read in non-data mode: length: %d char: %x", length, esp_dte->buffer[length-1]);
}
length = MIN(esp_dte->buffer_size - 1, length);
length = uart_read_bytes(esp_dte->uart_port, esp_dte->buffer + esp_dte->consumed, length - esp_dte->consumed, portMAX_DELAY);
const char separator = esp_dte->parent.dce->prompt == NULL ? '\n' : (esp_dte->parent.dce->prompt)[strlen(esp_dte->parent.dce->prompt)-1];
if (memchr(esp_dte->buffer + esp_dte->consumed, separator, length)) {
esp_dte->buffer[length] = '\0';
}
ESP_LOG_BUFFER_HEXDUMP("esp-modem: debug_data", esp_dte->buffer, length, ESP_LOG_DEBUG);
if (esp_dte->parent.dce->handle_line) {
/* Send new line to handle if handler registered */
esp_dte_handle_line(esp_dte);
ESP_LOG_BUFFER_HEXDUMP("esp-modem: pattern-detection", esp_dte->buffer, length, ESP_LOG_VERBOSE);
if (esp_dte->parent.dce->handle_line) {
/* Send new line to handle if handler registered */
if (esp_dte_handle_line(esp_dte, (char*)esp_dte->buffer, length, separator) == ESP_OK) {
esp_dte->consumed = 0;
return;
}
}
esp_dte->consumed += length;
}
return;
}

length = MIN(esp_dte->line_buffer_size, length);
length = MIN(esp_dte->buffer_size, length);
length = uart_read_bytes(esp_dte->uart_port, esp_dte->buffer, length, portMAX_DELAY);
/* pass the input data to configured callback */
if (length) {
Expand Down Expand Up @@ -266,16 +219,12 @@ static void uart_event_task_entry(void *param)
case UART_FRAME_ERR:
ESP_LOGE(MODEM_TAG, "Frame Error");
break;
case UART_PATTERN_DET:
esp_handle_uart_pattern(esp_dte);
break;
default:
ESP_LOGW(MODEM_TAG, "unknown uart event type: %d", event.type);
break;
}
}
}
vTaskDelete(NULL);
}

/**
Expand All @@ -292,9 +241,11 @@ static esp_err_t esp_modem_dte_send_cmd(modem_dte_t *dte, const char *command, u
{
esp_err_t ret = ESP_FAIL;
modem_dce_t *dce = dte->dce;
ESP_LOGD(MODEM_TAG, "Sending command:%s", command);
MODEM_CHECK(dce, "DTE has not yet bind with DCE", err);
MODEM_CHECK(command, "command is NULL", err);
esp_modem_dte_t *esp_dte = __containerof(dte, esp_modem_dte_t, parent);
esp_dte->consumed = 0;
/* Calculate timeout clock tick */
/* Reset runtime information */
dce->state = MODEM_STATE_PROCESSING;
Expand Down Expand Up @@ -364,19 +315,14 @@ static esp_err_t esp_modem_dte_send_wait(modem_dte_t *dte, const char *data, uin
MODEM_CHECK(prompt, "prompt is NULL", err_param);
modem_dce_t *dce = dte->dce;
MODEM_CHECK(dce, "DTE has not yet bind with DCE", err_param);
esp_modem_dte_t *esp_dte = __containerof(dte, esp_modem_dte_t, parent);
// We'd better change pattern detection here for a moment in case prompt string contains the pattern character
uart_enable_pattern_det_baud_intr(esp_dte->uart_port, ' ', 1, MIN_PATTERN_INTERVAL, MIN_POST_IDLE, MIN_PRE_IDLE);
dce->prompt = prompt;
dce->prompt = prompt; // the last character of this prompt will be used as a separator to call the line handker
dce->handle_line = esp_modem_dte_send_wait_default_handler;
MODEM_CHECK(dte->send_cmd(dte, data, timeout) == ESP_OK, "wait for prompt timeout", err);
MODEM_CHECK(dce->state == MODEM_STATE_SUCCESS, "wait for prompt failed", err);
dce->prompt = NULL;
uart_enable_pattern_det_baud_intr(esp_dte->uart_port, '\n', 1, MIN_PATTERN_INTERVAL, MIN_POST_IDLE, MIN_PRE_IDLE);
return ESP_OK;
err:
dce->prompt = NULL;
uart_enable_pattern_det_baud_intr(esp_dte->uart_port, '\n', 1, MIN_PATTERN_INTERVAL, MIN_POST_IDLE, MIN_PRE_IDLE);
err_param:
return ESP_FAIL;
}
Expand All @@ -394,27 +340,11 @@ static esp_err_t esp_modem_dte_change_mode(modem_dte_t *dte, modem_mode_t new_mo
{
modem_dce_t *dce = dte->dce;
MODEM_CHECK(dce, "DTE has not yet bind with DCE", err);
esp_modem_dte_t *esp_dte = __containerof(dte, esp_modem_dte_t, parent);
modem_mode_t current_mode = dce->mode;
MODEM_CHECK(current_mode != new_mode, "already in mode: %d", err, new_mode);
dce->mode = MODEM_TRANSITION_MODE; // mode switching will be finished in set_working_mode() on success
// (or restored on failure)
switch (new_mode) {
case MODEM_PPP_MODE:
MODEM_CHECK(dce->set_working_mode(dce, new_mode) == ESP_OK, "set new working mode:%d failed", err_restore_mode, new_mode);
uart_disable_pattern_det_intr(esp_dte->uart_port);
uart_enable_rx_intr(esp_dte->uart_port);
break;
case MODEM_COMMAND_MODE:
MODEM_CHECK(dce->set_working_mode(dce, new_mode) == ESP_OK, "set new working mode:%d failed", err_restore_mode, new_mode);
uart_disable_rx_intr(esp_dte->uart_port);
uart_flush(esp_dte->uart_port);
uart_enable_pattern_det_baud_intr(esp_dte->uart_port, '\n', 1, MIN_PATTERN_INTERVAL, MIN_POST_IDLE, MIN_PRE_IDLE);
uart_pattern_queue_reset(esp_dte->uart_port, esp_dte->pattern_queue_size);
break;
default:
break;
}
MODEM_CHECK(dce->set_working_mode(dce, new_mode) == ESP_OK, "set new working mode:%d failed", err_restore_mode, new_mode);
return ESP_OK;
err_restore_mode:
dce->mode = current_mode;
Expand Down Expand Up @@ -464,8 +394,8 @@ modem_dte_t *esp_modem_dte_init(const esp_modem_dte_config_t *config)
esp_modem_dte_t *esp_dte = calloc(1, sizeof(esp_modem_dte_t));
MODEM_CHECK(esp_dte, "calloc esp_dte failed", err_dte_mem);
/* malloc memory to storing lines from modem dce */
esp_dte->line_buffer_size = config->line_buffer_size;
esp_dte->buffer = calloc(1, config->line_buffer_size);
esp_dte->buffer_size = config->dte_buffer_size;
esp_dte->buffer = calloc(1, config->dte_buffer_size);
MODEM_CHECK(esp_dte->buffer, "calloc line memory failed", err_line_mem);
/* Set attributes */
esp_dte->uart_port = config->port_num;
Expand Down Expand Up @@ -514,15 +444,9 @@ modem_dte_t *esp_modem_dte_init(const esp_modem_dte_config_t *config)
res = uart_set_rx_timeout(esp_dte->uart_port, 1);
MODEM_CHECK(res == ESP_OK, "set rx timeout failed", err_uart_config);

/* Set pattern interrupt, used to detect the end of a line. */
res = uart_enable_pattern_det_baud_intr(esp_dte->uart_port, '\n', 1, MIN_PATTERN_INTERVAL, MIN_POST_IDLE, MIN_PRE_IDLE);
/* Set pattern queue size */
esp_dte->pattern_queue_size = config->pattern_queue_size;
res |= uart_pattern_queue_reset(esp_dte->uart_port, config->pattern_queue_size);
/* Starting in command mode -> explicitly disable RX interrupt */
uart_disable_rx_intr(esp_dte->uart_port);
res = uart_set_rx_full_threshold(config->port_num, ESP_MODEM_UART_RX_FULL_THRESHOLD);
MODEM_CHECK(res == ESP_OK, "config rx full threshold failed", err_uart_config);

MODEM_CHECK(res == ESP_OK, "config uart pattern failed", err_uart_pattern);
/* Create Event loop */
esp_event_loop_args_t loop_args = {
.queue_size = ESP_MODEM_EVENT_QUEUE_SIZE,
Expand Down Expand Up @@ -553,8 +477,6 @@ modem_dte_t *esp_modem_dte_init(const esp_modem_dte_config_t *config)
err_sem1:
esp_event_loop_delete(esp_dte->event_loop_hdl);
err_eloop:
uart_disable_pattern_det_intr(esp_dte->uart_port);
err_uart_pattern:
uart_driver_delete(esp_dte->uart_port);
err_uart_config:
free(esp_dte->buffer);
Expand Down
7 changes: 0 additions & 7 deletions examples/protocols/pppos_client/main/Kconfig.projbuild
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,6 @@ menu "Example Configuration"
help
Length of UART event queue.

config EXAMPLE_MODEM_UART_PATTERN_QUEUE_SIZE
int "UART Pattern Queue Size"
range 10 40
default 20
help
Length of UART pattern queue.

config EXAMPLE_MODEM_UART_TX_BUFFER_SIZE
int "UART TX Buffer Size"
range 256 2048
Expand Down
3 changes: 1 addition & 2 deletions examples/protocols/pppos_client/main/pppos_client_main.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,10 @@ void app_main(void)
config.cts_io_num = CONFIG_EXAMPLE_MODEM_UART_CTS_PIN;
config.rx_buffer_size = CONFIG_EXAMPLE_MODEM_UART_RX_BUFFER_SIZE;
config.tx_buffer_size = CONFIG_EXAMPLE_MODEM_UART_TX_BUFFER_SIZE;
config.pattern_queue_size = CONFIG_EXAMPLE_MODEM_UART_PATTERN_QUEUE_SIZE;
config.event_queue_size = CONFIG_EXAMPLE_MODEM_UART_EVENT_QUEUE_SIZE;
config.event_task_stack_size = CONFIG_EXAMPLE_MODEM_UART_EVENT_TASK_STACK_SIZE;
config.event_task_priority = CONFIG_EXAMPLE_MODEM_UART_EVENT_TASK_PRIORITY;
config.line_buffer_size = CONFIG_EXAMPLE_MODEM_UART_RX_BUFFER_SIZE / 2;
config.dte_buffer_size = CONFIG_EXAMPLE_MODEM_UART_RX_BUFFER_SIZE / 2;

modem_dte_t *dte = esp_modem_dte_init(&config);
/* Register event handler */
Expand Down

0 comments on commit a90817c

Please sign in to comment.