From ea0df31e0067358ab2aaaf9e187bd5b096e104f2 Mon Sep 17 00:00:00 2001 From: Euripedes Rocha Date: Thu, 14 Sep 2023 14:00:42 +0200 Subject: [PATCH] fix: Move buffer initialization to set config When calling set config message buffers were not affected because the creation was handled on init. Closes https://github.com/espressif/esp-mqtt/issues/267 --- include/mqtt_client.h | 3 +++ lib/include/mqtt_config.h | 1 + lib/mqtt_msg.c | 2 +- mqtt_client.c | 46 +++++++++++++++++++++------------------ 4 files changed, 30 insertions(+), 22 deletions(-) diff --git a/include/mqtt_client.h b/include/mqtt_client.h index 1b4858bf..2b40a22f 100644 --- a/include/mqtt_client.h +++ b/include/mqtt_client.h @@ -597,6 +597,9 @@ esp_err_t esp_mqtt_client_destroy(esp_mqtt_client_handle_t client); * @brief Set configuration structure, typically used when updating the config * (i.e. on "before_connect" event * + * Notes: + * - When calling this function make sure to have all the intendend configurations + * set, otherwise default values are set. * @param client *MQTT* client handle * * @param config *MQTT* configuration structure diff --git a/lib/include/mqtt_config.h b/lib/include/mqtt_config.h index 409fb3c6..d96e2ad4 100644 --- a/lib/include/mqtt_config.h +++ b/lib/include/mqtt_config.h @@ -100,6 +100,7 @@ #define MQTT_ENABLE_SSL CONFIG_MQTT_TRANSPORT_SSL #define MQTT_ENABLE_WS CONFIG_MQTT_TRANSPORT_WEBSOCKET #define MQTT_ENABLE_WSS CONFIG_MQTT_TRANSPORT_WEBSOCKET_SECURE +#define MQTT_DEFAULT_RETRANSMIT_TIMEOUT_MS 1000 #ifdef CONFIG_MQTT_EVENT_QUEUE_SIZE #define MQTT_EVENT_QUEUE_SIZE CONFIG_MQTT_EVENT_QUEUE_SIZE diff --git a/lib/mqtt_msg.c b/lib/mqtt_msg.c index 4e9a2f40..582fc92d 100644 --- a/lib/mqtt_msg.c +++ b/lib/mqtt_msg.c @@ -618,7 +618,7 @@ int mqtt_has_valid_msg_hdr(uint8_t *buffer, size_t length) esp_err_t mqtt_msg_buffer_init(mqtt_connection_t *connection, int buffer_size) { - memset(connection, 0, sizeof(mqtt_connection_t)); + memset(&connection->outbound_message, 0, sizeof(mqtt_message_t)); connection->buffer = (uint8_t *)calloc(buffer_size, sizeof(uint8_t)); if (!connection->buffer) { return ESP_ERR_NO_MEM; diff --git a/mqtt_client.c b/mqtt_client.c index 88dbcdad..e98fca08 100644 --- a/mqtt_client.c +++ b/mqtt_client.c @@ -383,9 +383,26 @@ esp_err_t esp_mqtt_set_config(esp_mqtt_client_handle_t client, const esp_mqtt_cl }); } + mqtt_msg_buffer_destroy(&client->mqtt_state.connection); + int buffer_size = config->buffer.size; + if (buffer_size <= 0) { + buffer_size = MQTT_BUFFER_SIZE_BYTE; + } + + // use separate value for output buffer size if configured + int out_buffer_size = config->buffer.out_size > 0 ? config->buffer.out_size : buffer_size; + if (mqtt_msg_buffer_init(&client->mqtt_state.connection, out_buffer_size) != ESP_OK) { + goto _mqtt_set_config_failed; + } + + free(client->mqtt_state.in_buffer); + client->mqtt_state.in_buffer = (uint8_t *)malloc(buffer_size); + ESP_MEM_CHECK(TAG, client->mqtt_state.in_buffer, goto _mqtt_set_config_failed); + client->mqtt_state.in_buffer_length = buffer_size; + client->config->message_retransmit_timeout = config->session.message_retransmit_timeout; if (config->session.message_retransmit_timeout <= 0) { - client->config->message_retransmit_timeout = 1000; + client->config->message_retransmit_timeout = MQTT_DEFAULT_RETRANSMIT_TIMEOUT_MS; } client->config->task_prio = config->task.priority; @@ -599,6 +616,8 @@ void esp_mqtt_destroy_config(esp_mqtt_client_handle_t client) if (client->config == NULL) { return; } + free(client->mqtt_state.in_buffer); + mqtt_msg_buffer_destroy(&client->mqtt_state.connection); free(client->config->host); free(client->config->uri); free(client->config->path); @@ -807,6 +826,11 @@ static bool create_client_data(esp_mqtt_client_handle_t client) client->api_lock = xSemaphoreCreateRecursiveMutex(); ESP_MEM_CHECK(TAG, client->api_lock, return false); + client->outbox = outbox_init(); + ESP_MEM_CHECK(TAG, client->outbox, return false); + client->status_bits = xEventGroupCreate(); + ESP_MEM_CHECK(TAG, client->status_bits, return false); + return true; } @@ -824,24 +848,6 @@ esp_mqtt_client_handle_t esp_mqtt_client_init(const esp_mqtt_client_config_t *co if (!create_client_data(client)) { goto _mqtt_init_failed; } - int buffer_size = config->buffer.size; - if (buffer_size <= 0) { - buffer_size = MQTT_BUFFER_SIZE_BYTE; - } - - // use separate value for output buffer size if configured - int out_buffer_size = config->buffer.out_size > 0 ? config->buffer.out_size : buffer_size; - if (mqtt_msg_buffer_init(&client->mqtt_state.connection, out_buffer_size) != ESP_OK) { - goto _mqtt_init_failed; - } - - client->mqtt_state.in_buffer = (uint8_t *)malloc(buffer_size); - ESP_MEM_CHECK(TAG, client->mqtt_state.in_buffer, goto _mqtt_init_failed); - client->mqtt_state.in_buffer_length = buffer_size; - client->outbox = outbox_init(); - ESP_MEM_CHECK(TAG, client->outbox, goto _mqtt_init_failed); - client->status_bits = xEventGroupCreate(); - ESP_MEM_CHECK(TAG, client->status_bits, goto _mqtt_init_failed); if (esp_mqtt_set_config(client, config) != ESP_OK) { goto _mqtt_init_failed; @@ -890,8 +896,6 @@ esp_err_t esp_mqtt_client_destroy(esp_mqtt_client_handle_t client) if (client->status_bits) { vEventGroupDelete(client->status_bits); } - free(client->mqtt_state.in_buffer); - mqtt_msg_buffer_destroy(&client->mqtt_state.connection); if (client->api_lock) { vSemaphoreDelete(client->api_lock); }