From 900f2d59e9f15acd33ad619939bf95a9b3157d4a Mon Sep 17 00:00:00 2001 From: Jakub <106778416+MarekZegare4@users.noreply.github.com> Date: Sun, 24 May 2026 22:20:42 +0200 Subject: [PATCH] refactor(ui): tag SettingsScreen ifdefs with semantic FEAT_* macros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extends Features.h with preprocessor flags (FEAT_BRIGHTNESS_SETTING, FEAT_CLOCK_SECONDS_SETTING, FEAT_DISPLAY_ROTATION_SETTING, FEAT_JOYSTICK_ROTATION_SETTING, FEAT_FULL_REFRESH_SETTING) since constexpr can't gate enum members or class fields. SettingsScreen.h now uses #if FEAT_X instead of #if defined(EINK_DISPLAY_MODEL) — grep on the feature name finds the toggle and every use site, and the condition names *what* the gate controls. Driver headers (DisplayDriver.h, GxEPDDisplay.h) keep raw EINK_DISPLAY_MODEL — they're below the companion_radio layer and the ifdef there gates on the build's panel type, not a UI feature. Co-Authored-By: Claude Sonnet 4.6 --- examples/companion_radio/Features.h | 34 ++++++++++++++---- .../companion_radio/ui-new/SettingsScreen.h | 36 ++++++++++--------- 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/examples/companion_radio/Features.h b/examples/companion_radio/Features.h index 5017256c..b71121fd 100644 --- a/examples/companion_radio/Features.h +++ b/examples/companion_radio/Features.h @@ -1,11 +1,33 @@ #pragma once -// Build-time feature flags derived from board defines. -// Use these instead of `#ifdef EINK_DISPLAY_MODEL` for runtime-shape decisions -// (booleans, timings, defaults). Compiler dead-branch-eliminates on the -// constexpr, so cost is identical to a preprocessor branch but the code -// remains readable and reachable to tooling. Conditional struct members -// (enum entries, NodePrefs fields) still need a real `#if defined(...)`. +// Build-time feature flags derived from board defines. Two flavours: +// +// FEAT_* preprocessor macros — for conditional compilation of struct +// members, enum entries, and class fields where the preprocessor must +// run. Use `#if FEAT_X` instead of `#ifdef EINK_DISPLAY_MODEL`; the +// name documents *what* the flag toggles rather than *why* it's bound +// to e-ink. +// +// Features::* constexpr values — for runtime-shape decisions (booleans, +// timings, defaults). Compiler dead-branch-eliminates on the constexpr, +// so cost is identical to a preprocessor branch but the code stays +// reachable to tooling. + +#if defined(EINK_DISPLAY_MODEL) + // E-ink build: slow refresh, rotatable panel, no per-second redraws + #define FEAT_BRIGHTNESS_SETTING 0 + #define FEAT_CLOCK_SECONDS_SETTING 0 + #define FEAT_DISPLAY_ROTATION_SETTING 1 + #define FEAT_JOYSTICK_ROTATION_SETTING 1 + #define FEAT_FULL_REFRESH_SETTING 1 +#else + // OLED build: fast refresh, fixed orientation + #define FEAT_BRIGHTNESS_SETTING 1 + #define FEAT_CLOCK_SECONDS_SETTING 1 + #define FEAT_DISPLAY_ROTATION_SETTING 0 + #define FEAT_JOYSTICK_ROTATION_SETTING 0 + #define FEAT_FULL_REFRESH_SETTING 0 +#endif namespace Features { diff --git a/examples/companion_radio/ui-new/SettingsScreen.h b/examples/companion_radio/ui-new/SettingsScreen.h index d7d8ef0e..6705c77b 100644 --- a/examples/companion_radio/ui-new/SettingsScreen.h +++ b/examples/companion_radio/ui-new/SettingsScreen.h @@ -2,13 +2,15 @@ // Custom screen — not part of upstream UITask.cpp // Included by UITask.cpp after SensorPlaceholders.h is defined. +#include "../Features.h" + class SettingsScreen : public UIScreen { UITask* _task; enum SettingItem { // Display section SECTION_DISPLAY, -#if !defined(EINK_DISPLAY_MODEL) +#if FEAT_BRIGHTNESS_SETTING BRIGHTNESS, #endif #if AUTO_OFF_MILLIS > 0 @@ -16,18 +18,18 @@ class SettingsScreen : public UIScreen { #endif AUTO_LOCK, BATT_DISPLAY, -#if !defined(EINK_DISPLAY_MODEL) +#if FEAT_CLOCK_SECONDS_SETTING CLOCK_SECONDS, #endif CLOCK_FORMAT, FONT, -#if defined(EINK_DISPLAY_MODEL) +#if FEAT_DISPLAY_ROTATION_SETTING ROTATION, #endif -#if defined(EINK_DISPLAY_MODEL) +#if FEAT_JOYSTICK_ROTATION_SETTING JOY_ROTATION, #endif -#if defined(EINK_DISPLAY_MODEL) +#if FEAT_FULL_REFRESH_SETTING EINK_FULL_REFRESH, #endif // Sound section @@ -88,7 +90,7 @@ class SettingsScreen : public UIScreen { static const int LOW_BAT_COUNT = 7; static const char* BATT_DISPLAY_LABELS[3]; static const int BATT_DISPLAY_COUNT = 3; -#if defined(EINK_DISPLAY_MODEL) +#if FEAT_FULL_REFRESH_SETTING static const char* EINK_FULL_REFRESH_LABELS[5]; static const int EINK_FULL_REFRESH_COUNT = 5; #endif @@ -322,7 +324,7 @@ class SettingsScreen : public UIScreen { display.print(sel ? ">" : " "); display.setCursor(display.getCharWidth() + 2, y); -#if !defined(EINK_DISPLAY_MODEL) +#if FEAT_BRIGHTNESS_SETTING if (item == BRIGHTNESS) { display.print("Bright"); renderBar(display, display.valCol(), y, (p ? p->display_brightness : 2) + 1, 5); @@ -410,7 +412,7 @@ class SettingsScreen : public UIScreen { display.setCursor(display.valCol(), y); uint8_t mode = p ? p->batt_display_mode : 0; display.print(BATT_DISPLAY_LABELS[mode < BATT_DISPLAY_COUNT ? mode : 0]); -#if !defined(EINK_DISPLAY_MODEL) +#if FEAT_CLOCK_SECONDS_SETTING } else if (item == CLOCK_SECONDS) { display.print("Seconds"); display.setCursor(display.valCol(), y); @@ -424,7 +426,7 @@ class SettingsScreen : public UIScreen { display.print("Font"); display.setCursor(display.valCol(), y); display.print((p && p->use_lemon_font) ? "Lemon" : "Default"); -#if defined(EINK_DISPLAY_MODEL) +#if FEAT_DISPLAY_ROTATION_SETTING } else if (item == ROTATION) { display.print("Rotation"); display.setCursor(display.valCol(), y); @@ -432,7 +434,7 @@ class SettingsScreen : public UIScreen { uint8_t r = p ? (p->display_rotation & 3) : 0; display.print(ROT_LABELS[r]); } #endif -#if defined(EINK_DISPLAY_MODEL) +#if FEAT_JOYSTICK_ROTATION_SETTING } else if (item == JOY_ROTATION) { display.print("Joystick"); display.setCursor(display.valCol(), y); @@ -440,7 +442,7 @@ class SettingsScreen : public UIScreen { uint8_t r = p ? (p->joystick_rotation & 3) : 0; display.print(ROT_LABELS[r]); } #endif -#if defined(EINK_DISPLAY_MODEL) +#if FEAT_FULL_REFRESH_SETTING } else if (item == EINK_FULL_REFRESH) { display.print("Full rfsh"); display.setCursor(display.valCol(), y); @@ -557,7 +559,7 @@ public: bool left = (c == KEY_LEFT || c == KEY_PREV); bool enter = (c == KEY_ENTER); -#if !defined(EINK_DISPLAY_MODEL) +#if FEAT_BRIGHTNESS_SETTING if (_selected == BRIGHTNESS) { uint8_t lvl = _task->getBrightnessLevel(); if (right && lvl < 4) { _task->setBrightnessLevel(lvl + 1); _dirty = true; return true; } @@ -646,7 +648,7 @@ public: if (left) idx = (idx + BATT_DISPLAY_COUNT - 1) % BATT_DISPLAY_COUNT; if (left || right) { p->batt_display_mode = idx; _dirty = true; return true; } } -#if !defined(EINK_DISPLAY_MODEL) +#if FEAT_CLOCK_SECONDS_SETTING if (_selected == CLOCK_SECONDS && p && (left || right || enter)) { p->clock_hide_seconds ^= 1; _dirty = true; @@ -664,7 +666,7 @@ public: _dirty = true; return true; } -#if defined(EINK_DISPLAY_MODEL) +#if FEAT_DISPLAY_ROTATION_SETTING if (_selected == ROTATION && p && (left || right || enter)) { p->display_rotation = (p->display_rotation + (left ? 3 : 1)) & 3; _task->applyRotation(); @@ -672,14 +674,14 @@ public: return true; } #endif -#if defined(EINK_DISPLAY_MODEL) +#if FEAT_JOYSTICK_ROTATION_SETTING if (_selected == JOY_ROTATION && p && (left || right || enter)) { p->joystick_rotation = (p->joystick_rotation + (left ? 3 : 1)) & 3; _dirty = true; return true; } #endif -#if defined(EINK_DISPLAY_MODEL) +#if FEAT_FULL_REFRESH_SETTING if (_selected == EINK_FULL_REFRESH && p && (left || right || enter)) { int idx = p->eink_full_refresh_every; if (idx >= EINK_FULL_REFRESH_COUNT) idx = 0; @@ -722,6 +724,6 @@ const char* SettingsScreen::GPS_INTERVAL_LABELS[6] = { "off", "30s", "1min", const uint16_t SettingsScreen::LOW_BAT_OPTS[7] = { 0, 3000, 3100, 3200, 3300, 3400, 3500 }; const char* SettingsScreen::LOW_BAT_LABELS[7] = { "off", "3.0V", "3.1V", "3.2V", "3.3V", "3.4V", "3.5V" }; const char* SettingsScreen::BATT_DISPLAY_LABELS[3] = { "icon", "%", "V" }; -#if defined(EINK_DISPLAY_MODEL) +#if FEAT_FULL_REFRESH_SETTING const char* SettingsScreen::EINK_FULL_REFRESH_LABELS[5] = { "off", "5", "10", "20", "30" }; #endif