From a476647d70ad2451323a00379fa0d811f361f87c Mon Sep 17 00:00:00 2001 From: Kyle Reed <3761006+kallanreed@users.noreply.github.com> Date: Mon, 21 Aug 2023 01:17:23 -0700 Subject: [PATCH] Don't use raw new/delete (#1398) * Use unique_ptr in ui_btngrid * Use unique_ptr for ui_menu * Use unique_ptr for rssi_dma * Use unique_ptr for painter --- firmware/application/ui/ui_btngrid.cpp | 35 +++++++++------------ firmware/application/ui/ui_btngrid.hpp | 8 +++-- firmware/application/ui/ui_menu.cpp | 26 ++++++--------- firmware/application/ui/ui_menu.hpp | 11 ++++--- firmware/baseband/proc_spectrum_painter.cpp | 33 ++++++++++--------- firmware/baseband/rssi_dma.cpp | 15 ++++----- firmware/baseband/touch_dma.cpp | 4 --- 7 files changed, 61 insertions(+), 71 deletions(-) diff --git a/firmware/application/ui/ui_btngrid.cpp b/firmware/application/ui/ui_btngrid.cpp index d43e41b8..4f29e0fe 100644 --- a/firmware/application/ui/ui_btngrid.cpp +++ b/firmware/application/ui/ui_btngrid.cpp @@ -33,7 +33,6 @@ BtnGridView::BtnGridView( bool keep_highlight) : keep_highlight{keep_highlight} { set_parent_rect(new_parent_rect); - set_focusable(true); signal_token_tick_second = rtc_time::signal_tick_second += [this]() { @@ -47,10 +46,6 @@ BtnGridView::BtnGridView( BtnGridView::~BtnGridView() { rtc_time::signal_tick_second -= signal_token_tick_second; - - for (auto item : menu_item_views) { - delete item; - } } void BtnGridView::set_max_rows(int rows) { @@ -68,31 +63,30 @@ void BtnGridView::set_parent_rect(const Rect new_parent_rect) { arrow_more.set_parent_rect({228, (Coord)(displayed_max * button_h), 8, 8}); displayed_max *= rows_; - // TODO: Clean this up :( - if (menu_item_views.size()) { - for (auto item : menu_item_views) { - remove_child(item); - delete item; - } + // Delete any existing buttons. + if (!menu_item_views.empty()) { + for (auto& item : menu_item_views) + remove_child(item.get()); + menu_item_views.clear(); } - button_w = 240 / rows_; + button_w = screen_width / rows_; for (size_t c = 0; c < displayed_max; c++) { - auto item = new NewButton{}; - menu_item_views.push_back(item); - add_child(item); - + auto item = std::make_unique(); + add_child(item.get()); item->set_parent_rect({(int)(c % rows_) * button_w, (int)(c / rows_) * button_h, button_w, button_h}); + + menu_item_views.push_back(std::move(item)); } update_items(); } -void BtnGridView::set_arrow_enabled(bool new_value) { - if (new_value) { +void BtnGridView::set_arrow_enabled(bool enabled) { + if (enabled) { add_child(&arrow_more); } else { remove_child(&arrow_more); @@ -118,6 +112,7 @@ void BtnGridView::add_items(std::initializer_list new_items) { for (auto item : new_items) { menu_items.push_back(item); } + update_items(); } @@ -130,7 +125,7 @@ void BtnGridView::update_items() { } else more = false; - for (NewButton* item : menu_item_views) { + for (auto& item : menu_item_views) { if ((i + offset) >= menu_items.size()) { item->hidden(true); item->set_text(" "); @@ -152,7 +147,7 @@ void BtnGridView::update_items() { } NewButton* BtnGridView::item_view(size_t index) const { - return menu_item_views[index]; + return menu_item_views[index].get(); } bool BtnGridView::set_highlighted(int32_t new_value) { diff --git a/firmware/application/ui/ui_btngrid.hpp b/firmware/application/ui/ui_btngrid.hpp index 9d62e85a..82b350de 100644 --- a/firmware/application/ui/ui_btngrid.hpp +++ b/firmware/application/ui/ui_btngrid.hpp @@ -31,8 +31,10 @@ #include "signal.hpp" #include -#include #include +#include +#include +#include namespace ui { @@ -62,7 +64,7 @@ class BtnGridView : public View { uint32_t highlighted_index(); void set_parent_rect(const Rect new_parent_rect) override; - void set_arrow_enabled(bool new_value); + void set_arrow_enabled(bool enabled); void on_focus() override; void on_blur() override; bool on_key(const KeyEvent event) override; @@ -77,7 +79,7 @@ class BtnGridView : public View { SignalToken signal_token_tick_second{}; std::vector menu_items{}; - std::vector menu_item_views{}; + std::vector> menu_item_views{}; Image arrow_more{ {228, 320 - 8, 8, 8}, diff --git a/firmware/application/ui/ui_menu.cpp b/firmware/application/ui/ui_menu.cpp index 945a1ee4..28ae1867 100644 --- a/firmware/application/ui/ui_menu.cpp +++ b/firmware/application/ui/ui_menu.cpp @@ -103,10 +103,6 @@ MenuView::MenuView( MenuView::~MenuView() { rtc_time::signal_tick_second -= signal_token_tick_second; - - for (auto item : menu_item_views) { - delete item; - } } void MenuView::set_parent_rect(const Rect new_parent_rect) { @@ -116,23 +112,22 @@ void MenuView::set_parent_rect(const Rect new_parent_rect) { arrow_more.set_parent_rect({228, (Coord)(displayed_max * item_height), 8, 8}); // TODO: Clean this up :( - if (menu_item_views.size()) { - for (auto item : menu_item_views) { - remove_child(item); - delete item; - } + if (!menu_item_views.empty()) { + for (auto& item : menu_item_views) + remove_child(item.get()); menu_item_views.clear(); } for (size_t c = 0; c < displayed_max; c++) { - auto item = new MenuItemView{keep_highlight}; - menu_item_views.push_back(item); - add_child(item); + auto item = std::make_unique(keep_highlight); + add_child(item.get()); Coord y_pos = c * item_height; item->set_parent_rect({{0, y_pos}, {size().width(), (Coord)item_height}}); + + menu_item_views.push_back(std::move(item)); } update_items(); @@ -150,9 +145,8 @@ void MenuView::on_tick_second() { } void MenuView::clear() { - for (auto item : menu_item_views) { + for (auto& item : menu_item_views) item->set_item(nullptr); - } menu_items.clear(); highlighted_item = 0; @@ -184,7 +178,7 @@ void MenuView::update_items() { } else more = false; - for (auto item : menu_item_views) { + for (auto& item : menu_item_views) { if (i >= menu_items.size()) break; // Assign item data to MenuItemViews according to offset @@ -201,7 +195,7 @@ void MenuView::update_items() { } MenuItemView* MenuView::item_view(size_t index) const { - return menu_item_views[index]; + return menu_item_views[index].get(); } bool MenuView::set_highlighted(int32_t new_value) { diff --git a/firmware/application/ui/ui_menu.hpp b/firmware/application/ui/ui_menu.hpp index 98591ab7..65364d66 100644 --- a/firmware/application/ui/ui_menu.hpp +++ b/firmware/application/ui/ui_menu.hpp @@ -30,8 +30,10 @@ #include "signal.hpp" #include -#include #include +#include +#include +#include namespace ui { @@ -75,7 +77,8 @@ class MenuView : public View { std::function on_left{}; std::function on_highlight{nullptr}; - MenuView(Rect new_parent_rect = {0, 0, 240, 304}, bool keep_highlight = false); + MenuView(Rect new_parent_rect = {0, 0, screen_width, screen_height - 16}, + bool keep_highlight = false); ~MenuView(); @@ -103,10 +106,10 @@ class MenuView : public View { SignalToken signal_token_tick_second{}; std::vector menu_items{}; - std::vector menu_item_views{}; + std::vector> menu_item_views{}; Image arrow_more{ - {228, 320 - 8, 8, 8}, + {228, screen_height - 8, 8, 8}, &bitmap_more, Color::white(), Color::black()}; diff --git a/firmware/baseband/proc_spectrum_painter.cpp b/firmware/baseband/proc_spectrum_painter.cpp index 1de2247a..aa74f27c 100644 --- a/firmware/baseband/proc_spectrum_painter.cpp +++ b/firmware/baseband/proc_spectrum_painter.cpp @@ -25,10 +25,12 @@ #include "random.hpp" #include +#include +#include // TODO move to class members SpectrumPainterProcessor -complex16_t* current_line_data = nullptr; -complex16_t* volatile next_line_data = nullptr; +std::unique_ptr current_line_data; +std::unique_ptr next_line_data; uint32_t current_line_index = 0; uint32_t current_line_width = 0; int32_t current_bw = 0; @@ -41,20 +43,17 @@ int max_val = 127; // This is called at 3072000/2048 = 1500Hz void SpectrumPainterProcessor::execute(const buffer_c8_t& buffer) { for (uint32_t i = 0; i < buffer.count; i++) { - if (current_line_data != nullptr) { + if (current_line_data) { auto data = current_line_data[(current_line_index++ * current_bw / 3072) % current_line_width]; buffer.p[i] = {(int8_t)data.real(), (int8_t)data.imag()}; } else buffer.p[i] = {0, 0}; } - // collect new data - if (next_line_data != nullptr) { - if (current_line_data != nullptr) - delete current_line_data; - - current_line_data = next_line_data; - next_line_data = nullptr; + // Move "next line" into "current line" if set. + if (next_line_data) { + current_line_data = std::move(next_line_data); + next_line_data.reset(); } } @@ -65,7 +64,7 @@ void SpectrumPainterProcessor::run() { init_genrand(22267); while (true) { - if (fifo.is_empty() == false && next_line_data == nullptr) { + if (fifo.is_empty() == false && !next_line_data) { std::vector data; fifo.out(data); @@ -74,8 +73,9 @@ void SpectrumPainterProcessor::run() { auto fft_width = picture_width * 2; auto qu = fft_width / 4; - complex16_t* v = new complex16_t[fft_width]; - complex16_t* tmp = new complex16_t[fft_width]; + // TODO: can these be statically allocated? + auto v = std::make_unique(fft_width); + auto tmp = std::make_unique(fft_width); for (uint32_t fft_index = 0; fft_index < fft_width; fft_index++) { if (fft_index < qu) { @@ -103,10 +103,10 @@ void SpectrumPainterProcessor::run() { } } - ifft(v, fft_width, tmp); + ifft(v.get(), fft_width, tmp.get()); // normalize - volatile int32_t maximum = 1; + int32_t maximum = 1; for (uint32_t i = 0; i < fft_width; i++) { if (v[i].real() > maximum) maximum = v[i].real(); @@ -127,8 +127,7 @@ void SpectrumPainterProcessor::run() { } } - delete tmp; - next_line_data = v; + next_line_data = std::move(v); ui++; } else { chThdSleepMilliseconds(1); diff --git a/firmware/baseband/rssi_dma.cpp b/firmware/baseband/rssi_dma.cpp index fbd60370..74cd4783 100644 --- a/firmware/baseband/rssi_dma.cpp +++ b/firmware/baseband/rssi_dma.cpp @@ -21,9 +21,10 @@ #include "rssi_dma.hpp" +#include #include #include -#include +#include #include "hal.h" #include "gpdma.hpp" @@ -98,8 +99,8 @@ struct buffers_config_t { static buffers_config_t buffers_config; -static sample_t* samples{nullptr}; -static gpdma::channel::LLI* lli{nullptr}; +static std::unique_ptr samples; +static std::unique_ptr lli; static ThreadWait thread_wait; @@ -128,8 +129,8 @@ void allocate(size_t buffer_count, size_t items_per_buffer) { const auto peripheral = reinterpret_cast(&LPC_ADC1->DR[portapack::adc1_rssi_input]) + 1; const auto control_value = control(gpdma::buffer_words(buffers_config.items_per_buffer, 1)); - samples = new sample_t[buffers_config.count * buffers_config.items_per_buffer]; - lli = new gpdma::channel::LLI[buffers_config.count]; + samples = std::make_unique(buffers_config.count * buffers_config.items_per_buffer); + lli = std::make_unique(buffers_config.count); for (size_t i = 0; i < buffers_config.count; i++) { const auto memory = reinterpret_cast(&samples[i * buffers_config.items_per_buffer]); @@ -141,8 +142,8 @@ void allocate(size_t buffer_count, size_t items_per_buffer) { } void free() { - delete samples; - delete lli; + samples.reset(); + lli.reset(); } void enable() { diff --git a/firmware/baseband/touch_dma.cpp b/firmware/baseband/touch_dma.cpp index 23de9c79..1205de3e 100644 --- a/firmware/baseband/touch_dma.cpp +++ b/firmware/baseband/touch_dma.cpp @@ -97,8 +97,6 @@ void init() { } void allocate() { - // samples = new sample_t[channel_samples_per_frame]; - // lli = new gpdma::channel::LLI; lli.srcaddr = reinterpret_cast(&LPC_ADC0->DR[0]); lli.destaddr = reinterpret_cast(&shared_memory.touch_adc_frame.dr[0]); lli.lli = lli_pointer(&lli); @@ -106,8 +104,6 @@ void allocate() { } void free() { - // delete samples; - // delete lli; } void enable() {