Ext Module more than 5 items (#3314)

* Refactor app display to use scrollable menu

Removed specific app name text fields and replaced them with a scrollable menu for an unlimited app list.

* Refactor app display logic in ExternalModuleView

Refactor ExternalModuleView to manage app display more efficiently by using a menu instead of individual text fields.

* Update menu item color to use theme foreground color

* Ext Module: self-heal app list on transient read failures

Only skip rebuilding the scrollable app list when the reported app
count is unchanged AND the current menu already holds that many items.
If some getStandaloneAppInfo() calls failed transiently (e.g. an I2C
read glitch), the menu is incomplete; rebuilding on a later tick lets
it self-heal instead of staying permanently truncated.

Addresses Copilot review feedback on #3314.
This commit is contained in:
Mo
2026-09-14 20:53:51 +02:00
committed by GitHub
parent 31b25a445b
commit d72b5ed0b1
2 changed files with 42 additions and 52 deletions
@@ -22,6 +22,7 @@
#include "ui_external_module_view.hpp" #include "ui_external_module_view.hpp"
#include "portapack.hpp" #include "portapack.hpp"
#include "ui_standalone_view.hpp" #include "ui_standalone_view.hpp"
#include "theme.hpp"
#include "i2cdevmanager.hpp" #include "i2cdevmanager.hpp"
#include "i2cdev_ppmod.hpp" #include "i2cdev_ppmod.hpp"
@@ -31,7 +32,7 @@
namespace ui { namespace ui {
void ExternalModuleView::focus() { void ExternalModuleView::focus() {
dummy.focus(); menu_apps.focus();
} }
void ExternalModuleView::on_tick_second() { void ExternalModuleView::on_tick_second() {
@@ -44,11 +45,10 @@ void ExternalModuleView::on_tick_second() {
text_name.set(""); text_name.set("");
text_version.set(""); text_version.set("");
text_number_apps.set(""); text_number_apps.set("");
text_app1_name.set(""); if (shown_count_ != -1) {
text_app2_name.set(""); menu_apps.clear();
text_app3_name.set(""); shown_count_ = -1;
text_app4_name.set(""); }
text_app5_name.set("");
return; return;
} }
@@ -59,11 +59,10 @@ void ExternalModuleView::on_tick_second() {
text_name.set(""); text_name.set("");
text_version.set(""); text_version.set("");
text_number_apps.set(""); text_number_apps.set("");
text_app1_name.set(""); if (shown_count_ != -1) {
text_app2_name.set(""); menu_apps.clear();
text_app3_name.set(""); shown_count_ = -1;
text_app4_name.set(""); }
text_app5_name.set("");
return; return;
} }
@@ -74,59 +73,56 @@ void ExternalModuleView::on_tick_second() {
text_version.set("Version: " + std::to_string(device_info->module_version)); text_version.set("Version: " + std::to_string(device_info->module_version));
text_number_apps.set("No# Apps: " + std::to_string(device_info->application_count)); text_number_apps.set("No# Apps: " + std::to_string(device_info->application_count));
for (uint32_t i = 0; i < device_info->application_count && i < 5; i++) { // Only skip the rebuild when the app count is unchanged AND the current
// list is complete. If some getStandaloneAppInfo() calls failed transiently
// (e.g. an I2C read glitch) the menu holds fewer items than reported, so we
// rebuild to let the list self-heal on a later tick.
if ((int32_t)device_info->application_count == shown_count_ &&
menu_apps.item_count() == (size_t)device_info->application_count) {
return;
}
menu_apps.clear();
for (uint32_t i = 0; i < device_info->application_count; i++) {
auto appInfo = dev->getStandaloneAppInfo(i); auto appInfo = dev->getStandaloneAppInfo(i);
if (appInfo.has_value() == false) { if (appInfo.has_value() == false) {
continue; continue;
} }
std::string btnText = (std::string) "App " + std::to_string(i + 1) + ": " + (const char*)appInfo->app_name; std::string itemText = (std::string) "App " + std::to_string(i + 1) + ": " + (const char*)appInfo->app_name;
switch (appInfo->menu_location) { switch (appInfo->menu_location) {
case app_location_t::UTILITIES: case app_location_t::UTILITIES:
btnText += " (Utilities)"; itemText += " (Utilities)";
break; break;
case app_location_t::RX: case app_location_t::RX:
btnText += " (RX)"; itemText += " (RX)";
break; break;
case app_location_t::TX: case app_location_t::TX:
btnText += " (TX)"; itemText += " (TX)";
break; break;
case app_location_t::TRX: case app_location_t::TRX:
btnText += " (TRX)"; itemText += " (TRX)";
break; break;
case app_location_t::SETTINGS: case app_location_t::SETTINGS:
btnText += " (Settings)"; itemText += " (Settings)";
break; break;
case app_location_t::DEBUG: case app_location_t::DEBUG:
btnText += " (Debug)"; itemText += " (Debug)";
break; break;
case app_location_t::HOME: case app_location_t::HOME:
btnText += " (Home)"; itemText += " (Home)";
break; break;
case app_location_t::GAMES: case app_location_t::GAMES:
btnText += " (Games)"; itemText += " (Games)";
break; break;
} }
switch (i) { menu_apps.add_item({itemText, ui::Theme::getInstance()->fg_light->foreground, nullptr, [](KeyEvent) {}});
case 0:
text_app1_name.set(btnText);
break;
case 1:
text_app2_name.set(btnText);
break;
case 2:
text_app3_name.set(btnText);
break;
case 3:
text_app4_name.set(btnText);
break;
case 4:
text_app5_name.set(btnText);
break;
}
} }
shown_count_ = (int32_t)device_info->application_count;
} }
} // namespace ui } // namespace ui
@@ -49,12 +49,7 @@ class ExternalModuleView : public View {
&text_name, &text_name,
&text_version, &text_version,
&text_number_apps, &text_number_apps,
&text_app1_name, &menu_apps});
&text_app2_name,
&text_app3_name,
&text_app4_name,
&text_app5_name,
&dummy});
text_header.set("No module connected"); text_header.set("No module connected");
@@ -77,15 +72,14 @@ class ExternalModuleView : public View {
Text text_version{{24, 48, 200, 16}}; Text text_version{{24, 48, 200, 16}};
Text text_number_apps{{24, 64, 200, 16}}; Text text_number_apps{{24, 64, 200, 16}};
Text text_app1_name{{24, 96, 200, 16}}; // Scrollable, unlimited app list.
Text text_app2_name{{24, 112, 200, 16}}; MenuView menu_apps{
Text text_app3_name{{24, 128, 200, 16}}; {0, 84, screen_width, screen_height - 84},
Text text_app4_name{{24, 144, 200, 16}}; true};
Text text_app5_name{{24, 160, 200, 16}};
Button dummy{ // Rebuild the list only when the reported app count changes, so the user
{screen_width, 0, 0, 0}, // can scroll without it resetting every second.
""}; int32_t shown_count_{-1};
SignalToken signal_token_tick_second{}; SignalToken signal_token_tick_second{};