From 5e79c992d9d2a8507b6f20ec6dddbb60e72c0497 Mon Sep 17 00:00:00 2001 From: jrhoffa Date: Sun, 18 Sep 2022 15:41:17 -0700 Subject: [PATCH] Fix memory leak in pattern states Also simplified sparkle pattern memory --- main/device.cpp | 2 +- main/leds.hpp | 7 +++++-- main/main.cpp | 33 ++++++++++++++++++++------------- main/patterns/sparkle.cpp | 29 ++++++++++++++--------------- 4 files changed, 40 insertions(+), 31 deletions(-) diff --git a/main/device.cpp b/main/device.cpp index f58ee42..6aafeef 100644 --- a/main/device.cpp +++ b/main/device.cpp @@ -125,7 +125,7 @@ void Device::publish_state_locked() { ESP_ERROR_CHECK(esp_mqtt_client_publish( client, state_topic.c_str(), config, 0, 0, 0 )); - cJSON_Delete(json); cJSON_free(config); + cJSON_Delete(json); } } diff --git a/main/leds.hpp b/main/leds.hpp index 3520c66..3a23040 100644 --- a/main/leds.hpp +++ b/main/leds.hpp @@ -16,9 +16,11 @@ constexpr int shift = 16; class Pattern { public: - virtual ~Pattern() {}; + virtual ~Pattern() = default; - struct State {}; + struct State { + virtual ~State() = default; + }; virtual State* start(int) const = 0; @@ -29,6 +31,7 @@ public: class LEDStrip { public: LEDStrip(int length = 0); + ~LEDStrip() { if (state) delete state; } void step(); diff --git a/main/main.cpp b/main/main.cpp index 5bc76a3..9755e9e 100644 --- a/main/main.cpp +++ b/main/main.cpp @@ -88,7 +88,8 @@ static void log_dir(const std::string &path) { constexpr char config_path[] = "/spiffs/blinky.json"; -//#define PROFILE +//#define PROFILE_PERF +//#define PROFILE_MEM // Entry Point @@ -135,7 +136,7 @@ extern "C" void app_main(void) { ESP_LOGI(TAG, "Configuring LEDs"); device.lock(); - const std::string &effect = device.get_effect(); + const std::string effect = device.get_effect(); bool is_on = device.is_on(); int length = device.get_strip_length(); cJSON *json = device.make_json_config_locked(); @@ -174,24 +175,30 @@ extern "C" void app_main(void) { ESP_LOGI(TAG, "Starting animation"); int period_us = 1000000 / 60; - int64_t start = time_us(); - int64_t target_us = start; -#ifdef PROFILE + int64_t target_us = time_us(); + int64_t profile_start = target_us; +#ifdef PROFILE_PERF int64_t total_delay_us = 0; int n_delays = 0; int64_t after_sleep_us = 0; int n_processes = 0; int64_t total_process_us = 0; -#endif // PROFILE +#endif // PROFILE_PERF while (true) { int64_t now = time_us(); -#ifdef PROFILE +#ifdef PROFILE_PERF if (after_sleep_us > 0) { total_process_us += now - after_sleep_us; ++n_processes; } +#endif // PROFILE_PERF - if (now - start >= 5000000) { + if (now - profile_start >= 5000000) { +#ifdef PROFILE_MEM + heap_caps_print_heap_info(MALLOC_CAP_8BIT); +#endif // PROFILE_MEM + +#ifdef PROFILE_PERF ESP_LOGI(TAG, "Average delay: %f ms", (double)total_delay_us / (n_delays * 1000)); total_delay_us = 0; n_delays = 0; @@ -199,10 +206,11 @@ extern "C" void app_main(void) { ESP_LOGI(TAG, "Average processing: %f us", (double)total_process_us / n_processes); total_process_us = 0; n_processes = 0; +#endif // PROFILE_PERF - start = now; + profile_start = now; } -#endif // PROFILE + int64_t delay_us = target_us - now; if (delay_us < 0) delay_us = 0; if (device.wait(delay_us / 1000)) { @@ -211,12 +219,11 @@ extern "C" void app_main(void) { } target_us = now + delay_us + period_us; -#ifdef PROFILE +#ifdef PROFILE_PERF after_sleep_us = time_us(); - total_delay_us += (after_sleep_us - now); ++n_delays; -#endif // PROFILE +#endif // PROFILE_PERF LEDs->step(); LEDs->show(); } diff --git a/main/patterns/sparkle.cpp b/main/patterns/sparkle.cpp index 7dfdf80..871ef73 100644 --- a/main/patterns/sparkle.cpp +++ b/main/patterns/sparkle.cpp @@ -17,6 +17,7 @@ int norm(int mean) { } struct Sparkle { + Sparkle() : start_us(0), end_us(0), pos(0) {} Sparkle(int64_t time_us, int mean_dur_us, int len): start_us(time_us), end_us(time_us + (mean_dur_us)), pos(esp_random() % len) {} @@ -46,7 +47,11 @@ Color fade(Color c, unsigned int i) { class SparklePattern : public Pattern { private: struct State : Pattern::State { - std::list sparkles; + State(int n) : n_sparkles(n), sparkles(new Sparkle[n]) {} + ~State() { delete[] sparkles; } + + int n_sparkles; + Sparkle *sparkles; }; public: @@ -55,29 +60,23 @@ public: delay_us(_delay_ms * 1000), density(_density) {} - State* start(int len) const { return new State(); } + State* start(int len) const { return new State((len / density) + 1); } void step(Color colors[], int len, Pattern::State *_state) const { State *state = (State*)_state; int64_t now_us = time_us(); - int64_t start_us = now_us; - for ( int open = (len / density) + 1 - state->sparkles.size(); - open > 0; - --open, start_us += delay_us - ) { - state->sparkles.push_front(Sparkle(start_us, duration_us, len)); - } - memset(colors, 0, sizeof(*colors) * len); - for (auto iter = state->sparkles.begin(); iter != state->sparkles.end(); ) { - if (iter->expired(now_us)) { - state->sparkles.erase(iter++); + int64_t start_us = now_us; + for (int i = 0; i < state->n_sparkles; ++i, start_us += delay_us) { + Sparkle &sparkle = state->sparkles[i]; + + if (sparkle.expired(now_us)) { + sparkle = Sparkle(start_us, duration_us, len); } else { - colors[iter->pos] = fade(color, iter->intensity_at(now_us)); - ++iter; + colors[sparkle.pos] = fade(color, sparkle.intensity_at(now_us)); } } }