From 9b6336a9d413a65955b0b52715f5a38e1d23d4a4 Mon Sep 17 00:00:00 2001 From: 0xFEEDC0DE64 Date: Thu, 15 Apr 2021 21:20:08 +0200 Subject: [PATCH] Performance improvements, better logging, destructor finally implemented --- src/asynchttprequest.cpp | 284 ++++++++++++++++++++++++++++++--------- src/asynchttprequest.h | 20 +-- 2 files changed, 230 insertions(+), 74 deletions(-) diff --git a/src/asynchttprequest.cpp b/src/asynchttprequest.cpp index d8d2984..c5436b6 100644 --- a/src/asynchttprequest.cpp +++ b/src/asynchttprequest.cpp @@ -5,6 +5,7 @@ // system includes #include +#include #include #include @@ -23,7 +24,10 @@ constexpr const char * const TAG = "ASYNC_HTTP"; constexpr int TASK_RUNNING = BIT0; constexpr int START_REQUEST_BIT = BIT1; -constexpr int REQUEST_FINISHED_BIT = BIT2; +constexpr int REQUEST_RUNNING_BIT = BIT2; +constexpr int REQUEST_FINISHED_BIT = BIT3; +constexpr int END_TASK_BIT = BIT4; +constexpr int TASK_ENDED = BIT5; } // namespace AsyncHttpRequest::AsyncHttpRequest(const char *taskName, espcpputils::CoreAffinity coreAffinity) : @@ -31,21 +35,132 @@ AsyncHttpRequest::AsyncHttpRequest(const char *taskName, espcpputils::CoreAffini m_coreAffinity{coreAffinity} { assert(eventGroup.handle); - - eventGroup.clearBits(TASK_RUNNING); - eventGroup.clearBits(START_REQUEST_BIT); - eventGroup.clearBits(REQUEST_FINISHED_BIT); - - if (const auto failed = startTask()) - { - ESP_LOGE(TAG, "%s", failed->c_str()); - } } AsyncHttpRequest::~AsyncHttpRequest() { - // TODO exit task - abort(); + endTask(); +} + +std::optional AsyncHttpRequest::startTask() +{ + if (const auto bits = eventGroup.getBits(); + bits & TASK_RUNNING || taskHandle) + { + constexpr auto msg = "task already started"; + ESP_LOGW(TAG, "%s", msg); + return msg; + } + + eventGroup.clearBits(TASK_RUNNING | START_REQUEST_BIT | REQUEST_RUNNING_BIT | REQUEST_FINISHED_BIT | END_TASK_BIT | TASK_ENDED); + + const auto result = espcpputils::createTask(requestTask, m_taskName, 4096, this, 10, &taskHandle, m_coreAffinity); + if (result != pdPASS) + { + auto msg = std::string{"failed creating http task "} + std::to_string(result); + ESP_LOGE(TAG, "%s", msg.c_str()); + return msg; + } + + if (!taskHandle) + { + constexpr auto msg = "http task handle is null"; + ESP_LOGW(TAG, "%s", msg); + return msg; + } + + ESP_LOGD(TAG, "created http task %s", m_taskName); + + if (const auto bits = eventGroup.waitBits(TASK_RUNNING, false, false, std::chrono::ceil(1s).count()); + bits & TASK_RUNNING) + return std::nullopt; + + ESP_LOGW(TAG, "http task %s TASK_RUNNING bit not yet set...", m_taskName); + + while (true) + if (const auto bits = eventGroup.waitBits(TASK_RUNNING, false, false, portMAX_DELAY); + bits & TASK_RUNNING) + break; + + return std::nullopt; +} + +std::optional AsyncHttpRequest::endTask() +{ + if (const auto bits = eventGroup.getBits(); + !(bits & TASK_RUNNING)) + return std::nullopt; + else if (bits & END_TASK_BIT) + { + constexpr auto msg = "Another end request is already pending"; + ESP_LOGE(TAG, "%s", msg); + return msg; + } + + eventGroup.setBits(END_TASK_BIT); + + if (const auto bits = eventGroup.waitBits(TASK_ENDED, true, false, std::chrono::ceil(1s).count()); + bits & TASK_ENDED) + { + ESP_LOGD(TAG, "http task %s ended", m_taskName); + return std::nullopt; + } + + ESP_LOGW(TAG, "http task %s TASK_ENDED bit not yet set...", m_taskName); + + while (true) + if (const auto bits = eventGroup.waitBits(TASK_ENDED, true, false, portMAX_DELAY); + bits & TASK_ENDED) + break; + + ESP_LOGD(TAG, "http task %s ended", m_taskName); + + return std::nullopt; +} + +std::optional AsyncHttpRequest::createClient(const std::string &url) +{ + if (client) + { + constexpr auto msg = "client already created"; + ESP_LOGE(TAG, "%s", msg); + return msg; + } + + esp_http_client_config_t config{}; + config.url = url.c_str(); + config.event_handler = httpEventHandler; + config.user_data = this; + + client = espcpputils::http_client{&config}; + + if (!client) + { + constexpr auto msg = "http client could not be constructed"; + ESP_LOGE(TAG, "%s", msg); + return msg; + } + + ESP_LOGD(TAG, "created http client %s", m_taskName); + + return std::nullopt; +} + +std::optional AsyncHttpRequest::deleteClient() +{ + if (!client) + return std::nullopt; + + if (inProgress()) + { + constexpr auto msg = "another request still in progress"; + ESP_LOGW(TAG, "%s", msg); + return msg; + } + + client = {}; + + return std::nullopt; } std::optional AsyncHttpRequest::start(const std::string &url) @@ -56,28 +171,39 @@ std::optional AsyncHttpRequest::start(const std::string &url) return *failed; } + if (inProgress()) + { + constexpr auto msg = "another request still in progress"; + ESP_LOGW(TAG, "%s", msg); + return msg; + } + + if (!client) + { + if (const auto failed = createClient(url)) + return *failed; + } + else + { + const auto result = client.set_url(url.c_str()); + ESP_LOG_LEVEL_LOCAL((result == ESP_OK ? ESP_LOG_DEBUG : ESP_LOG_ERROR), TAG, "client.set_url() returned: %s (%s)", esp_err_to_name(result), url.c_str()); + if (result != ESP_OK) + return std::string{"client.set_url() failed: "} + esp_err_to_name(result) + " (" + url + ')'; + } + buf.clear(); - this->url = url; - config = esp_http_client_config_t{}; - config.url = url.c_str(); - config.event_handler = httpEventHandler; - config.user_data = this; - - request.construct(&config); - - auto helper = cpputils::makeCleanupHelper([&request=request](){ request.destruct(); }); - - if (!request->handle) - return "request could not be constructed"; - - eventGroup.clearBits(REQUEST_FINISHED_BIT); + clearFinished(); eventGroup.setBits(START_REQUEST_BIT); - helper.disarm(); // this stops the helper from destructing the request return std::nullopt; } +bool AsyncHttpRequest::inProgress() const +{ + return eventGroup.getBits() & (START_REQUEST_BIT | REQUEST_RUNNING_BIT); +} + bool AsyncHttpRequest::finished() const { return eventGroup.getBits() & REQUEST_FINISHED_BIT; @@ -85,24 +211,37 @@ bool AsyncHttpRequest::finished() const std::optional AsyncHttpRequest::failed() const { + if (const auto bits = eventGroup.getBits(); + bits & REQUEST_RUNNING_BIT) + { + constexpr auto msg = "request still running"; + ESP_LOGW(TAG, "%s", msg); + return msg; + } + else if (!(bits & REQUEST_FINISHED_BIT)) + { + constexpr auto msg = "request not finished"; + ESP_LOGW(TAG, "%s", msg); + return msg; + } + if (result != ESP_OK) return std::string{"http request failed: "} + esp_err_to_name(result); - if (request->get_status_code() != 200) - return std::string{"http request failed: "} + std::to_string(request->get_status_code()); + if (statusCode != HttpStatus_Ok) + return std::string{"http request failed: "} + std::to_string(statusCode); return std::nullopt; } -void AsyncHttpRequest::cleanup() +void AsyncHttpRequest::clearFinished() { - request.destruct(); - buf.clear(); + eventGroup.clearBits(REQUEST_FINISHED_BIT); } esp_err_t AsyncHttpRequest::httpEventHandler(esp_http_client_event_t *evt) { - auto _this = (AsyncHttpRequest*)evt->user_data; + auto _this = reinterpret_cast(evt->user_data); assert(_this); @@ -126,10 +265,12 @@ esp_err_t AsyncHttpRequest::httpEventHandler(esp_http_client_event_t *evt) } break; case HTTP_EVENT_ON_DATA: - //assert(!_this->request->is_chunked_response()); - - if (evt->data && evt->data_len) - _this->buf += std::string((const char *)evt->data, evt->data_len); + if (!evt->data) + ESP_LOGW(TAG, "handler with invalid data ptr"); + else if (evt->data_len <= 0) + ESP_LOGW(TAG, "handler with invalid data_len %i", evt->data_len); + else + _this->buf += std::string_view((const char *)evt->data, evt->data_len); break; default:; @@ -140,47 +281,58 @@ esp_err_t AsyncHttpRequest::httpEventHandler(esp_http_client_event_t *evt) void AsyncHttpRequest::requestTask(void *ptr) { - auto helper = cpputils::makeCleanupHelper([](){ vTaskDelete(NULL); }); - - auto _this = (AsyncHttpRequest*)ptr; + auto _this = reinterpret_cast(ptr); assert(_this); + _this->eventGroup.setBits(TASK_RUNNING); - auto helper2 = cpputils::makeCleanupHelper([=](){ _this->eventGroup.clearBits(TASK_RUNNING); }); + + // cleanup on task exit + auto helper = cpputils::makeCleanupHelper([&](){ + _this->eventGroup.clearBits(TASK_RUNNING); + _this->eventGroup.setBits(TASK_ENDED); + _this->taskHandle = NULL; + vTaskDelete(NULL); + }); while (true) { + if (const auto bits = _this->eventGroup.waitBits(START_REQUEST_BIT|END_TASK_BIT, true, false, portMAX_DELAY); + bits & END_TASK_BIT) + break; + else if (!(bits & START_REQUEST_BIT)) + continue; + + assert(_this->client); + { - const auto bits = _this->eventGroup.waitBits(START_REQUEST_BIT, true, true, espcpputils::ticks{1s}.count()); - if (!(bits & START_REQUEST_BIT)) - continue; + const auto bits = _this->eventGroup.getBits(); + assert(!(bits & START_REQUEST_BIT)); + assert(!(bits & REQUEST_RUNNING_BIT)); + assert(!(bits & REQUEST_FINISHED_BIT)); } - assert(_this->request.constructed()); - assert(!(_this->eventGroup.getBits() & REQUEST_FINISHED_BIT)); + _this->eventGroup.setBits(REQUEST_RUNNING_BIT); + + auto helper2 = cpputils::makeCleanupHelper([&](){ + _this->eventGroup.clearBits(REQUEST_RUNNING_BIT); + _this->eventGroup.setBits(REQUEST_FINISHED_BIT); + }); - do { - _this->result = _this->request->perform(); + esp_err_t result; + do + result = _this->client.perform(); + while (result == EAGAIN || result == EINPROGRESS); + ESP_LOG_LEVEL_LOCAL((result == ESP_OK ? ESP_LOG_VERBOSE : ESP_LOG_ERROR), TAG, "client.perform() returned: %s", esp_err_to_name(result)); + _this->result = result; + _this->statusCode = _this->client.get_status_code(); } - while (_this->result == EAGAIN || _this->result == EINPROGRESS); - _this->eventGroup.setBits(REQUEST_FINISHED_BIT); + // workaround for esp-idf bug, every request after the first one fails with ESP_ERR_HTTP_FETCH_HEADER + const auto result = _this->client.close(); + ESP_LOG_LEVEL_LOCAL((result == ESP_OK ? ESP_LOG_VERBOSE : ESP_LOG_ERROR), TAG, "client.close() returned: %s", esp_err_to_name(result)); + if (result != ESP_OK) + _this->client = {}; } } - -std::optional AsyncHttpRequest::startTask() -{ - if (taskHandle) - return "task already started"; - - const auto result = espcpputils::createTask(requestTask, m_taskName, 4096, this, 10, &taskHandle, m_coreAffinity); - if (result != pdPASS) - return std::string{"failed creating http task "} + std::to_string(result); - - if (!taskHandle) - return "http task handle is null"; - - ESP_LOGD(TAG, "created http task"); - return std::nullopt; -} diff --git a/src/asynchttprequest.h b/src/asynchttprequest.h index 6f18777..455e5ed 100644 --- a/src/asynchttprequest.h +++ b/src/asynchttprequest.h @@ -10,7 +10,6 @@ #include // local includes -#include "delayedconstruction.h" #include "wrappers/http_client.h" #include "wrappers/event_group.h" #include "taskutils.h" @@ -21,29 +20,34 @@ public: AsyncHttpRequest(const char *taskName="httpRequestTask", espcpputils::CoreAffinity coreAffinity=espcpputils::CoreAffinity::Core1); ~AsyncHttpRequest(); + std::optional startTask(); + std::optional endTask(); + + std::optional createClient(const std::string &url); + std::optional deleteClient(); + std::optional start(const std::string &url); + bool inProgress() const; + bool finished() const; std::optional failed() const; + void clearFinished(); + const std::string &buffer() const { return buf; } std::string &&takeBuffer() { return std::move(buf); } - void cleanup(); - private: static esp_err_t httpEventHandler(esp_http_client_event_t *evt); static void requestTask(void *ptr); - std::optional startTask(); - cpputils::DelayedConstruction request; + espcpputils::http_client client; std::string buf; TaskHandle_t taskHandle{NULL}; espcpputils::event_group eventGroup; esp_err_t result{}; - - std::string url; - esp_http_client_config_t config; + int statusCode{}; const char * const m_taskName; const espcpputils::CoreAffinity m_coreAffinity;