From 8f4cb4f4f3e7e206d77cb90dc013f946b6211eed Mon Sep 17 00:00:00 2001 From: Jan-Henrik Bruhn Date: Sun, 23 Feb 2020 17:14:07 +0100 Subject: [PATCH] Null safety for menu items, remove initialized flag from part_menu --- midi2cv/menu/menu_items.h | 38 +++++++++++++----- midi2cv/midi2cv.cc | 7 ++-- midi2cv/ui.cc | 19 ++++----- midi2cv/ui.h | 8 +++- midi2cv/ui/main_menu.cc | 72 --------------------------------- midi2cv/ui/main_menu.h | 83 ++++++++++++++++++++++++++++++++++----- midi2cv/ui/part_menu.h | 2 +- 7 files changed, 122 insertions(+), 107 deletions(-) delete mode 100644 midi2cv/ui/main_menu.cc diff --git a/midi2cv/menu/menu_items.h b/midi2cv/menu/menu_items.h index c701302..31a5806 100644 --- a/midi2cv/menu/menu_items.h +++ b/midi2cv/menu/menu_items.h @@ -21,6 +21,7 @@ class MenuItem : public AbstractMenuItem { char stringRepresentation[24]; protected: + MenuItem() {}; MenuItem(const char* _label, T* _value) : label(_label) , value(_value) {}; @@ -29,6 +30,8 @@ class MenuItem : public AbstractMenuItem { void set_value(T value) { + if (!this->value) + return; *this->value = value; this->to_string(stringRepresentation); }; @@ -46,7 +49,9 @@ class MenuItem : public AbstractMenuItem { char* get_string_representation() { - if (!stringRepresentation[0]) + if (!this->value) { + sprintf(stringRepresentation, "ERROR"); + } else if (!stringRepresentation[0]) this->to_string(stringRepresentation); return stringRepresentation; } @@ -60,6 +65,7 @@ class NumberMenuItem : public MenuItem { T step; protected: + NumberMenuItem() {}; NumberMenuItem(const char* _label, T* _value, T _minimumValue, T _maximumValue, T _step) : MenuItem(_label, _value) , minimumValue(_minimumValue) @@ -76,14 +82,16 @@ class NumberMenuItem : public MenuItem { public: void increase() { - if (*this->get_value_ptr() + step <= maximumValue && *this->get_value_ptr() + step >= minimumValue) - this->set_value(*this->get_value_ptr() + step); + if (this->get_value_ptr()) + if (*this->get_value_ptr() + step <= maximumValue && *this->get_value_ptr() + step >= minimumValue) + this->set_value(*this->get_value_ptr() + step); }; void decrease() { - if (*this->get_value_ptr() - step >= minimumValue && *this->get_value_ptr() - step <= maximumValue) - this->set_value(*this->get_value_ptr() - step); + if (this->get_value_ptr()) + if (*this->get_value_ptr() - step >= minimumValue && *this->get_value_ptr() - step <= maximumValue) + this->set_value(*this->get_value_ptr() - step); }; }; @@ -98,6 +106,7 @@ class UInt32MenuItem : public NumberMenuItem { public: UInt32MenuItem(const char* _label, uint32_t* _value, uint32_t _minimumValue, uint32_t _maximumValue, uint32_t _step) : NumberMenuItem(_label, _value, _minimumValue, _maximumValue, _step) {}; + UInt32MenuItem() {}; }; class Int32MenuItem : public NumberMenuItem { @@ -111,6 +120,7 @@ class Int32MenuItem : public NumberMenuItem { public: Int32MenuItem(const char* _label, int32_t* _value, int32_t _minimumValue, int32_t _maximumValue, int32_t _step) : NumberMenuItem(_label, _value, _minimumValue, _maximumValue, _step) {}; + Int32MenuItem(); }; class FloatMenuItem : public NumberMenuItem { @@ -124,6 +134,7 @@ class FloatMenuItem : public NumberMenuItem { public: FloatMenuItem(const char* _label, float* _value, float _minimumValue, float _maximumValue, float _step) : NumberMenuItem(_label, _value, _minimumValue, _maximumValue, _step) {}; + FloatMenuItem() {}; }; class BoolMenuItem : public NumberMenuItem { @@ -135,8 +146,9 @@ class BoolMenuItem : public NumberMenuItem { protected: const char* get_format_string() { - bool value = *this->get_value_ptr(); - + bool value = false; + if (this->get_value_ptr()) + value = *this->get_value_ptr(); if (value) return this->on_string; else @@ -148,6 +160,7 @@ class BoolMenuItem : public NumberMenuItem { : NumberMenuItem(_label, _value, 0, 1, 1) , on_string(_on_string) , off_string(_off_string) {}; + BoolMenuItem() {}; }; class StringListMenuItem : public NumberMenuItem { @@ -157,13 +170,17 @@ class StringListMenuItem : public NumberMenuItem { protected: const char* get_format_string() { - return this->string_labels[*this->get_value_ptr()]; + size_t index = 0; + if (this->get_value_ptr()) + index = *this->get_value_ptr(); + return this->string_labels[index]; } public: StringListMenuItem(const char* _label, uint8_t* _value, const char** _stringLabels, size_t _itemCount) : NumberMenuItem(_label, _value, 0, _itemCount - 1, 1) , string_labels(_stringLabels) {}; + StringListMenuItem() {}; }; class MidiNoteMenuItem : public NumberMenuItem { @@ -177,7 +194,9 @@ class MidiNoteMenuItem : public NumberMenuItem { public: char* get_string_representation() { - uint8_t currentNote = *this->get_value_ptr(); + uint8_t currentNote = 0; + if (this->get_value_ptr()) + currentNote = *this->get_value_ptr(); int note = currentNote % 12; int octave = (currentNote / 12) - 1; @@ -202,4 +221,5 @@ class MidiNoteMenuItem : public NumberMenuItem { note_strings[10] = "A#"; note_strings[11] = "B"; }; + MidiNoteMenuItem() {}; }; diff --git a/midi2cv/midi2cv.cc b/midi2cv/midi2cv.cc index d1d492a..b642b04 100644 --- a/midi2cv/midi2cv.cc +++ b/midi2cv/midi2cv.cc @@ -1,5 +1,6 @@ #include +#include "config.h" #include "drivers/display.h" #include "drivers/encoder.h" #include "drivers/gpio.h" @@ -8,7 +9,6 @@ #include "part.h" #include "stmlib/system/system_clock.h" #include "ui.h" -#include "config.h" using namespace stmlib; @@ -16,8 +16,9 @@ GPIO gpio; Display display; Encoder encoder; -UI ui; -Part part[PART_COUNT]; +Part parts[PART_COUNT]; +Part* part_pointers[PART_COUNT] = { &parts[0], &parts[1], &parts[2], &parts[3] }; +UI ui(part_pointers); // Default interrupt handlers. extern "C" { diff --git a/midi2cv/ui.cc b/midi2cv/ui.cc index 96091a3..6f25b4f 100644 --- a/midi2cv/ui.cc +++ b/midi2cv/ui.cc @@ -7,17 +7,12 @@ #include "part.h" #include "stmlib/utils/random.h" #include - +#include using namespace stmlib; const uint32_t kEncoderLongPressTime = 600; -// TODO: This is kind of ugly, can we improve this somehow? -Part parts[4]; -Part* part_pointers[4] = {&parts[0], &parts[1], &parts[2], &parts[3]}; -MainMenu mainMenu(part_pointers); - -UI::UI() +UI::UI(Part** part_pointers) : main_menu(part_pointers) { this->input_queue.Init(); } @@ -53,7 +48,7 @@ void UI::Draw() { display.u8g2()->clearBuffer(); - mainMenu.render(display.u8g2(), 0, 0, DISPLAY_WIDTH, DISPLAY_HEIGHT); + main_menu.render(display.u8g2(), 0, 0, DISPLAY_WIDTH, DISPLAY_HEIGHT); display.Swap(); } @@ -90,18 +85,18 @@ void UI::DoEvents() void UI::OnClick() { - mainMenu.enter(); + main_menu.enter(); } void UI::OnLongClick() { - mainMenu.back(); + main_menu.back(); } void UI::OnIncrement(Event& e) { if (e.data > 0) - mainMenu.down(); + main_menu.down(); else - mainMenu.up(); + main_menu.up(); } diff --git a/midi2cv/ui.h b/midi2cv/ui.h index 75c4e45..5bdffa9 100644 --- a/midi2cv/ui.h +++ b/midi2cv/ui.h @@ -4,9 +4,13 @@ #include "stmlib/stmlib.h" #include "stmlib/ui/event_queue.h" +#include "config.h" +#include "ui/main_menu.h" +#include + class UI { public: - UI(); + UI(Part** part_pointers); ~UI() {} void Poll(); @@ -21,6 +25,8 @@ class UI { bool encoder_long_press_event_sent_; uint32_t encoder_press_time_; + MainMenu main_menu; + void Draw(); void OnClick(); diff --git a/midi2cv/ui/main_menu.cc b/midi2cv/ui/main_menu.cc deleted file mode 100644 index 52eb9bf..0000000 --- a/midi2cv/ui/main_menu.cc +++ /dev/null @@ -1,72 +0,0 @@ -#include "main_menu.h" -#include "stmlib/stmlib.h" -#include - -const int kHeaderHeight = 13; -const char* kPartNames[] = { "A", "B", "C", "D" }; - -void MainMenu::back() -{ - if (this->activePartMenu >= 0) { - if (this->partMenus[this->activePartMenu].back()) - this->activePartMenu = -1; - } -} - -void MainMenu::enter() -{ - if (this->activePartMenu >= 0) { - if (this->partMenus[this->activePartMenu].enter()) - this->activePartMenu = -1; - } else { - this->activePartMenu = this->selectedPart; - } -} - -void MainMenu::up() -{ - if (this->activePartMenu >= 0) { - this->partMenus[this->activePartMenu].up(); - } else { - this->selectedPart--; - CONSTRAIN(this->selectedPart, 0, PART_COUNT - 1); - } -} - -void MainMenu::down() -{ - if (this->activePartMenu >= 0) { - this->partMenus[this->activePartMenu].down(); - } else { - this->selectedPart++; - CONSTRAIN(this->selectedPart, 0, PART_COUNT - 1); - } -} - -void MainMenu::render(U8G2* u8g2, int x, int y, int width, int height) -{ - u8g2->setFont(/*u8g2_font_5x8_tf*/ u8g2_font_pcsenior_8u); - for (int i = 0; i < PART_COUNT; i++) { - u8g2->setFontMode(1); - u8g2->setDrawColor(1); - - if (this->selectedPart == i) { - if (this->activePartMenu == i) { - u8g2->drawBox(x + 1 + i * (width / PART_COUNT), y + 1, (width / PART_COUNT) - 3, kHeaderHeight - 2); - } else { - u8g2->drawFrame(x + 1 + i * (width / PART_COUNT), y + 1, (width / PART_COUNT) - 3, kHeaderHeight - 2); - u8g2->drawFrame(x + i * (width / PART_COUNT), y, (width / PART_COUNT) - 1, kHeaderHeight); - } - } else { - u8g2->drawFrame(x + 1 + i * (width / PART_COUNT), y + 1, (width / PART_COUNT) - 3, kHeaderHeight - 2); - } - - u8g2->setDrawColor(2); - u8g2->drawStr(x + i * (width / PART_COUNT) + 5, y + 9, kPartNames[i]); - u8g2->setDrawColor(1); - } - - u8g2->drawHLine(0, kHeaderHeight + 1, width); - - this->partMenus[this->selectedPart].render(u8g2, x, y + kHeaderHeight + 3, width, height - kHeaderHeight - 3); -} diff --git a/midi2cv/ui/main_menu.h b/midi2cv/ui/main_menu.h index 9f62002..ac141c2 100644 --- a/midi2cv/ui/main_menu.h +++ b/midi2cv/ui/main_menu.h @@ -1,25 +1,90 @@ #pragma once -#include "../config.h" #include "part_menu.h" +#include "stmlib/stmlib.h" #include +static const int kHeaderHeight = 13; +static const char* kPartNames[] = { "A", "B", "C", "D" }; + +template class MainMenu { public: - void back(); - void enter(); - void up(); - void down(); + void back() + { + if (this->activePartMenu >= 0) { + if (this->partMenus[this->activePartMenu].back()) + this->activePartMenu = -1; + } + }; + void enter() + { + if (this->activePartMenu >= 0) { + if (this->partMenus[this->activePartMenu].enter()) + this->activePartMenu = -1; + } else { + this->activePartMenu = this->selectedPart; + } + }; + void up() + { + if (this->activePartMenu >= 0) { + this->partMenus[this->activePartMenu].up(); + } else { + this->selectedPart--; + CONSTRAIN(this->selectedPart, 0, (int)part_count - 1); + } + }; + void down() + { + if (this->activePartMenu >= 0) { + this->partMenus[this->activePartMenu].down(); + } else { + this->selectedPart++; + CONSTRAIN(this->selectedPart, 0, (int)part_count - 1); + } + }; - void render(U8G2* u8g2, int x, int y, int width, int height); + void render(U8G2* u8g2, int x, int y, int width, int height) + { + u8g2->setFont(/*u8g2_font_5x8_tf*/ u8g2_font_pcsenior_8u); + for (size_t i = 0; i < part_count; i++) { + u8g2->setFontMode(1); + u8g2->setDrawColor(1); + + if (this->selectedPart == (int)i) { + if (this->activePartMenu == (int)i) { + u8g2->drawBox(x + 1 + i * (width / part_count), y + 1, (width / part_count) - 3, kHeaderHeight - 2); + } else { + u8g2->drawFrame(x + 1 + i * (width / part_count), y + 1, (width / part_count) - 3, kHeaderHeight - 2); + u8g2->drawFrame(x + i * (width / part_count), y, (width / part_count) - 1, kHeaderHeight); + } + } else { + u8g2->drawFrame(x + 1 + i * (width / part_count), y + 1, (width / part_count) - 3, kHeaderHeight - 2); + } + + u8g2->setDrawColor(2); + u8g2->drawStr(x + i * (width / part_count) + 5, y + 9, kPartNames[i]); + u8g2->setDrawColor(1); + } + + u8g2->drawHLine(0, kHeaderHeight + 1, width); + + this->partMenus[this->selectedPart].render(u8g2, x, y + kHeaderHeight + 3, width, height - kHeaderHeight - 3); + } MainMenu(Part** parts) - : partMenus({ parts[0], parts[1], parts[2], parts[3] }) + : partMenus() , activePartMenu(0) - , selectedPart(0) {}; + , selectedPart(0) + { + for (size_t i = 0; i < part_count; i++) { // This is quite hacky, but it works.. we call the constructor again, this time with our parameters (Part*), as we cant do so directly from the default constructor for the array (thanks c++...) + new (&partMenus[i]) PartMenu(parts[i]); + } + }; private: - PartMenu partMenus[PART_COUNT]; + PartMenu partMenus[part_count]; int activePartMenu; int selectedPart; diff --git a/midi2cv/ui/part_menu.h b/midi2cv/ui/part_menu.h index 0ac6bd6..986f950 100644 --- a/midi2cv/ui/part_menu.h +++ b/midi2cv/ui/part_menu.h @@ -8,7 +8,7 @@ class PartMenu { public: PartMenu(Part* _part); - + PartMenu() {} bool enter(); bool back(); void up();