-
Notifications
You must be signed in to change notification settings - Fork 278
Cvs 194742 progress bar fix #4560
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
256c468
e31e15f
14d3f3c
1fa4c0e
d3279b0
018f715
9e0bf98
16e1199
a608871
5f43cbb
8b72d76
6d6fac1
69fdb7c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,10 +15,13 @@ | |
| //***************************************************************************** | ||
| #include "curl_downloader.hpp" | ||
|
|
||
| #include <array> | ||
| #include <cstdlib> | ||
| #include <ctime> | ||
| #include <filesystem> | ||
| #include <iostream> | ||
| #include <memory> | ||
| #include <mutex> | ||
| #include <string> | ||
|
|
||
| #include <curl/curl.h> | ||
|
|
@@ -30,7 +33,7 @@ | |
|
|
||
| namespace ovms { | ||
|
|
||
| static const char* sizeUnits[] = {"B", "KB", "MB", "GB", "TB", NULL}; | ||
| static constexpr std::array<const char*, 5> sizeUnits = {"B", "KB", "MB", "GB", "TB"}; | ||
|
|
||
| static void print_download_speed_info(size_t received_size, size_t elapsed_time) { | ||
| double recv_len = (double)received_size; | ||
|
|
@@ -39,20 +42,47 @@ static void print_download_speed_info(size_t received_size, size_t elapsed_time) | |
| rate = elapsed ? recv_len / elapsed : received_size; | ||
|
|
||
| size_t rate_unit_idx = 0; | ||
| while (rate > 1000 && sizeUnits[rate_unit_idx + 1]) { | ||
| while (rate > 1000 && rate_unit_idx + 1 < sizeUnits.size()) { | ||
| rate /= 1000.0; | ||
| rate_unit_idx++; | ||
| } | ||
| printf(" [%.2f %s/s] ", rate, sizeUnits[rate_unit_idx]); | ||
| printf(" [%.2f %s/s] ", rate, sizeUnits.at(rate_unit_idx)); | ||
| } | ||
|
|
||
| int computeProgressBarCells(size_t count, size_t max, int barWidth) { | ||
| if (max == 0 || barWidth <= 0) { | ||
| return 0; | ||
| } | ||
| const double ratio = static_cast<double>(count) / static_cast<double>(max); | ||
| if (ratio <= 0.0) { | ||
| return 0; | ||
| } else if (ratio >= 1.0) { | ||
| return barWidth; | ||
| } | ||
| return static_cast<int>(ratio * barWidth); | ||
| } | ||
|
|
||
| static void print_progress(size_t count, size_t max, bool first_run, size_t elapsed_time) { | ||
| // A response with no Content-Length reports dltotal == 0, so there is no ratio to show | ||
| if (max == 0) { | ||
| double received = (double)count; | ||
| size_t receivedUnitId = 0; | ||
| while (received > 1000 && receivedUnitId + 1 < sizeUnits.size()) { | ||
| received /= 1000.0; | ||
| receivedUnitId++; | ||
|
mzegla marked this conversation as resolved.
|
||
| } | ||
| printf("\rProgress: %.2f %s downloaded, total size unknown", received, sizeUnits.at(receivedUnitId)); | ||
| print_download_speed_info(count, elapsed_time); | ||
| fflush(stdout); | ||
| return; | ||
| } | ||
|
|
||
| float progress = (float)count / max; | ||
| if (!first_run && progress < 0.01 && count > 0) | ||
| return; | ||
|
|
||
| const int bar_width = 50; | ||
| int bar_length = progress * bar_width; | ||
| const int bar_length = computeProgressBarCells(count, max, bar_width); | ||
|
|
||
| printf("\rProgress: ["); | ||
| int i; | ||
|
|
@@ -64,11 +94,11 @@ static void print_progress(size_t count, size_t max, bool first_run, size_t elap | |
| } | ||
| size_t totalSizeUnitId = 0; | ||
| double totalSize = max; | ||
| while (totalSize > 1000 && sizeUnits[totalSizeUnitId + 1]) { | ||
| while (totalSize > 1000 && totalSizeUnitId + 1 < sizeUnits.size()) { | ||
| totalSize /= 1000.0; | ||
| totalSizeUnitId++; | ||
| } | ||
| printf("] %.2f%% of %.2f %s", progress * 100, totalSize, sizeUnits[totalSizeUnitId]); | ||
| printf("] %.2f%% of %.2f %s", progress * 100, totalSize, sizeUnits.at(totalSizeUnitId)); | ||
| print_download_speed_info(count, elapsed_time); | ||
| if (progress == 1.0) | ||
| printf("\n"); | ||
|
|
@@ -117,6 +147,25 @@ static size_t file_write_callback(void* buffer, size_t size, size_t nmemb, void* | |
| } \ | ||
| } while (0) | ||
|
|
||
| // Keep one balanced libcurl global initialization for this downloader's process lifetime. | ||
| // The previous per-call guard held a null unique_ptr, so its deleter never ran and every | ||
| // download added another unmatched curl_global_init() call. | ||
| static Status ensureCurlGlobalInit() { | ||
| static std::once_flag initFlag; | ||
| static CURLcode initResult = CURLE_OK; | ||
| std::call_once(initFlag, []() { | ||
| initResult = curl_global_init(CURL_GLOBAL_DEFAULT); | ||
| if (initResult == CURLE_OK) { | ||
| std::atexit([]() { curl_global_cleanup(); }); | ||
| } | ||
|
rasapala marked this conversation as resolved.
|
||
| }); | ||
| if (initResult != CURLE_OK) { | ||
| SPDLOG_ERROR("curl error: {}. Error code: {}", curl_easy_strerror(initResult), (int)initResult); | ||
| return StatusCode::INTERNAL_ERROR; | ||
| } | ||
| return StatusCode::OK; | ||
| } | ||
|
Comment on lines
+150
to
+167
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means that every unit test that did start OVMS before did curl init and curl cleanup. So ovms behaves now more differently in ovms gtest scenarios than in prod. Why could we not just move this initialization to pull module? Why we can't follow pattern used in httpservermodule.cpp? Then we could dispose whole part of curl_global_init from functions used here/.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. First question - yes.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is used only in pull_module. The other place we use it is: but its entirely different curl call with their own guards. Why did previous solution not work? |
||
|
|
||
| struct ProgressData { | ||
| time_t started_download; | ||
| time_t last_print_time; | ||
|
|
@@ -159,9 +208,10 @@ Status downloadFileWithCurl(const std::string& url, const std::string& filePath, | |
| std::string agentString = std::string(PROJECT_NAME) + "/" + std::string(PROJECT_VERSION); | ||
|
|
||
| CURL* curl = nullptr; | ||
| CHECK_CURL_CALL(curl_global_init(CURL_GLOBAL_DEFAULT)); | ||
| auto globalCurlGuard = std::unique_ptr<void, void (*)(void*)>( | ||
| nullptr, [](void*) { curl_global_cleanup(); }); | ||
| auto initStatus = ensureCurlGlobalInit(); | ||
| if (!initStatus.ok()) { | ||
| return initStatus; | ||
| } | ||
| curl = curl_easy_init(); | ||
| if (!curl) { | ||
| SPDLOG_ERROR("Failed to initialize cURL."); | ||
|
|
@@ -211,9 +261,10 @@ Status fetchUrlToString(const std::string& url, const std::string& authToken, st | |
| std::string agentString = std::string(PROJECT_NAME) + "/" + std::string(PROJECT_VERSION); | ||
|
|
||
| CURL* curl = nullptr; | ||
| CHECK_CURL_CALL(curl_global_init(CURL_GLOBAL_DEFAULT)); | ||
| auto globalCurlGuard = std::unique_ptr<void, void (*)(void*)>( | ||
| nullptr, [](void*) { curl_global_cleanup(); }); | ||
| auto initStatus = ensureCurlGlobalInit(); | ||
| if (!initStatus.ok()) { | ||
| return initStatus; | ||
| } | ||
| curl = curl_easy_init(); | ||
| if (!curl) { | ||
| SPDLOG_ERROR("Failed to initialize cURL."); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to include mutex in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::once_flag and std::call_once