From 28f5610922e2adb4e349c600cb8c95c9f314109a Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 18 Sep 2026 22:44:25 +0000 Subject: [PATCH 1/9] Upgrade trunk (#11369) Co-authored-by: vidplace7 <1779290+vidplace7@users.noreply.github.com> --- .trunk/trunk.yaml | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/.trunk/trunk.yaml b/.trunk/trunk.yaml index bf2b87afd..23bfc479a 100644 --- a/.trunk/trunk.yaml +++ b/.trunk/trunk.yaml @@ -4,7 +4,7 @@ cli: plugins: sources: - id: trunk - ref: v1.10.2 + ref: v1.11.0 uri: https://github.com/trunk-io/plugins lint: # Custom file set + formatter that rewrites Unicode em-dash (U+2014, UTF-8 @@ -116,28 +116,28 @@ lint: - node-id-format@SYSTEM - unity-exit@SYSTEM - unset-sentinel-millis@SYSTEM - - checkov@3.3.8 - - renovate@44.2.3 - - prettier@3.9.6 - - trufflehog@3.96.0 + - checkov@3.3.19 + - renovate@44.103.0 + - prettier@3.9.8 + - trufflehog@3.97.5 - yamllint@1.38.0 - bandit@1.9.4 - - trivy@0.72.0 + - trivy@0.74.0 - taplo@0.10.0 - - ruff@0.16.0 - - isort@8.0.1 + - ruff@0.16.8 + - isort@9.0.1 - markdownlint@0.49.1 - - oxipng@10.1.1 - - svgo@4.0.2 + - oxipng@10.2.1 + - svgo@4.1.0 - actionlint@1.7.12 - flake8@7.3.0 - - hadolint@2.14.0 - - shfmt@3.6.0 + - hadolint@2.15.1 + - shfmt@3.14.1 - shellcheck@0.11.0 - black@26.5.1 - git-diff-check - gitleaks@8.30.1 - - clang-format@16.0.3 + - clang-format@20.1.0 ignore: - linters: [ALL] paths: From 24dc0e9d62b2f6c70d375d4009cb10f3179a973d Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 19 Sep 2026 06:21:30 -0500 Subject: [PATCH 2/9] Upgrade trunk (#11909) Co-authored-by: vidplace7 <1779290+vidplace7@users.noreply.github.com> --- .trunk/trunk.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.trunk/trunk.yaml b/.trunk/trunk.yaml index 23bfc479a..588e9bb7f 100644 --- a/.trunk/trunk.yaml +++ b/.trunk/trunk.yaml @@ -117,7 +117,7 @@ lint: - unity-exit@SYSTEM - unset-sentinel-millis@SYSTEM - checkov@3.3.19 - - renovate@44.103.0 + - renovate@44.103.2 - prettier@3.9.8 - trufflehog@3.97.5 - yamllint@1.38.0 From fa87e7d29cf5838ac90c807f6eb8b4024b126fa0 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Sat, 19 Sep 2026 08:44:00 +0000 Subject: [PATCH 3/9] chore(deps): update libch341-spi-userspace digest to d85aceb (#11907) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- variants/native/portduino.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/variants/native/portduino.ini b/variants/native/portduino.ini index 4564b621a..d0bfcc88c 100644 --- a/variants/native/portduino.ini +++ b/variants/native/portduino.ini @@ -28,7 +28,7 @@ lib_deps = # renovate: datasource=custom.pio depName=LovyanGFX packageName=lovyan03/library/LovyanGFX lovyan03/LovyanGFX@1.2.28 ; # renovate: datasource=git-refs depName=libch341-spi-userspace packageName=https://github.com/meshtastic/libch341-spi-userspace gitBranch=main - https://github.com/meshtastic/libch341-spi-userspace/archive/eaaef01997788e6cbd2432eee15ea57d5300a06a.zip + https://github.com/meshtastic/libch341-spi-userspace/archive/d85aceb760da291d90e0ff526ba17b2e1323d988.zip # renovate: datasource=custom.pio depName=adafruit/Adafruit seesaw Library packageName=adafruit/library/Adafruit seesaw Library adafruit/Adafruit seesaw Library@1.7.9 # renovate: datasource=git-refs depName=RAK12034-BMX160 packageName=https://github.com/RAKWireless/RAK12034-BMX160 gitBranch=main From ee02cc3426f3dc2fb46c620d8221c4e4eb877df1 Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Sun, 20 Sep 2026 04:11:18 +0000 Subject: [PATCH 4/9] Games joystick input (#11917) * fix(games): correct the high-score announcement argument order GAMES_HIGH_SCORE_STRING is "New %s high score %lu by %s!" but the arguments were passed as (name, initials, score): the initials string was formatted through %lu and the score integer through %s. That is a format/argument mismatch, so the announcement printed garbage at best and dereferenced the score as a pointer at worst. Co-Authored-By: Claude Opus 5 (1M context) * feat(input): report which physical gamepad button produced an event A joystick event only carried the action it was mapped to, so a consumer could not tell two buttons apart once they shared one action, and games were limited to the handful of actions the broker defines. Carry the originating evdev button code in InputEvent::kbchar, encoded into a reserved 0xC0..0xDF range that misses printable ASCII and every INPUT_BROKER_MSG_ value (SystemCommands switches on kbchar without looking at inputEvent, so a collision there would reboot the node rather than move a paddle). D-pad events are axes, not buttons, and keep leaving kbchar at 0 -- which is exactly what lets a consumer tell stick from button. Co-Authored-By: Claude Opus 5 (1M context) * feat(portduino): let one joystick action bind several buttons Input.JoystickButtons took a single evdev code per action, so a pad's A and Y could not both select, and the shoulder buttons could not sit alongside the D-pad. Accept a list of codes as well as a bare scalar; the config writer inverts its code->action map back out, emitting a list only where an action has more than one button. ConfigCheck gains a real checker for the section (it was previously waved through as free-form) covering the three ways a mapping silently does nothing: an action name the driver does not know, an evdev name where the numeric code belongs, and one code claimed by two actions. Two fixtures and shell-test cases cover the clean list form and those three faults. Co-Authored-By: Claude Opus 5 (1M context) * feat(games): use the gamepad's extra buttons, and return home when idle Games now receive the physical button alongside the action, so a pad with more than two usable buttons controls more than two things: - Snake: a shoulder button mapped to left/right turns relative to the snake's heading (L counter-clockwise, R clockwise) while the D-pad keeps steering absolutely. The two are told apart by kbchar, not by hardcoding one pad's codes. - Breakout: the ball now rides the paddle after each serve until the player fires it with B or A, so a life is not lost to a ball already in flight when the player looks up. The paddle also keeps its position between lives. A game can claim BACK for the duration (Game::wantsBackButton) so B serves instead of pausing, and releases it once the ball is live. - Start (BTN_BASE4 / BTN_START) is mapped to select like any other button, so it launches games and drives the menus; inside a running game GamesModule picks it out of kbchar and pauses instead. Separately, the games frame no longer holds a walked-away device hostage: after 15 s with no input it returns to the home frame, so the device still reads as a Meshtastic node. The timer is suspended while a picker or banner is up (e.g. high-score initials entry, which the input handler never sees) so it cannot yank the user out mid-entry. Screen::isInteractionBusy() generalises the old module-intercept check -- modal module, intercepting module, game, or an open interactive overlay -- and MessageRenderer uses it before popping an incoming-message banner. A transient banner REPLACES an active overlay, so an arriving message could otherwise discard a half-entered high score. The message is still stored, its thread still selected, and the unread indicator still set. Co-Authored-By: Claude Opus 5 (1M context) * feat(ui): compose freetext on the on-screen keyboard from a gamepad A gamepad can drive the on-screen keyboard but cannot type, so on a host with a joystick and no configured keyboard device the OSK is the only way to compose freetext. Set osk_found there, and gate the "Freetext" menu entries on whether the device can enter text at all (physical keyboard, OSK, or touchscreen virtual keyboard) rather than on kb_found alone -- those entries were hidden on exactly the devices that needed them. The OSK prompt that CannedMessageModule already had inline in the message selector becomes showOnScreenKeyboard(), so the menu path can reach it too. Menus call in from a banner callback and the banner is torn down as soon as that callback returns, which would take the keyboard down with it, so the menu path defers the launch to runOnce(). Co-Authored-By: Claude Opus 5 (1M context) * fix(games): address review on frame fallback and joystick input gating Breakout: the paddle suppression was far too broad. aLinuxJoystick is constructed on every Linux host whether or not a gamepad is configured (InputBroker.cpp), so `aLinuxJoystick && kbchar == 0` was true everywhere and swallowed LEFT/RIGHT from the keyboard, trackball and ExpressLRS -- on a host with no joystick attached at all. Gate on the stick actually driving the paddle instead: LinuxJoystick assigns heldX before it emits and only auto-repeats while heldX is set, so every axis LEFT/RIGHT arrives with a zone held and nothing else does. kbchar == 0 still distinguishes an axis from a shoulder button mapped to left/right, which must keep nudging the paddle. Screen: showHomeFrame() did nothing when the home frame was hidden, since setFrames() only assigns positions.home for !hiddenFrames.home. That stranded the games inactivity bounce on the frame it was trying to leave. Fall back to the messages frame, which setFrames() always adds. Test: rename test_ballWaitsOnPaddleUntilLaunched to test_ball_waitsOnPaddleUntilLaunched, matching the repo convention and its neighbours in the file. Co-Authored-By: Claude Opus 5 (1M context) * fix(games): give games the whole InputEvent so Breakout can identify the source Follow-up to review on #11917. The previous narrowing still could not tell sources apart: kbchar == 0 is shared by the joystick's D-pad axis and by every other driver that sends a bare LEFT/RIGHT, so while the D-pad was held a keyboard or touchscreen press was still discarded. heldXZone() proves the axis is driving, not that this particular event came from it. Pass the event itself to Game::handleInput() rather than (ev, kbchar). Games that only care about the action read event->inputEvent; Snake keeps using kbchar for shoulder steering; Breakout now also checks event->source against LinuxJoystick's origin name, so only that driver's own axis repeats are suppressed. Chose the event over a third positional parameter so the signature does not have to grow again the next time a game needs something the event already carries. All three conditions in Breakout are load-bearing: source says it came from this gamepad, kbchar == 0 says it is the axis rather than a shoulder button mapped to left/right, and heldXZone() != 0 says the axis is what is driving right now so tick() already has it covered. LinuxJoystick::originName() exposes the name the driver stamps into InputEvent::source, alongside the existing heldXZone()/heldYZone() accessors. Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- bin/config-dist.yaml | 14 +- bin/test-config-check.sh | 12 ++ src/graphics/Screen.cpp | 55 ++++++-- src/graphics/Screen.h | 17 ++- src/graphics/draw/MenuHandler.cpp | 27 ++-- src/graphics/draw/MessageRenderer.cpp | 9 +- src/input/InputBroker.h | 42 ++++++ src/input/LinuxJoystick.cpp | 9 +- src/input/LinuxJoystick.h | 6 +- src/main.cpp | 9 +- src/modules/CannedMessageModule.cpp | 129 +++++++++++------- src/modules/CannedMessageModule.h | 5 + src/modules/games/Breakout.cpp | 68 +++++++-- src/modules/games/Breakout.h | 13 +- src/modules/games/ChirpyRunner.cpp | 3 +- src/modules/games/ChirpyRunner.h | 2 +- src/modules/games/Game.h | 16 ++- src/modules/games/GamesModule.cpp | 75 +++++++++- src/modules/games/GamesModule.h | 7 + src/modules/games/Snake.cpp | 23 +++- src/modules/games/Snake.h | 14 +- src/modules/games/Tetris.cpp | 4 +- src/modules/games/Tetris.h | 2 +- src/platform/portduino/ConfigCheck.cpp | 60 +++++++- src/platform/portduino/PortduinoGlue.cpp | 32 +++-- src/platform/portduino/PortduinoGlue.h | 18 ++- test/fixtures/portduino-config/README.md | 10 ++ .../joystick-buttons-bad.yaml | 16 +++ .../portduino-config/joystick-buttons.yaml | 17 +++ test/test_breakout/test_main.cpp | 64 +++++++-- test/test_snake/test_main.cpp | 42 ++++++ 31 files changed, 689 insertions(+), 131 deletions(-) create mode 100644 test/fixtures/portduino-config/joystick-buttons-bad.yaml create mode 100644 test/fixtures/portduino-config/joystick-buttons.yaml diff --git a/bin/config-dist.yaml b/bin/config-dist.yaml index cf0d56da2..276808045 100644 --- a/bin/config-dist.yaml +++ b/bin/config-dist.yaml @@ -211,10 +211,18 @@ Input: # JoystickDevice: /dev/input/by-id/usb-0079_USB_Gamepad-event-joystick ### Map evdev button codes (hex or decimal) to actions. Omit to use built-in ### defaults (select=0x121, cancel=0x122). Actions: select, cancel, back, up, -### down, left, right, user. +### down, left, right, user. Give an action a list of codes to bind several +### physical buttons to it -- e.g. the shoulder buttons alongside the D-pad. +### Games can tell which physical button was pressed even when several share an +### action: in Snake the shoulders turn relative to the snake's heading while +### the D-pad steers absolutely, and Start pauses in play while selecting +### everywhere else. +### The codes below are an example only -- confirm yours with `evtest`. # JoystickButtons: -# select: 0x122 -# cancel: 0x121 +# select: [0x121, 0x123, 0x129] # A, Y and Start +# cancel: [0x122, 0x120] # B and X +# left: [0x124] # L shoulder, alongside the D-pad +# right: [0x125] # R shoulder ### Standard User Button Config # UserButton: 6 diff --git a/bin/test-config-check.sh b/bin/test-config-check.sh index b39f19277..7cee12d7a 100755 --- a/bin/test-config-check.sh +++ b/bin/test-config-check.sh @@ -376,6 +376,18 @@ assert "pin value that resolves to -1" 1 pin-unreadable.yaml check \ "Lora.CS is set, but its value could not be read as a pin number" \ "Result: 1 error, 0 warnings" +echo +echo "joystick buttons:" +# Keyed by action rather than by button so that one action can list several codes. +# The clean case is the regression guard on the list form staying accepted. +assert "several buttons bound to one action" 0 joystick-buttons.yaml check \ + "Result: 0 errors, 0 warnings" +assert "joystick mappings that do nothing" 0 joystick-buttons-bad.yaml check \ + "'fire' is not a recognised action" \ + "'BTN_SOUTH' is not an evdev button code" \ + "button 0x121 is mapped to both 'select' and 'cancel'" \ + "Result: 0 errors, 3 warnings" + echo echo "CH341 USB-SPI adapters:" # The Lora pins of a ch341 device are indexes on the adapter, driven by the usermode diff --git a/src/graphics/Screen.cpp b/src/graphics/Screen.cpp index 8d9c35ae4..e19400c30 100644 --- a/src/graphics/Screen.cpp +++ b/src/graphics/Screen.cpp @@ -2275,18 +2275,8 @@ int Screen::handleInputEvent(const InputEvent *event) // so long as a mesh module isn't using these events for some other purpose if (showingNormalScreen) { - // Ask any MeshModules if they're handling keyboard input right now - bool inputIntercepted = false; - for (MeshModule *module : moduleFrames) { - if (module && module->interceptingKeyboardInput()) - inputIntercepted = true; - } -#if BASEUI_HAS_GAMES - // The games frame isn't a moduleFrame, so check it explicitly: while a game is running it - // owns the D-pad (turns/pause) and we must not switch frames or open menus underneath it. - if (gamesModule && gamesModule->interceptingKeyboardInput()) - inputIntercepted = true; -#endif + // Ask any MeshModules (and the games frame) if they're handling keyboard input right now + const bool inputIntercepted = anyModuleInterceptingInput(); // If no modules are using the input, move between frames if (!inputIntercepted) { @@ -2449,6 +2439,47 @@ bool Screen::isGamesFrameShown() return framesetInfo.positions.games != 255 && ui && ui->getUiState()->currentFrame == framesetInfo.positions.games; } +void Screen::showHomeFrame() +{ + if (!ui) + return; + // Home is optional -- setFrames() only adds it when !hiddenFrames.home, leaving the position + // 255. Bouncing to nothing would strand the caller on the frame it wanted to leave, so fall + // back to the messages frame, which setFrames() always adds. + const uint8_t target = + (framesetInfo.positions.home != 255) ? framesetInfo.positions.home : framesetInfo.positions.textMessage; + if (target != 255) + ui->switchToFrame(target); +} + +bool Screen::anyModuleInterceptingInput() +{ + for (MeshModule *module : moduleFrames) { + if (module && module->interceptingKeyboardInput()) + return true; + } +#if BASEUI_HAS_GAMES + // The games frame isn't a moduleFrame, so check it explicitly: while a game is running it owns + // the D-pad (turns/pause) and we must not switch frames or open menus underneath it. + if (gamesModule && gamesModule->interceptingKeyboardInput()) + return true; +#endif + return false; +} + +bool Screen::isInteractionBusy() +{ + // Something is holding the D-pad -- the user is mid-interaction. A modal module owns the whole + // screen; an intercepting one owns the keys on its own frame. + if (hasModalModule() || anyModuleInterceptingInput()) + return true; + // An interactive overlay (picker / text entry) is open. Showing a transient banner REPLACES the + // active overlay, so this would silently discard whatever the user was entering. A plain + // text_banner is itself transient, so superseding one of those is fine. + const notificationTypeEnum nt = NotificationRenderer::current_notification_type; + return nt != notificationTypeEnum::none && nt != notificationTypeEnum::text_banner; +} + } // namespace graphics #else diff --git a/src/graphics/Screen.h b/src/graphics/Screen.h index 7a99c4820..59548e34e 100644 --- a/src/graphics/Screen.h +++ b/src/graphics/Screen.h @@ -260,7 +260,7 @@ class Screen : public concurrency::OSThread std::vector indicatorIcons; // Per-frame custom icon pointers #if defined(OLED_COMPACT_UI) - std::vector frameTitles; // Per-frame short labels, parallel to indicatorIcons + std::vector frameTitles; // Per-frame short labels, parallel to indicatorIcons #endif Screen(const Screen &) = delete; Screen &operator=(const Screen &) = delete; @@ -294,6 +294,17 @@ class Screen : public concurrency::OSThread // ignore D-pad input when the player has navigated to a different frame. bool isGamesFrameShown(); + // Jump straight to the home (device-focused) frame. Used to bounce back to a clearly "this is a + // Meshtastic node" screen after a game is left idle. Home is optional, so when it is hidden this + // falls back to the messages frame rather than staying put. + void showHomeFrame(); + + // True when the user is in the middle of something that must not be interrupted: a module (or + // game) is holding the D-pad, or an interactive overlay (picker / text entry) is open. Callers + // that would pop a transient banner should check this first -- a banner both covers the screen + // and REPLACES any interactive overlay, discarding a half-finished entry. + bool isInteractionBusy(); + bool isScreenOn() { return screenOn; } // Stores the last 4 of our hardware ID, to make finding the device for pairing easier @@ -850,6 +861,10 @@ class Screen : public concurrency::OSThread #endif /// UI helper for rendering to frames and switching between them + // True if any module frame -- or the games frame, which is not a moduleFrame -- is currently + // holding the D-pad. Shared by the input router and isInteractionBusy(). + bool anyModuleInterceptingInput(); + OLEDDisplayUi *ui; }; diff --git a/src/graphics/draw/MenuHandler.cpp b/src/graphics/draw/MenuHandler.cpp index 45a3e9760..da723eb6c 100644 --- a/src/graphics/draw/MenuHandler.cpp +++ b/src/graphics/draw/MenuHandler.cpp @@ -121,6 +121,18 @@ const StoredMessage *getNewestMessageForActiveThread() return nullptr; } +// Freetext compose is offered whenever the device can enter text at all: a physical +// keyboard, an on-screen keyboard driven by rotary/trackball/joystick, or a touchscreen +// virtual keyboard. +bool freetextAvailable() +{ +#if defined(USE_VIRTUAL_KEYBOARD) + return true; +#else + return kb_found || osk_found; +#endif +} + void launchReplyForMessage(const StoredMessage &message, bool freetext) { if (message.type == MessageType::BROADCAST || message.dest == NODENUM_BROADCAST) { @@ -156,12 +168,7 @@ uint8_t test_count = 0; void menuHandler::loraMenu() { static const char *optionsArray[] = { - "Back", - "Device Role", - "Radio Preset", - "Frequency Slot", - "LoRa Region", - "Transmit Enabled", + "Back", "Device Role", "Radio Preset", "Frequency Slot", "LoRa Region", "Transmit Enabled", #if HAS_LORA_FEM "FEM LNA", #endif @@ -918,8 +925,8 @@ void menuHandler::replyMenu() optionsArray[options] = "With Preset"; optionsEnumArray[options++] = ReplyPreset; - // Freetext reply (only when keyboard exists) - if (kb_found) { + // Freetext reply (only when the device can enter text) + if (freetextAvailable()) { optionsArray[options] = "With Freetext"; optionsEnumArray[options++] = ReplyFreetext; } @@ -1285,7 +1292,7 @@ void menuHandler::textMessageBaseMenu() int options = 1; optionsArray[options] = "New Preset Msg"; optionsEnumArray[options++] = Preset; - if (kb_found) { + if (freetextAvailable()) { optionsArray[options] = "New Freetext Msg"; optionsEnumArray[options++] = Freetext; } @@ -1408,7 +1415,7 @@ void menuHandler::favoriteBaseMenu() } optionsEnumArray[options++] = Preset; - if (kb_found) { + if (freetextAvailable()) { optionsArray[options] = "New Freetext Msg"; optionsEnumArray[options++] = Freetext; } diff --git a/src/graphics/draw/MessageRenderer.cpp b/src/graphics/draw/MessageRenderer.cpp index 284bfc6c4..0790bd234 100644 --- a/src/graphics/draw/MessageRenderer.cpp +++ b/src/graphics/draw/MessageRenderer.cpp @@ -1215,9 +1215,14 @@ void handleNewMessage(OLEDDisplay *display, const StoredMessage &sm, const mesht screen->setOn(true); } - if (!suppressBanner && !menuShowing && !screen->hasModalModule()) { + // Don't let the banner interrupt whatever the user is in the middle of -- it would cover an + // active module/game, and worse, a transient banner replaces any interactive overlay, so it + // would discard a half-entered picker / text entry (e.g. high-score initials). The message + // is still stored, its thread still selected below, and the unread indicator set, so nothing + // is lost -- the user just sees it once they're done. (isInteractionBusy() subsumes the + // modal-module check this guard used to make.) + if (!screen->isInteractionBusy() && !menuShowing && !suppressBanner) screen->showSimpleBanner(banner, inThread ? 1000 : 3000); - } } // Always focus into the correct conversation thread when a message with real text arrives diff --git a/src/input/InputBroker.h b/src/input/InputBroker.h index e30e84ff7..2d5bdfc1e 100644 --- a/src/input/InputBroker.h +++ b/src/input/InputBroker.h @@ -50,6 +50,48 @@ enum input_broker_event { #define INPUT_BROKER_MSG_TAB 0x09 #define INPUT_BROKER_MSG_EMOTE_LIST 0x8F +// Which physical joystick/gamepad button produced an event, carried in InputEvent::kbchar +// beside the action it is mapped to. Several buttons may share one action, so this is what +// lets a game tell SELECT-pressed-on-Y from SELECT-pressed-on-B and use more inputs than the +// handful of actions the broker defines. +// +// The value is the button's evdev code offset into a reserved kbchar range. 0x120..0x13f spans +// both the classic joystick codes (BTN_TRIGGER..BTN_DEAD) and the modern gamepad ones +// (BTN_SOUTH..BTN_THUMBR). The range deliberately misses printable ASCII (0x20-0x7e, which +// CannedMessages appends to a message) and every INPUT_BROKER_MSG_ value above: SystemCommands +// switches on kbchar without looking at inputEvent, so a collision there would toggle Bluetooth +// or reboot the node rather than move a paddle. +#define INPUT_BROKER_MSG_JOY_BUTTON_FIRST 0xC0 +#define INPUT_BROKER_MSG_JOY_BUTTON_LAST 0xDF +#define INPUT_BROKER_JOY_CODE_FIRST 0x120 +#define INPUT_BROKER_JOY_CODE_LAST 0x13F + +// evdev button code -> the kbchar that reports it, or 0 for a code outside the encodable range +// (0 is also "no button", which is what every non-joystick source leaves in kbchar). +constexpr unsigned char joyButtonToKbchar(int code) +{ + return (code >= INPUT_BROKER_JOY_CODE_FIRST && code <= INPUT_BROKER_JOY_CODE_LAST) + ? (unsigned char)(INPUT_BROKER_MSG_JOY_BUTTON_FIRST + (code - INPUT_BROKER_JOY_CODE_FIRST)) + : 0; +} + +// True if this kbchar names a gamepad button, i.e. the event came from a real button press +// rather than from a D-pad axis (which has no button and leaves kbchar 0) or a keyboard key. +// Lets a consumer treat "LEFT from a shoulder button" differently from "LEFT from the D-pad" +// without hardcoding one pad's button codes. +constexpr bool isJoyButton(unsigned char kbchar) +{ + return kbchar >= INPUT_BROKER_MSG_JOY_BUTTON_FIRST && kbchar <= INPUT_BROKER_MSG_JOY_BUTTON_LAST; +} + +// The Start button. Map it to "select" like any other button: it selects normally, and a consumer +// that wants Start specifically (GamesModule pauses on it) picks it out of kbchar. 0x129 +// (BTN_BASE4) is Start on the classic 10-button pads, 0x13b (BTN_START) on modern gamepads. +constexpr bool isJoyStartButton(unsigned char kbchar) +{ + return kbchar == joyButtonToKbchar(0x129) || kbchar == joyButtonToKbchar(0x13b); +} + typedef struct _InputEvent { const char *source; input_broker_event inputEvent; diff --git a/src/input/LinuxJoystick.cpp b/src/input/LinuxJoystick.cpp index d8f99ccec..826e49ad9 100644 --- a/src/input/LinuxJoystick.cpp +++ b/src/input/LinuxJoystick.cpp @@ -79,12 +79,15 @@ static int axisZone(int value) return 0; } -void LinuxJoystick::emitEvent(input_broker_event event) +// kbchar carries which physical button produced the event (0 for the D-pad, which is an axis +// rather than a button). Several buttons can be mapped to one action, so this is how a consumer +// tells them apart -- see joyButtonToKbchar() in InputBroker.h. +void LinuxJoystick::emitEvent(input_broker_event event, unsigned char kbchar) { InputEvent e = {}; e.inputEvent = event; e.source = this->_originName; - e.kbchar = 0; + e.kbchar = kbchar; // LOG_DEBUG("joystick: %s event %d", this->_originName, event); this->notifyObservers(&e); } @@ -158,7 +161,7 @@ int32_t LinuxJoystick::runOnce() // Buttons fire once per press (no auto-repeat). auto mapped = buttonMap.find(code); if (mapped != buttonMap.end()) - emitEvent(mapped->second); + emitEvent(mapped->second, joyButtonToKbchar(code)); } } } diff --git a/src/input/LinuxJoystick.h b/src/input/LinuxJoystick.h index 7e309b961..7413978d7 100644 --- a/src/input/LinuxJoystick.h +++ b/src/input/LinuxJoystick.h @@ -37,11 +37,15 @@ class LinuxJoystick : public Observable, public concurrency: int heldXZone() const { return heldX; } int heldYZone() const { return heldY; } + // The name this driver stamps into InputEvent::source. Lets a consumer tell an event that came + // from this gamepad from one that came from a keyboard or touchscreen carrying the same action. + const char *originName() const { return _originName; } + protected: virtual int32_t runOnce() override; private: - void emitEvent(input_broker_event event); + void emitEvent(input_broker_event event, unsigned char kbchar = 0); const char *_originName; bool firstTime = true; diff --git a/src/main.cpp b/src/main.cpp index 3f7baa2fd..1c444c33d 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1082,6 +1082,13 @@ void setup() #ifndef HAS_PHYSICAL_KEYBOARD osk_found = true; #endif +#endif +#if ARCH_PORTDUINO && defined(__linux__) + // Same idea for a gamepad: it can drive the on-screen keyboard but cannot type, so without a + // configured keyboard device it is the only way to compose freetext on this host. + if (portduino_config.joystickDevice != "" && portduino_config.keyboardDevice == "") { + osk_found = true; + } #endif // Now that the mesh service is created, create any modules @@ -1188,7 +1195,7 @@ void setup() #ifndef ARCH_PORTDUINO - // Initialize Wifi + // Initialize Wifi #if HAS_WIFI initWifi(); #endif diff --git a/src/modules/CannedMessageModule.cpp b/src/modules/CannedMessageModule.cpp index ea6f5ea96..84694fcc0 100644 --- a/src/modules/CannedMessageModule.cpp +++ b/src/modules/CannedMessageModule.cpp @@ -115,6 +115,61 @@ void CannedMessageModule::LaunchWithDestination(NodeNum newDest, uint8_t newChan LOG_TRACE("[CannedMessage] LaunchWithDestination dest=0x%08x ch=%d", dest, channel); } +// Compose through the on-screen keyboard used by devices without a physical one +// (rotary encoder, trackball, joystick). Returns false when no such keyboard exists, +// so callers can fall back to the plain freetext screen. +bool CannedMessageModule::showOnScreenKeyboard() +{ + if (!osk_found || !screen) + return false; + + char headerBuffer[64]; + if (this->dest == NODENUM_BROADCAST) { + snprintf(headerBuffer, sizeof(headerBuffer), "To: #%s", channels.getName(this->channel)); + } else { + snprintf(headerBuffer, sizeof(headerBuffer), "To: @%s", getNodeName(this->dest)); + } + screen->showTextInput(headerBuffer, "", 300000, [this](const std::string &text) { + if (!text.empty()) { + this->freetext = text.c_str(); + this->payload = CANNED_MESSAGE_RUN_STATE_FREETEXT; + updateState(CANNED_MESSAGE_RUN_STATE_SENDING_ACTIVE); + currentMessageIndex = -1; + + UIFrameEvent e; + e.action = UIFrameEvent::Action::REGENERATE_FRAMESET; + this->notifyObservers(&e); + screen->forceDisplay(); + + setIntervalFromNow(500); + return; + } else { + // Don't delete virtual keyboard immediately - it might still be executing + // Instead, just clear the callback and reset banner to stop input processing + graphics::NotificationRenderer::textInputCallback = nullptr; + graphics::NotificationRenderer::resetBanner(); + + // Return to inactive state + this->updateState(CANNED_MESSAGE_RUN_STATE_INACTIVE); + this->currentMessageIndex = -1; + this->freetext = ""; + this->cursor = 0; + + // Force display update to show normal screen + UIFrameEvent e; + e.action = UIFrameEvent::Action::REGENERATE_FRAMESET; + this->notifyObservers(&e); + screen->forceDisplay(); + + // Schedule cleanup for next loop iteration to ensure safe deletion + setIntervalFromNow(50); + return; + } + }); + + return true; +} + void CannedMessageModule::LaunchFreetextWithDestination(NodeNum newDest, uint8_t newChannel) { // Do NOT override explicit broadcast replies @@ -130,6 +185,19 @@ void CannedMessageModule::LaunchFreetextWithDestination(NodeNum newDest, uint8_t lastChannel = channel; lastDestSet = true; +#if !defined(USE_VIRTUAL_KEYBOARD) + // The freetext screen needs real key input, so devices without a physical keyboard + // compose on the on-screen keyboard instead. Open it from runOnce() rather than here: + // menus call us from a banner callback, and the banner is torn down as soon as that + // callback returns, which would take the keyboard down with it. + if (!kb_found && osk_found && screen) { + pendingOskLaunch = true; + setIntervalFromNow(0); + LOG_TRACE("[CannedMessage] LaunchFreetextWithDestination (OSK) dest=0x%08x ch=%d", dest, channel); + return; + } +#endif + updateState(CANNED_MESSAGE_RUN_STATE_FREETEXT, true); UIFrameEvent e; e.action = UIFrameEvent::Action::REGENERATE_FRAMESET; @@ -717,65 +785,18 @@ bool CannedMessageModule::handleMessageSelectorInput(const InputEvent *event, bo } // [Free Text] triggers the free text input (virtual keyboard) -#if defined(USE_VIRTUAL_KEYBOARD) if (strcmp(current, "[-- Free Text --]") == 0) { +#if defined(USE_VIRTUAL_KEYBOARD) updateState(CANNED_MESSAGE_RUN_STATE_FREETEXT, true); UIFrameEvent e; e.action = UIFrameEvent::Action::REGENERATE_FRAMESET; notifyObservers(&e); return true; - } #else - if (strcmp(current, "[-- Free Text --]") == 0) { - if (osk_found && screen) { - char headerBuffer[64]; - if (this->dest == NODENUM_BROADCAST) { - snprintf(headerBuffer, sizeof(headerBuffer), "To: #%s", channels.getName(this->channel)); - } else { - snprintf(headerBuffer, sizeof(headerBuffer), "To: @%s", getNodeName(this->dest)); - } - screen->showTextInput(headerBuffer, "", 300000, [this](const std::string &text) { - if (!text.empty()) { - this->freetext = text.c_str(); - this->payload = CANNED_MESSAGE_RUN_STATE_FREETEXT; - updateState(CANNED_MESSAGE_RUN_STATE_SENDING_ACTIVE); - currentMessageIndex = -1; - - UIFrameEvent e; - e.action = UIFrameEvent::Action::REGENERATE_FRAMESET; - this->notifyObservers(&e); - screen->forceDisplay(); - - setIntervalFromNow(500); - return; - } else { - // Don't delete virtual keyboard immediately - it might still be executing - // Instead, just clear the callback and reset banner to stop input processing - graphics::NotificationRenderer::textInputCallback = nullptr; - graphics::NotificationRenderer::resetBanner(); - - // Return to inactive state - this->updateState(CANNED_MESSAGE_RUN_STATE_INACTIVE); - this->currentMessageIndex = -1; - this->freetext = ""; - this->cursor = 0; - - // Force display update to show normal screen - UIFrameEvent e; - e.action = UIFrameEvent::Action::REGENERATE_FRAMESET; - this->notifyObservers(&e); - screen->forceDisplay(); - - // Schedule cleanup for next loop iteration to ensure safe deletion - setIntervalFromNow(50); - return; - } - }); - + if (showOnScreenKeyboard()) return true; - } - } #endif + } // Normal canned message selection if (runState == CANNED_MESSAGE_RUN_STATE_INACTIVE || runState == CANNED_MESSAGE_RUN_STATE_DISABLED) { @@ -1144,6 +1165,14 @@ void CannedMessageModule::sendText(NodeNum dest, ChannelIndex channel, const cha int32_t CannedMessageModule::runOnce() { + // A menu asked to compose freetext on the on-screen keyboard; the menu banner is gone + // by now, so it is safe to bring the keyboard up. + if (this->pendingOskLaunch) { + this->pendingOskLaunch = false; + if (showOnScreenKeyboard()) + return INT32_MAX; // the text input callback drives everything from here + } + if (this->runState == CANNED_MESSAGE_RUN_STATE_DESTINATION_SELECTION && needsUpdate) { updateDestinationSelectionList(); needsUpdate = false; diff --git a/src/modules/CannedMessageModule.h b/src/modules/CannedMessageModule.h index fe12bd468..e67d137e8 100644 --- a/src/modules/CannedMessageModule.h +++ b/src/modules/CannedMessageModule.h @@ -190,6 +190,11 @@ class CannedMessageModule : public SinglePortModule, public Observable // strcmp, for the joystick-source test in handleInput() // =========================================================================== // Pure BreakoutGame logic (no display/FS dependencies; always compiled) @@ -26,11 +27,18 @@ void BreakoutGame::serveBall() { // Centre the paddle and launch the ball upward from just above it, at a slight angle whose // side is chosen randomly so successive serves are not identical. - paddleLeft = (BOARD_W - PADDLE_W) / 2; + ballPxX = static_cast(BOARD_W / 2) * SUBPX; ballPxY = static_cast(PADDLE_Y - 2) * SUBPX; ballVx = (nextRandom() & 1u) ? 28 : -28; ballVy = -BALL_VY; + ballDocked = true; // wait for the player to fire before the ball moves +} + +void BreakoutGame::launchBall() +{ + if (alive) + ballDocked = false; } void BreakoutGame::nextLevel() @@ -48,6 +56,9 @@ void BreakoutGame::reset(uint32_t seed) levelNum = 1; alive = true; ballTick = false; + // A new game starts centred. serveBall() deliberately does NOT re-centre, so the paddle keeps + // the player's position between lives. + paddleLeft = (BOARD_W - PADDLE_W) / 2; buildBricks(); serveBall(); } @@ -78,6 +89,14 @@ bool BreakoutGame::step() if (!alive) return false; + // Waiting to serve: the ball rides the centre of the paddle (so it still aims by sliding) and + // no physics, collisions or life losses happen until the player fires it. + if (ballDocked) { + ballPxX = static_cast(paddleLeft + PADDLE_W / 2) * SUBPX; + ballPxY = static_cast(PADDLE_Y - 2) * SUBPX; + return true; + } + // The ball advances on every other step() so the caller can tick (and poll the paddle) at twice // the ball's rate -- this keeps the ball speed constant while paddle control refreshes faster. ballTick = !ballTick; @@ -224,20 +243,41 @@ bool Breakout::tick() return game.step(); } -void Breakout::handleInput(input_broker_event ev) +void Breakout::handleInput(const InputEvent *event) { -#if ARCH_PORTDUINO && defined(__linux__) - // When a joystick is present the paddle is polled continuously in tick(); ignore the discrete - // (and slow) repeat events so we don't double-move. - if (aLinuxJoystick) - return; -#endif + const input_broker_event ev = event->inputEvent; switch (ev) { case INPUT_BROKER_LEFT: - game.moveLeft(); - break; case INPUT_BROKER_RIGHT: - game.moveRight(); +#if ARCH_PORTDUINO && defined(__linux__) + // While the stick is held, tick() polls heldXZone() and moves the paddle itself, so the + // joystick's own discrete (and slow) repeat events would double-move. Suppress exactly + // those and nothing else. + // + // All three parts are load-bearing: + // source -- the event actually came from this gamepad. Without it, LEFT/RIGHT from the + // keyboard or touchscreen is swallowed too; aLinuxJoystick is constructed on + // every Linux host whether or not a gamepad is configured, so a pointer check + // alone is true even with nothing attached. + // kbchar -- no button produced it, so it is the D-pad axis. A shoulder button mapped to + // left/right is a discrete press the axis poll knows nothing about, and must + // still nudge the paddle. + // heldX -- the axis is what is driving right now, so tick() already has it covered. + if (aLinuxJoystick && event->kbchar == 0 && event->source && aLinuxJoystick->originName() && + strcmp(event->source, aLinuxJoystick->originName()) == 0 && aLinuxJoystick->heldXZone() != 0) + break; +#endif + if (ev == INPUT_BROKER_LEFT) + game.moveLeft(); + else + game.moveRight(); + break; + // Serve: B (CANCEL/BACK, forwarded here via wantsBackButton) or A (SELECT) fires a docked ball. + case INPUT_BROKER_CANCEL: + case INPUT_BROKER_BACK: + case INPUT_BROKER_SELECT: + case INPUT_BROKER_SELECT_LONG: + game.launchBall(); break; default: break; @@ -298,6 +338,12 @@ void Breakout::drawPlaying(OLEDDisplay *display, int16_t x, int16_t y) // Ball. display->fillRect(x + game.ballX(), y + game.ballY(), 2, 2); + // Waiting to serve: prompt the player (the ball rides the paddle until then). + if (game.isBallDocked()) { + display->setTextAlignment(TEXT_ALIGN_CENTER); + display->drawString(x + display->getWidth() / 2, y + BreakoutGame::PADDLE_Y - FONT_HEIGHT_SMALL - 4, "B TO SERVE"); + } + #if GRAPHICS_TFT_COLORING_ENABLED // Colour the wall by row, plus a blue paddle and white ball. One region per brick row (the row's // lit bricks take the colour; cleared cells and gaps stay background). Paddle then ball register diff --git a/src/modules/games/Breakout.h b/src/modules/games/Breakout.h index 9c4fe8dfb..3d4fe6dbd 100644 --- a/src/modules/games/Breakout.h +++ b/src/modules/games/Breakout.h @@ -52,6 +52,11 @@ class BreakoutGame void moveLeft(); void moveRight(); + /** After every serve the ball rides the paddle until the player fires it. launchBall() releases + * it; while docked the ball tracks the paddle and no collisions or life losses occur. */ + void launchBall(); + bool isBallDocked() const { return ballDocked; } + bool isPlaying() const { return alive; } uint32_t score() const { return points; } uint8_t lives() const { return livesLeft; } @@ -87,7 +92,8 @@ class BreakoutGame uint8_t levelNum = 1; uint32_t rng = 1; // xorshift32 state (never 0) bool alive = false; - bool ballTick = false; // ball advances on every other step() (see step()) + bool ballTick = false; // ball advances on every other step() (see step()) + bool ballDocked = false; // ball is waiting on the paddle to be launched }; #include "configuration.h" @@ -115,7 +121,10 @@ class Breakout : public Game uint32_t score() const override { return game.score(); } int32_t tickIntervalMs() const override; - void handleInput(input_broker_event ev) override; + void handleInput(const InputEvent *event) override; + // Claim BACK/CANCEL (the gamepad's B) while the ball waits on the paddle, so it serves instead + // of pausing. Released as soon as the ball is in play, so BACK pauses normally again. + bool wantsBackButton() const override { return game.isBallDocked(); } void drawAttract(OLEDDisplay *display, int16_t x, int16_t y) override; void drawPlaying(OLEDDisplay *display, int16_t x, int16_t y) override; diff --git a/src/modules/games/ChirpyRunner.cpp b/src/modules/games/ChirpyRunner.cpp index 4e1a18521..3caba0b7a 100644 --- a/src/modules/games/ChirpyRunner.cpp +++ b/src/modules/games/ChirpyRunner.cpp @@ -181,9 +181,10 @@ ChirpyRunner::ChirpyRunner() scores_.load(); } -void ChirpyRunner::handleInput(input_broker_event ev) +void ChirpyRunner::handleInput(const InputEvent *event) { // SELECT is the jump (as requested); UP is accepted as a convenient alternate. + const input_broker_event ev = event->inputEvent; if (ev == INPUT_BROKER_SELECT || ev == INPUT_BROKER_SELECT_LONG || ev == INPUT_BROKER_UP) game.jump(); } diff --git a/src/modules/games/ChirpyRunner.h b/src/modules/games/ChirpyRunner.h index b86e1d775..6d4f1fb98 100644 --- a/src/modules/games/ChirpyRunner.h +++ b/src/modules/games/ChirpyRunner.h @@ -154,7 +154,7 @@ class ChirpyRunner : public Game uint32_t score() const override { return game.score(); } int32_t tickIntervalMs() const override { return 33; } // ~30 fps; difficulty ramps via scroll speed - void handleInput(input_broker_event ev) override; + void handleInput(const InputEvent *event) override; void drawAttract(OLEDDisplay *display, int16_t x, int16_t y) override; void drawPlaying(OLEDDisplay *display, int16_t x, int16_t y) override; diff --git a/src/modules/games/Game.h b/src/modules/games/Game.h index 751b2d6b5..277252973 100644 --- a/src/modules/games/Game.h +++ b/src/modules/games/Game.h @@ -35,7 +35,21 @@ class Game virtual int32_t tickIntervalMs() const = 0; // per-game speed curve // --- Input while PLAYING (the host handles the BACK-to-pause and menu keys) --- - virtual void handleInput(input_broker_event ev) = 0; + // The whole event, not just the action, because an action alone cannot answer everything a + // game needs to know: + // - kbchar is a keystroke from a keyboard, or the physical gamepad button behind the action + // (INPUT_BROKER_MSG_JOY_BUTTON_FIRST..LAST, see InputBroker.h), or 0 when the source has + // nothing to add. Several buttons can share one action, so this is what lets a game act on + // the button rather than the action, and use more inputs than SELECT and CANCEL. + // - source names the driver that produced it. Two sources can send the same action with the + // same (zero) kbchar, so this is the only thing that tells them apart -- Breakout needs it + // to ignore the joystick's own D-pad repeats without also ignoring the keyboard. + // A game that only cares about the action just reads event->inputEvent. + virtual void handleInput(const InputEvent *event) = 0; + + // While true, the host forwards BACK/CANCEL to handleInput() instead of using it to pause. Lets + // a game briefly claim that button (e.g. Breakout's "press B to serve" before the ball launches). + virtual bool wantsBackButton() const { return false; } // --- Rendering (the host draws the shared PAUSED / GAME OVER / HIGH SCORES chrome) --- virtual void drawAttract(OLEDDisplay *display, int16_t x, int16_t y) = 0; // title/art + hi + hint diff --git a/src/modules/games/GamesModule.cpp b/src/modules/games/GamesModule.cpp index 606d1d505..2a5ce6dfa 100644 --- a/src/modules/games/GamesModule.cpp +++ b/src/modules/games/GamesModule.cpp @@ -56,10 +56,19 @@ void GamesModule::startPlaying() active->start(static_cast(random()) ^ millis()); uiState = GAMES_PLAYING; lastAwakeKickMs = millis(); + noteActivity(); kickTick(); requestRedraw(); } +void GamesModule::goHome() +{ + // Left idle too long: drop any game and return to the clearly-Meshtastic home frame. + exitToIdle(); + if (screen) + screen->showHomeFrame(); +} + void GamesModule::enterGameOver() { lastScore = active ? active->score() : 0; @@ -113,7 +122,7 @@ void GamesModule::announceHighScore(const char *initials, uint32_t score) // One shared message for every game, with the game's name spliced in. ASCII only -- avoids tofu // if a receiving node's font lacks a glyph. p->decoded.payload.size = snprintf(reinterpret_cast(p->decoded.payload.bytes), sizeof(p->decoded.payload.bytes), - GAMES_HIGH_SCORE_STRING, active->name(), initials, static_cast(score)); + GAMES_HIGH_SCORE_STRING, active->name(), static_cast(score), initials); service->sendToMesh(p); LOG_INFO("Games: announced new %s high score %lu", active->name(), static_cast(score)); } @@ -151,6 +160,27 @@ void GamesModule::kickTick() int32_t GamesModule::runOnce() { + const uint32_t now = millis(); + + // Whether the games UI is actually in front of the player: a game is active (which forces the + // games frame), or the attract screen is the current frame. When it is, an idle stretch bounces + // back to the home frame so a walked-away device reads as a Meshtastic node. + const bool gamesVisible = (uiState != GAMES_IDLE) || (screen && screen->isGamesFrameShown()); + if (gamesVisible) { + if (screen && screen->isOverlayBannerShowing()) { + // A picker or banner is up (e.g. high-score initials entry, which our handleInputEvent + // never sees). The user is busy with it -- don't time out and yank them away. + lastActivityMs = now; + } else if (now - lastActivityMs >= INACTIVITY_TIMEOUT_MS) { + goHome(); + return disable(); + } + } else { + // Not in front of the player (attract screen is just one of the rotating frames, and the + // player is elsewhere): keep the timer fresh so a later visit starts a full 15 s. + lastActivityMs = now; + } + if (uiState == GAMES_PLAYING && active) { if (!active->tick()) { enterGameOver(); @@ -158,7 +188,6 @@ int32_t GamesModule::runOnce() } // Keep the display awake through long runs that generate no key presses. - const uint32_t now = millis(); if (now - lastAwakeKickMs > 1500) { powerFSM.trigger(EVENT_PRESS); lastAwakeKickMs = now; @@ -168,13 +197,18 @@ int32_t GamesModule::runOnce() return active->tickIntervalMs(); } - // Idle: service any game that broadcasts periodically; sleep until the soonest one is due. + // Idle-ish (attract / paused / game-over / high scores): service any periodic mesh broadcast, + // and while the games UI is visible keep a slow poll running so the inactivity timeout fires. int32_t next = -1; for (Game *g : games) { const int32_t due = g->meshTick(*this); if (due >= 0 && (next < 0 || due < next)) next = due; } + if (gamesVisible) { + const int32_t poll = 1000; + return (next >= 0 && next < poll) ? next : poll; + } return next < 0 ? disable() : next; } @@ -191,9 +225,16 @@ int GamesModule::handleInputEvent(const InputEvent *event) if (screen->isOverlayBannerShowing()) return 0; // a menu banner is up; don't steal its input + noteActivity(); // any input on the games frame resets the return-to-home timer + const input_broker_event ev = event->inputEvent; const bool isBack = (ev == INPUT_BROKER_CANCEL || ev == INPUT_BROKER_BACK); + // Start is mapped to select like any other button, so it launches games and works the menus. + // Inside a running game it means pause/resume instead, which we can tell only because kbchar + // names the physical button behind the action. + const bool isPauseButton = (ev == INPUT_BROKER_SELECT && isJoyStartButton(event->kbchar)); + switch (uiState) { case GAMES_IDLE: // Attract screen: UP/DOWN cycle which game is shown; SELECT (handled by Screen) launches it; @@ -214,12 +255,22 @@ int GamesModule::handleInputEvent(const InputEvent *event) return 0; case GAMES_PLAYING: - if (isBack) { + // Start pauses, and is never forwarded to the game: unlike BACK it has no second meaning + // in play, so a game cannot claim it the way Breakout claims BACK to serve. + if (isPauseButton) { + uiState = GAMES_PAUSED; + disable(); + requestRedraw(); + return 1; + } + // BACK pauses, unless the active game has temporarily claimed that button (see + // Game::wantsBackButton) -- then it is forwarded like any other key. + if (isBack && !(active && active->wantsBackButton())) { uiState = GAMES_PAUSED; // BACK to pause; from there choose resume or quit disable(); requestRedraw(); } else if (active) { - active->handleInput(ev); + active->handleInput(event); if (!active->isPlaying()) { enterGameOver(); return 1; @@ -232,7 +283,7 @@ int GamesModule::handleInputEvent(const InputEvent *event) if (isBack) { exitToIdle(); // quit from pause } else if (ev == INPUT_BROKER_SELECT || ev == INPUT_BROKER_UP || ev == INPUT_BROKER_DOWN || ev == INPUT_BROKER_LEFT || - ev == INPUT_BROKER_RIGHT) { + ev == INPUT_BROKER_RIGHT) { // Start arrives as SELECT, so it resumes too uiState = GAMES_PLAYING; kickTick(); requestRedraw(); @@ -317,6 +368,18 @@ void GamesModule::drawFrame(OLEDDisplay *display, OLEDDisplayUiState * /*state*/ { display->setColor(WHITE); + // drawFrame runs only while the games frame is the current frame. Several idle-ish states + // (attract / paused / game-over / high-scores) otherwise leave the tick thread asleep, so use + // this render as the trigger to (re)start the slow inactivity poll and reset the timer -- so + // arriving on such a screen gets a fresh 15 s before we bounce back home, and walking away from + // it eventually does. (While a game is PLAYING the thread is already ticking, so !enabled is + // false here and the play-time timer is left to run.) + if (!enabled) { + noteActivity(); + enabled = true; + setIntervalFromNow(1000); + } + switch (uiState) { case GAMES_IDLE: if (!games.empty()) diff --git a/src/modules/games/GamesModule.h b/src/modules/games/GamesModule.h index eb4642f19..f24ec0080 100644 --- a/src/modules/games/GamesModule.h +++ b/src/modules/games/GamesModule.h @@ -67,12 +67,18 @@ class GamesModule : public SinglePortModule, public Observable inputObserver = CallbackObserver(this, &GamesModule::handleInputEvent); + // After this long with no input while the games frame is up, bounce back to the home frame so a + // walked-away device clearly reads as a Meshtastic node rather than sitting on a game screen. + static constexpr uint32_t INACTIVITY_TIMEOUT_MS = 15000; + // === State transitions === void startPlaying(); void enterGameOver(); void exitToIdle(); void requestRedraw(); void kickTick(); + void noteActivity() { lastActivityMs = millis(); } // reset the inactivity timer + void goHome(); // exit any game and switch to the home frame // === Shared game-over / high-score flow === void promptForInitials(); @@ -94,6 +100,7 @@ class GamesModule : public SinglePortModule, public Observable RIGHT -> DOWN -> LEFT. + static constexpr Direction CW[4] = {DIR_RIGHT, DIR_LEFT, DIR_UP, DIR_DOWN}; + static constexpr Direction CCW[4] = {DIR_LEFT, DIR_RIGHT, DIR_DOWN, DIR_UP}; + setDirection(clockwise ? CW[dir] : CCW[dir]); +} + uint32_t SnakeGame::nextRandom() { uint32_t x = rng; @@ -172,8 +181,20 @@ int32_t Snake::tickIntervalMs() const return iv < 70 ? 70 : iv; } -void Snake::handleInput(input_broker_event ev) +void Snake::handleInput(const InputEvent *event) { + const input_broker_event ev = event->inputEvent; + const unsigned char kbchar = event->kbchar; + + // Shoulder-button steering: a gamepad button mapped to left/right turns the snake relative to + // where it is already heading (L counter-clockwise, R clockwise) rather than setting an + // absolute heading. The D-pad and keyboard keep steering absolutely -- the D-pad is an axis, + // so it arrives with no button in kbchar, which is exactly what tells the two apart. + if (isJoyButton(kbchar) && (ev == INPUT_BROKER_LEFT || ev == INPUT_BROKER_RIGHT)) { + game.turn(ev == INPUT_BROKER_RIGHT); + return; + } + switch (ev) { case INPUT_BROKER_UP: game.setDirection(SnakeGame::DIR_UP); diff --git a/src/modules/games/Snake.h b/src/modules/games/Snake.h index 293707ff4..c32258d6e 100644 --- a/src/modules/games/Snake.h +++ b/src/modules/games/Snake.h @@ -47,6 +47,18 @@ class SnakeGame */ bool setDirection(Direction d); + /** + * Steer relative to the current heading: a quarter turn clockwise, or counter-clockwise. + * Clockwise is what the player sees on screen (UP -> RIGHT -> DOWN -> LEFT), remembering that + * y grows downward. A quarter turn from the committed heading can never be a reversal, so + * unlike setDirection() this always takes. + * + * Turning from the committed direction rather than the pending one is deliberate, and matches + * setDirection()'s reasoning: two presses inside one tick then settle on a single quarter + * turn instead of chaining into the 180 that would run the head into the neck. + */ + void turn(bool clockwise); + /** * Advance the simulation by one tick. Returns true if the snake is still alive afterwards, * false if this move ended the game (wall hit, self-collision, or board filled == win). @@ -128,7 +140,7 @@ class Snake : public Game uint32_t score() const override { return game.score(); } int32_t tickIntervalMs() const override; - void handleInput(input_broker_event ev) override; + void handleInput(const InputEvent *event) override; void drawAttract(OLEDDisplay *display, int16_t x, int16_t y) override; void drawPlaying(OLEDDisplay *display, int16_t x, int16_t y) override; diff --git a/src/modules/games/Tetris.cpp b/src/modules/games/Tetris.cpp index 7c24b3269..1a39ca59d 100644 --- a/src/modules/games/Tetris.cpp +++ b/src/modules/games/Tetris.cpp @@ -285,9 +285,9 @@ int32_t Tetris::tickIntervalMs() const return iv < 50 ? 50 : iv; } -void Tetris::handleInput(input_broker_event ev) +void Tetris::handleInput(const InputEvent *event) { - switch (ev) { + switch (event->inputEvent) { case INPUT_BROKER_UP: game.rotate(); break; diff --git a/src/modules/games/Tetris.h b/src/modules/games/Tetris.h index 4b429ec2e..1844769bd 100644 --- a/src/modules/games/Tetris.h +++ b/src/modules/games/Tetris.h @@ -131,7 +131,7 @@ class Tetris : public Game uint32_t score() const override { return game.score(); } int32_t tickIntervalMs() const override; - void handleInput(input_broker_event ev) override; + void handleInput(const InputEvent *event) override; void drawAttract(OLEDDisplay *display, int16_t x, int16_t y) override; void drawPlaying(OLEDDisplay *display, int16_t x, int16_t y) override; diff --git a/src/platform/portduino/ConfigCheck.cpp b/src/platform/portduino/ConfigCheck.cpp index d03ca7c09..950c063b9 100644 --- a/src/platform/portduino/ConfigCheck.cpp +++ b/src/platform/portduino/ConfigCheck.cpp @@ -37,6 +37,9 @@ constexpr int MAX_NODES_SANITY_CEILING = 16000; const std::set kLoraPinKeys = {"CS", "IRQ", "Busy", "Reset", "TXen", "RXen", "SX126X_ANT_SW", "GPIO_DETECT_PA"}; +// Action names LinuxJoystick understands; anything else leaves the button unmapped. +const std::set kJoystickActions = {"select", "cancel", "back", "up", "down", "left", "right", "user", "userpress"}; + const std::map> &schema() { static const std::map> s = { @@ -366,6 +369,61 @@ void checkPinNode(const std::string &file, const std::string &path, const YAML:: } } +// Keyed by action, not by button, so one action can list several codes and have every one of +// those buttons drive it. The value is a single evdev code or a list of them. +void checkJoystickButtons(const std::string &file, const YAML::Node &node, std::vector &findings) +{ + if (!node.IsMap()) { + findings.push_back( + {kError, file, lineOf(node), "Input.JoystickButtons must be a mapping of action name to evdev button code"}); + return; + } + + std::map owner; // code -> the action that claimed it first + for (const auto &entry : node) { + std::string action = entry.first.as(""); + for (auto &c : action) + c = tolower(c); + if (!kJoystickActions.count(action)) { + findings.push_back({kWarn, file, lineOf(entry.first), + "Input.JoystickButtons: '" + action + + "' is not a recognised action, so those buttons do nothing. Valid actions are select, " + "cancel, back, up, down, left, right and user"}); + continue; + } + + std::vector codeNodes; + if (entry.second.IsSequence()) + for (const auto &codeNode : entry.second) + codeNodes.push_back(codeNode); + else + codeNodes.push_back(entry.second); + + for (const auto &codeNode : codeNodes) { + const std::string raw = codeNode.as(""); + int code = 0; + try { + code = std::stoi(raw, nullptr, 0); + } catch (const std::exception &) { + code = 0; + } + if (code == 0) { + findings.push_back({kWarn, file, lineOf(codeNode), + "Input.JoystickButtons." + action + ": '" + raw + + "' is not an evdev button code (hex like 0x121, or decimal), so it is unmapped"}); + continue; + } + // One button cannot do two things: the later action silently replaces the earlier one. + const auto claimed = owner.find(code); + if (claimed != owner.end() && claimed->second != action) + findings.push_back({kWarn, file, lineOf(codeNode), + "Input.JoystickButtons: button " + raw + " is mapped to both '" + claimed->second + + "' and '" + action + "'. Only '" + action + "' takes effect"}); + owner[code] = action; + } + } +} + void checkRfSwitchTable(const std::string &file, const YAML::Node &table, std::vector &findings) { if (!table.IsMap()) { @@ -837,7 +895,7 @@ void checkSection(const std::string &file, const std::string §ion, const YAM for (const auto &pin : value) checkPinNode(file, section + "." + key, pin, findings); } else if (key == "JoystickButtons") { - // Free-form: any action name mapped to an evdev code. + checkJoystickButtons(file, value, findings); } else if ((section == "Lora" && kLoraPinKeys.count(key)) || (section == "Display" && (key == "DC" || key == "CS" || key == "Backlight" || key == "BacklightPWMChannel" || key == "Reset")) || diff --git a/src/platform/portduino/PortduinoGlue.cpp b/src/platform/portduino/PortduinoGlue.cpp index 8fe452aa0..9c908a4bc 100644 --- a/src/platform/portduino/PortduinoGlue.cpp +++ b/src/platform/portduino/PortduinoGlue.cpp @@ -1297,21 +1297,33 @@ bool loadConfig(const char *configPath) portduino_config.pointerDevice = (yamlConfig["Input"]["PointerDevice"]).as(""); portduino_config.joystickDevice = (yamlConfig["Input"]["JoystickDevice"]).as(""); if (yamlConfig["Input"]["JoystickButtons"]) { - // action name -> evdev button code (hex like 0x122 or decimal); stored inverted - // as code -> lowercase action name for the driver to look up per keypress. + // action name -> evdev button code (hex like 0x122 or decimal), or a list of codes + // so several physical buttons drive the same action. Stored inverted as + // code -> lowercase action name for the driver to look up per keypress. for (const auto &button : yamlConfig["Input"]["JoystickButtons"]) { std::string action = button.first.as(""); for (auto &c : action) c = tolower(c); - int code = 0; - try { - // base 0 accepts hex (0x122) or decimal; a malformed value just skips this entry. - code = std::stoi(button.second.as(""), nullptr, 0); - } catch (const std::exception &) { - code = 0; + if (action == "") + continue; + // A bare scalar is just a one-entry list. + std::vector codeNodes; + if (button.second.IsSequence()) + for (const auto &codeNode : button.second) + codeNodes.push_back(codeNode); + else + codeNodes.push_back(button.second); + for (const auto &codeNode : codeNodes) { + int code = 0; + try { + // base 0 accepts hex (0x122) or decimal; a malformed value just skips this entry. + code = std::stoi(codeNode.as(""), nullptr, 0); + } catch (const std::exception &) { + code = 0; + } + if (code != 0) + portduino_config.joystickButtons[code] = action; } - if (code != 0 && action != "") - portduino_config.joystickButtons[code] = action; } } diff --git a/src/platform/portduino/PortduinoGlue.h b/src/platform/portduino/PortduinoGlue.h index 3e77894f5..31838d42a 100644 --- a/src/platform/portduino/PortduinoGlue.h +++ b/src/platform/portduino/PortduinoGlue.h @@ -545,9 +545,23 @@ extern struct portduino_config_struct { if (joystickDevice != "") out << YAML::Key << "JoystickDevice" << YAML::Value << joystickDevice; if (!joystickButtons.empty()) { - out << YAML::Key << "JoystickButtons" << YAML::Value << YAML::BeginMap; + // Stored as code -> action; invert so each action lists every code bound to it. + // Several buttons may share one action, so a multi-code action emits a list. + std::map> codesByAction; for (const auto &button : joystickButtons) - out << YAML::Key << button.second << YAML::Value << button.first; + codesByAction[button.second].push_back(button.first); + out << YAML::Key << "JoystickButtons" << YAML::Value << YAML::BeginMap; + for (const auto &action : codesByAction) { + out << YAML::Key << action.first << YAML::Value; + if (action.second.size() == 1) { + out << action.second.front(); + } else { + out << YAML::Flow << YAML::BeginSeq; + for (const int code : action.second) + out << code; + out << YAML::EndSeq; + } + } out << YAML::EndMap; } diff --git a/test/fixtures/portduino-config/README.md b/test/fixtures/portduino-config/README.md index 729d794fa..f0e386ed3 100644 --- a/test/fixtures/portduino-config/README.md +++ b/test/fixtures/portduino-config/README.md @@ -161,6 +161,16 @@ if those yield nothing meshtasticd exits with "Blank MAC Address not allowed!". | `mac-malformed.yaml` | `AA:BB:CC` is under 12 hex digits, so it is silently dropped. | | `mac-source-missing.yaml` | Names an interface with no `/sys/class/net//address`. Warning, not an error: it is machine-dependent and may be checked on another host. | +## Joystick buttons + +`Input.JoystickButtons` is keyed by action, not by button, so an action can name a +single evdev code or a list of them and every one of those buttons drives it. + +| File | Expected | +| --------------------------- | -------------------------------------------------------------------------------------------------------------- | +| `joystick-buttons.yaml` | **Clean, and a regression guard** - four actions, two of them with several codes. | +| `joystick-buttons-bad.yaml` | Three silent no-ops: an action name nothing reads, an evdev name where a code belongs, one code claimed twice. | + ## CH341 USB-SPI adapters `spidev: ch341` is a different hardware model, not a variant of the same one. The Lora diff --git a/test/fixtures/portduino-config/joystick-buttons-bad.yaml b/test/fixtures/portduino-config/joystick-buttons-bad.yaml new file mode 100644 index 000000000..c25aad54c --- /dev/null +++ b/test/fixtures/portduino-config/joystick-buttons-bad.yaml @@ -0,0 +1,16 @@ +# FAULT: three ways a joystick mapping silently does nothing. 'fire' is not an action +# LinuxJoystick knows, so those buttons are dropped; 'BTN_SOUTH' is a name rather than the +# numeric evdev code the loader parses; and 0x121 appears under two actions, where the +# later one wins and the earlier binding is lost. +Lora: + Module: sx1262 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 +Input: + JoystickDevice: /dev/input/by-id/usb-0079_USB_Gamepad-event-joystick + JoystickButtons: + select: [0x121, BTN_SOUTH] + fire: 0x123 + cancel: [0x122, 0x121] diff --git a/test/fixtures/portduino-config/joystick-buttons.yaml b/test/fixtures/portduino-config/joystick-buttons.yaml new file mode 100644 index 000000000..d325f6f24 --- /dev/null +++ b/test/fixtures/portduino-config/joystick-buttons.yaml @@ -0,0 +1,17 @@ +# CLEAN. Several physical buttons bound to one action: an action's value may be a single +# evdev code or a list of them, so A and Y both select, B and X both cancel, and the +# shoulder buttons work as left/right alongside the D-pad. Nothing here is a fault -- the +# case exists so the checker cannot start rejecting the list form. +Lora: + Module: sx1262 + CS: 21 + IRQ: 16 + Busy: 20 + Reset: 18 +Input: + JoystickDevice: /dev/input/by-id/usb-0079_USB_Gamepad-event-joystick + JoystickButtons: + select: [0x121, 0x123] + cancel: [0x122, 0x120] + left: 0x124 + right: 0x125 diff --git a/test/test_breakout/test_main.cpp b/test/test_breakout/test_main.cpp index 1ca2da5e2..d1b7b8dbe 100644 --- a/test/test_breakout/test_main.cpp +++ b/test/test_breakout/test_main.cpp @@ -2,11 +2,21 @@ #include "modules/games/Breakout.h" #include -// Pure-logic tests for BreakoutGame: initial serve/brick state, paddle clamping, brick-clearing on -// a straight-up serve, and the ball staying within the board. No device globals or display stack. +// Pure-logic tests for BreakoutGame: initial serve/brick state, paddle clamping, the ball waiting +// on the paddle until launched, brick-clearing on a straight-up serve, and the ball staying within +// the board. No device globals or display stack. static const uint32_t kSeed = 0xC0FFEEu; +// The ball docks on the paddle after every serve (including after losing a life), so a test that +// wants continuous play has to fire it whenever it is waiting. +static void stepLaunched(BreakoutGame &game) +{ + if (game.isBallDocked()) + game.launchBall(); + game.step(); +} + void test_reset_initialState() { BreakoutGame game; @@ -21,6 +31,37 @@ void test_reset_initialState() TEST_ASSERT_EQUAL_INT16((BreakoutGame::BOARD_W - BreakoutGame::PADDLE_W) / 2, game.paddleX()); TEST_ASSERT_TRUE(game.ballX() >= 0 && game.ballX() < BreakoutGame::BOARD_W); TEST_ASSERT_TRUE(game.ballY() >= 0 && game.ballY() < BreakoutGame::BOARD_H); + // The ball waits on the paddle until the player serves it. + TEST_ASSERT_TRUE(game.isBallDocked()); +} + +void test_ball_waitsOnPaddleUntilLaunched() +{ + BreakoutGame game; + game.reset(kSeed); + const int16_t restY = game.ballY(); + + // Stepping without serving must not move the ball vertically, lose a life, or end the run. + for (int i = 0; i < 50; i++) + game.step(); + TEST_ASSERT_TRUE(game.isBallDocked()); + TEST_ASSERT_EQUAL_INT16(restY, game.ballY()); + TEST_ASSERT_EQUAL_UINT8(BreakoutGame::START_LIVES, game.lives()); + TEST_ASSERT_TRUE(game.isPlaying()); + + // A docked ball tracks the paddle, so it can still be aimed before serving. + const int16_t beforeX = game.ballX(); + for (int i = 0; i < 5; i++) + game.moveLeft(); + game.step(); + TEST_ASSERT_TRUE(game.ballX() < beforeX); + + // Once launched it is in play and starts climbing toward the bricks. + game.launchBall(); + TEST_ASSERT_FALSE(game.isBallDocked()); + for (int i = 0; i < 4; i++) + game.step(); + TEST_ASSERT_TRUE(game.ballY() < restY); } void test_paddle_clampsToEdges() @@ -39,8 +80,9 @@ void test_serve_clearsABrickAndScores() { BreakoutGame game; game.reset(kSeed); - // The ball serves upward from just above the paddle straight into the brick field; within a - // few dozen steps it must clear at least one brick and score. + // Once served, the ball travels upward from just above the paddle straight into the brick field; + // within a few dozen steps it must clear at least one brick and score. + game.launchBall(); for (int i = 0; i < 60 && game.bricksRemaining() == static_cast(BreakoutGame::BRICK_ROWS) * BreakoutGame::BRICK_COLS; i++) game.step(); @@ -59,7 +101,7 @@ void test_ball_staysInBounds() game.moveLeft(); else game.moveRight(); - game.step(); + stepLaunched(game); TEST_ASSERT_TRUE(game.ballX() >= 0 && game.ballX() < BreakoutGame::BOARD_W); TEST_ASSERT_TRUE(game.ballY() >= 0); } @@ -69,10 +111,15 @@ void test_deadGame_stepIsNoOp() { BreakoutGame game; game.reset(kSeed); - // Park the paddle in a corner and never move it; the ball is eventually lost every life. - game.moveLeft(); + // Serve each ball, then steer the paddle AWAY from it so every ball is missed and all lives + // drain. (The ball re-docks after each loss, so it has to be re-served. Note it now serves from + // the paddle's centre, so simply parking the paddle would let it rally instead of dying.) for (int i = 0; i < 20000 && game.isPlaying(); i++) { - for (int j = 0; j < 40; j++) // hold the paddle pinned left + if (game.isBallDocked()) + game.launchBall(); + else if (game.ballX() < game.paddleX()) + game.moveRight(); + else game.moveLeft(); game.step(); } @@ -92,6 +139,7 @@ void setup() initializeTestEnvironment(); UNITY_BEGIN(); RUN_TEST(test_reset_initialState); + RUN_TEST(test_ball_waitsOnPaddleUntilLaunched); RUN_TEST(test_paddle_clampsToEdges); RUN_TEST(test_serve_clearsABrickAndScores); RUN_TEST(test_ball_staysInBounds); diff --git a/test/test_snake/test_main.cpp b/test/test_snake/test_main.cpp index 8668e66a5..5fa576ac6 100644 --- a/test/test_snake/test_main.cpp +++ b/test/test_snake/test_main.cpp @@ -64,6 +64,46 @@ static void test_setDirection_rejectsReversal() TEST_ASSERT_FALSE(game.setDirection(SnakeGame::DIR_LEFT)); } +// turn() is the shoulder-button steering: a quarter turn relative to the current heading rather +// than an absolute direction. A quarter turn is never a reversal, so it always takes. +static void test_turn_cyclesThroughHeadings() +{ + SnakeGame game; + game.reset(kSeed); // heading right + game.placeFoodAt(0, 0); + + // Clockwise on screen (y grows downward): RIGHT -> DOWN -> LEFT -> UP -> RIGHT. + const SnakeGame::Direction cw[] = {SnakeGame::DIR_DOWN, SnakeGame::DIR_LEFT, SnakeGame::DIR_UP, SnakeGame::DIR_RIGHT}; + for (SnakeGame::Direction want : cw) { + game.turn(true); + TEST_ASSERT_TRUE(game.step()); // commit the pending turn + TEST_ASSERT_EQUAL_INT(want, game.direction()); + } + + // Counter-clockwise runs the cycle backwards: RIGHT -> UP -> LEFT -> DOWN -> RIGHT. + const SnakeGame::Direction ccw[] = {SnakeGame::DIR_UP, SnakeGame::DIR_LEFT, SnakeGame::DIR_DOWN, SnakeGame::DIR_RIGHT}; + for (SnakeGame::Direction want : ccw) { + game.turn(false); + TEST_ASSERT_TRUE(game.step()); + TEST_ASSERT_EQUAL_INT(want, game.direction()); + } +} + +// Two turns inside one tick must not chain into a 180 that runs the head into its own neck -- +// the second turn is taken from the committed heading, not the pending one. +static void test_turn_twiceInOneTickIsNotAReversal() +{ + SnakeGame game; + game.reset(kSeed); // heading right + game.placeFoodAt(0, 0); + + game.turn(true); + game.turn(true); // would be RIGHT -> DOWN -> LEFT if it chained + TEST_ASSERT_TRUE(game.step()); + TEST_ASSERT_EQUAL_INT(SnakeGame::DIR_DOWN, game.direction()); + TEST_ASSERT_TRUE(game.isPlaying()); +} + static void test_step_movesAndTailFollows() { SnakeGame game; @@ -168,6 +208,8 @@ void setup() RUN_TEST(test_reset_initialState); RUN_TEST(test_food_isValidAndOffBody); RUN_TEST(test_setDirection_rejectsReversal); + RUN_TEST(test_turn_cyclesThroughHeadings); + RUN_TEST(test_turn_twiceInOneTickIsNotAReversal); RUN_TEST(test_step_movesAndTailFollows); RUN_TEST(test_eat_growsAndScores); RUN_TEST(test_wallCollision_endsGame); From a0c230e091c3b7cf8f01cdd8aa6355bf1752dd13 Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Sun, 20 Sep 2026 04:12:35 +0000 Subject: [PATCH 5/9] fix(baseui): stop the connection footer erasing the last body row (#11918) drawCommonFooter() black-fills the bottom (connection_icon_height + 2) rows before drawing the API-connected link icon. On colour builds that fill spans the whole screen width; the monochrome path fills only the icon's own width. The body grid does not shrink with the panel. textSixthLine is 58 rows down whatever the display height, while the footer band starts at SCREEN_HEIGHT - 1 - connection_icon_height. On a 64-row panel that is row 58 -- exactly the sixth body line -- so the bar erases the last thing the frame drew. On BaseUI that is the sixth body row, the LoRa frame's ChUtil bar (rows 52-59) and the bottom of the clock. Taller panels put the band clear of the grid and are unaffected: at 80 rows it starts at 74, and on high-res it is far below. It only shows once a client is connected, since the function early-returns on !isAPIConnected(), which is why it reads as the blue link icon eating the screen rather than as a layout bug. Only the icon's own rect is registered for colour tinting, so the wide fill buys the tint nothing. Keep it where it clears the body, and fall back to the icon-width fill -- what the monochrome path already does -- where it would not. Panels with room below the body are byte-identical to before. Seen on a 128x64 HUB75 running meshtasticd; esp32s3/visualizer-hub75 is the same geometry. Co-authored-by: Claude Opus 5 (1M context) --- src/graphics/SharedUIDisplay.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/graphics/SharedUIDisplay.cpp b/src/graphics/SharedUIDisplay.cpp index 762b61823..0b721bfc2 100644 --- a/src/graphics/SharedUIDisplay.cpp +++ b/src/graphics/SharedUIDisplay.cpp @@ -603,7 +603,17 @@ void drawCommonFooter(OLEDDisplay *display, int16_t x, int16_t y) display->setColor(BLACK); #if GRAPHICS_TFT_COLORING_ENABLED - display->fillRect(0, footerY, SCREEN_WIDTH, footerH); + // The full-width bar reads as a clean footer where there is room below the body, but the band is + // the bottom (connection_icon_height + 2) rows and the body grid does not shrink with the panel: + // textSixthLine is 58 whatever the height, so on a 64-row display the bar lands exactly on it and + // erases the last thing the frame drew (the sixth body line, the LoRa ChUtil bar, the clock). + // Only the icon's own rect is colour-tinted, so the wide fill buys the tint nothing - fall back to + // the icon-width fill, as the monochrome path already does, whenever it would overlap the body. + const int bodyBottom = getTextPositions(display)[6] + FONT_HEIGHT_SMALL; + if (footerY >= bodyBottom) + display->fillRect(0, footerY, SCREEN_WIDTH, footerH); + else + display->fillRect(0, footerY, connection_icon_width + 1, footerH); #else display->fillRect(0, footerY, connection_icon_width + 1, footerH); #endif From d3b4b343e7e9194757debb5c1a34cf931e2bdeb2 Mon Sep 17 00:00:00 2001 From: Jonathan Bennett Date: Sun, 20 Sep 2026 05:21:06 +0000 Subject: [PATCH 6/9] Send an ack over PKC when no channel can carry it (#11891) * Send an ack over PKC when no channel can carry it PKI needs only the two keys, so a DM can reach us over a channel we do not carry. Its ack is a ROUTING packet, which wouldEncryptWithPKC() excludes, so today it is channel-encoded, fails at setActiveByIndex() with NO_CHANNEL, and is never sent. The sender sees nothing and retransmits to exhaustion for a message that was in fact delivered. Fall back to PKC for exactly that case. This is the one place an ack is deliberately made opaque to relays; normally that costs next-hop learning and intermediate retransmission cancel, which is why ROUTING is PKC-excluded in general, but here there is no readable alternative to lose, because without this the ack does not exist. The predicate is scoped as tightly as that argument reaches: a unicast ROUTING packet we originate, carrying a request_id, to a destination whose key we hold, under the same ham/sim/private-key preconditions PKC always has, and only when the channel index does not resolve. It tests channels.getHash() rather than setActiveByIndex() so it has no side effect; generateHash already returns -1 for an invalid key, so the two agree on which indexes are unusable. Four cases in test_packet_signing pin the corners: the fallback fires, it does not paper over an ack with no destination key, it does not catch a non-ack on the same unusable channel, and an ack on a channel that does resolve still goes out readable. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8 * Short-circuit the fallback so an out-of-range channel logs no error wouldEncryptWithPKC() reaches channels.getName(chIndex) before its portnum exclusion, and getByIndex() logs "Invalid channel index" on the way past. With the general predicate tested first, an ack on an out-of-range index printed that error and then went on to encode successfully. Test ackFallback first so the case that is about to succeed never asks. Also record why the range check leads inside the predicate: getHash() is a bare hashes[i] with no bounds test of its own. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012hLcVif8GDEmA2k77hmFG8 --------- Co-authored-by: Claude --- src/mesh/Router.cpp | 41 +++++++++- test/test_packet_signing/test_main.cpp | 104 +++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 1 deletion(-) diff --git a/src/mesh/Router.cpp b/src/mesh/Router.cpp index 9b95a57eb..a72a14dd8 100644 --- a/src/mesh/Router.cpp +++ b/src/mesh/Router.cpp @@ -1206,6 +1206,36 @@ bool wouldEncryptWithPKC(const meshtastic_MeshPacket *p, ChannelIndex chIndex, b // to handle the case where the remote node has our key, but we don't have theirs. !(p->decoded.portnum == meshtastic_PortNum_KEY_VERIFICATION_APP && !haveDestKey); } + +/** + * PKC fallback for an ack that has no channel in common with the sender. + * + * PKI needs only the two keys, so a DM can reach us over a channel we do not carry. Its ack is a + * ROUTING packet, which wouldEncryptWithPKC() excludes, so it would be channel-encoded, fail at + * setActiveByIndex() with NO_CHANNEL, and never be sent - leaving the sender to retransmit to + * exhaustion for a message that was in fact delivered. + * + * This is the one place an ack is deliberately made opaque to relays. Normally that costs next-hop + * learning and intermediate retransmission cancel, which is why ROUTING is PKC-excluded in general; + * here there is no readable alternative to lose, because without this the ack does not exist. + * + * Scoped as tightly as that argument reaches: a unicast ROUTING packet we originate, carrying a + * request_id, to a destination whose key we hold, under the same ham/sim/private-key preconditions + * PKC always has - and only when the channel index does not resolve. It tests channels.getHash() + * rather than setActiveByIndex() so the predicate has no side effect; generateHash already returns + * -1 for an invalid key, so the two agree on which indexes are unusable. The range check has to come + * first and stay first: getHash() is a bare hashes[i] with no bounds test of its own. + */ +static bool ackNeedsPkcFallback(const meshtastic_MeshPacket *p, ChannelIndex chIndex, bool haveDestKey) +{ + return isFromUs(p) && +#if ARCH_PORTDUINO + !portduino_config.force_simradio && +#endif + !owner.is_licensed && config.security.private_key.size == 32 && haveDestKey && !isBroadcast(p->to) && + p->decoded.portnum == meshtastic_PortNum_ROUTING_APP && p->decoded.request_id != 0 && + (chIndex >= MAX_NUM_CHANNELS || channels.getHash(chIndex) < 0); +} #endif /** Return 0 for success or a Routing_Error code for failure @@ -1299,9 +1329,18 @@ meshtastic_Routing_Error perhapsEncode(meshtastic_MeshPacket *p) crypto->getPendingPublicKey(p->to, destKey)) { haveDestKey = true; } + const bool ackFallback = ackNeedsPkcFallback(p, chIndex, haveDestKey); + if (ackFallback) + LOG_INFO("No usable channel %d for ack of 0x%08x, send it over PKC", chIndex, p->decoded.request_id); + // We may want to retool things so we can send a PKC packet when the client specifies a key and nodenum, even if the node // is not in the local nodedb - if (wouldEncryptWithPKC(p, chIndex, haveDestKey)) { + // + // ackFallback is tested first so an out-of-range chIndex short-circuits: wouldEncryptWithPKC + // reaches channels.getName(chIndex) before its portnum exclusion, and getByIndex() logs + // "Invalid channel index" on the way past. Without the short-circuit this path would print + // an error and then go on to encode the packet successfully. + if (ackFallback || wouldEncryptWithPKC(p, chIndex, haveDestKey)) { LOG_DEBUG("Use PKI"); if (numbytes + MESHTASTIC_HEADER_LENGTH + MESHTASTIC_PKC_OVERHEAD > MAX_LORA_PAYLOAD_LEN) return meshtastic_Routing_Error_TOO_LARGE; diff --git a/test/test_packet_signing/test_main.cpp b/test/test_packet_signing/test_main.cpp index 50162dc5c..2a5cfdc98 100644 --- a/test/test_packet_signing/test_main.cpp +++ b/test/test_packet_signing/test_main.cpp @@ -1091,6 +1091,106 @@ void test_B13_licensed_port_and_destination_signing_matrix(void) } } +// B14: PKI needs only the two keys, so a DM can arrive over a channel we do not carry. Its ack is a +// ROUTING packet, which is PKC-excluded, so it would be channel-encoded and die with NO_CHANNEL - +// and the sender would then retransmit to exhaustion for a message that WAS delivered. Fall back to +// PKC for exactly that case. +void test_B14_ack_with_no_usable_channel_falls_back_to_pkc(void) +{ + uint8_t localPub[32], localPriv[32], remotePub[32], remotePriv[32]; + crypto->generateKeyPair(localPub, localPriv); + crypto->generateKeyPair(remotePub, remotePriv); + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->setPublicKey(LOCAL_NODE, localPub); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePub); + memcpy(config.security.private_key.bytes, localPriv, sizeof(localPriv)); + config.security.private_key.size = sizeof(localPriv); + crypto->setDHPrivateKey(localPriv); + + // A secondary channel index that does not resolve - otherwise the test proves nothing. + const ChannelIndex deadChannel = 1; + TEST_ASSERT_LESS_THAN_MESSAGE(0, channels.getHash(deadChannel), "test needs an unusable channel index"); + + meshtastic_MeshPacket ack = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_ROUTING_APP, SMALL_PAYLOAD); + ack.decoded.request_id = 0xFEED5150; + ack.channel = deadChannel; + + TEST_ASSERT_EQUAL_MESSAGE(meshtastic_Routing_Error_NONE, perhapsEncode(&ack), + "ack on an unusable channel must not fail to send"); + TEST_ASSERT_TRUE_MESSAGE(ack.pki_encrypted, "it must have gone out over PKC"); +} + +// The fallback must not paper over a genuinely unsendable ack: with no key for the destination there +// is nothing to encrypt to, and NO_CHANNEL is still the honest answer. +void test_B15_ack_with_no_channel_and_no_key_still_fails(void) +{ + uint8_t localPub[32], localPriv[32]; + crypto->generateKeyPair(localPub, localPriv); + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->setPublicKey(LOCAL_NODE, localPub); + memcpy(config.security.private_key.bytes, localPriv, sizeof(localPriv)); + config.security.private_key.size = sizeof(localPriv); + crypto->setDHPrivateKey(localPriv); + // REMOTE_NODE deliberately absent from the DB, so we hold no key for it. + + const ChannelIndex deadChannel = 1; + TEST_ASSERT_LESS_THAN(0, channels.getHash(deadChannel)); + + meshtastic_MeshPacket ack = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_ROUTING_APP, SMALL_PAYLOAD); + ack.decoded.request_id = 0xFEED5150; + ack.channel = deadChannel; + + TEST_ASSERT_EQUAL_MESSAGE(meshtastic_Routing_Error_NO_CHANNEL, perhapsEncode(&ack), + "without a destination key the ack is genuinely unsendable"); +} + +// The fallback is scoped to acks: a non-ROUTING unicast on an unusable channel still fails, so this +// does not quietly turn every channel-less packet into a PKC packet. +void test_B16_non_ack_on_unusable_channel_still_fails(void) +{ + uint8_t localPub[32], localPriv[32], remotePub[32], remotePriv[32]; + crypto->generateKeyPair(localPub, localPriv); + crypto->generateKeyPair(remotePub, remotePriv); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePub); + memcpy(config.security.private_key.bytes, localPriv, sizeof(localPriv)); + config.security.private_key.size = sizeof(localPriv); + crypto->setDHPrivateKey(localPriv); + + const ChannelIndex deadChannel = 1; + TEST_ASSERT_LESS_THAN(0, channels.getHash(deadChannel)); + + // TRACEROUTE is PKC-excluded like ROUTING, but carries no request_id and is not an ack. + meshtastic_MeshPacket p = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_TRACEROUTE_APP, SMALL_PAYLOAD); + p.channel = deadChannel; + + TEST_ASSERT_EQUAL_MESSAGE(meshtastic_Routing_Error_NO_CHANNEL, perhapsEncode(&p), "the PKC fallback must apply to acks only"); +} + +// A ROUTING packet on a channel that DOES resolve keeps taking the channel path, so relays retain +// the readable acks they use for next-hop learning and retransmission cancel. +void test_B17_ack_on_a_usable_channel_stays_on_the_channel(void) +{ + uint8_t localPub[32], localPriv[32], remotePub[32], remotePriv[32]; + crypto->generateKeyPair(localPub, localPriv); + crypto->generateKeyPair(remotePub, remotePriv); + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->setPublicKey(LOCAL_NODE, localPub); + mockNodeDB->addNode(REMOTE_NODE); + mockNodeDB->setPublicKey(REMOTE_NODE, remotePub); + memcpy(config.security.private_key.bytes, localPriv, sizeof(localPriv)); + config.security.private_key.size = sizeof(localPriv); + crypto->setDHPrivateKey(localPriv); + + meshtastic_MeshPacket ack = makeDecoded(LOCAL_NODE, REMOTE_NODE, meshtastic_PortNum_ROUTING_APP, SMALL_PAYLOAD); + ack.decoded.request_id = 0xFEED5150; + ack.channel = 0; // the primary, which initDefaults() made usable + + TEST_ASSERT_EQUAL(meshtastic_Routing_Error_NONE, perhapsEncode(&ack)); + TEST_ASSERT_FALSE_MESSAGE(ack.pki_encrypted, "a sendable ack must stay readable to relays"); +} + // =========================================================================== // Group C - routing pipeline and NodeInfo authentication ordering // =========================================================================== @@ -2185,6 +2285,10 @@ void setup() RUN_TEST(test_B11_normal_unicast_still_uses_pki); RUN_TEST(test_B12_licensed_receiver_does_not_decrypt_pki); RUN_TEST(test_B13_licensed_port_and_destination_signing_matrix); + RUN_TEST(test_B14_ack_with_no_usable_channel_falls_back_to_pkc); + RUN_TEST(test_B15_ack_with_no_channel_and_no_key_still_fails); + RUN_TEST(test_B16_non_ack_on_unusable_channel_still_fails); + RUN_TEST(test_B17_ack_on_a_usable_channel_stays_on_the_channel); printf("\n=== Group C: routing pipeline authentication ordering ===\n"); RUN_TEST(test_C1_invalid_first_copy_does_not_poison_valid_same_id); From 20f7ab1be9a02c3d399634c7d3d699ef4c2025c9 Mon Sep 17 00:00:00 2001 From: Tom <116762865+NomDeTom@users.noreply.github.com> Date: Sun, 20 Sep 2026 10:29:48 +0000 Subject: [PATCH 7/9] Relay a PKI unicast with a known party in LOCAL_ONLY and KNOWN_ONLY (#11898) * fix(router): relay a PKI unicast with a known party in LOCAL_ONLY and KNOWN_ONLY A frame the relay cannot decrypt takes the OPAQUE_RELAY_ONLY path in Router::perhapsHandleReceived() and returns before handleReceived(), so it never reaches a module. The rebroadcast_mode rule for opaque traffic is therefore the IS_ONE_OF list in relayOpaquePacket(), and LOCAL_ONLY and KNOWN_ONLY were not on it: a node in either mode dropped every opaque frame, including a PKI unicast with a party it knows. The gate in RoutingModule::handleReceivedProtobuf() that used to allow exactly that is unreachable for these packets and no longer decides anything. Add both modes to the list, with the identity rule the unreachable gate carried: a PKI-shaped unicast (channel 0, not broadcast) with `from` or `to` known to us. An unreadable broadcast and a unicast between two strangers stay dropped in these modes, which is what the proto documents - they ignore foreign meshes. This is not only direct messages. Remote administration and key verification are PKI unicasts too, and a KNOWN_ONLY relay was black-holing those between two other nodes just the same. CORE_PORTNUMS_ONLY reached this list the same way in #11843; this is the remaining pair. * test(rebroadcast_mode): pin the relay decision per mode, in both directions What this node carries for other nodes, per DeviceConfig.rebroadcast_mode, for packets it can read and packets it cannot. The harness pushes a real PKI frame through Router::perhapsHandleReceived() and counts what reaches the radio. Both directions are load-bearing, and each is guarded by cases the other leaves green. Measured by rebuilding the firmware three ways: - as shipped: 9/9 pass. - with the two modes taken back out of relayOpaquePacket(): the known-party and remote-admin cases fail, the stranger and foreign-mesh cases still pass. - with the modes listed but the identity qualifier deleted: the stranger and foreign-mesh cases fail, the known-party cases still pass. So a revert and an over-broadening each fail their own tests, and neither can be satisfied by breaking the other. The suite and its harness come from the opaque-packet-handling branch and compile against develop unmodified. Two assertions were dropped because they pin behaviour this branch does not add: delivery of unreadable frames to the phone, and a queued copy of our own suppressing the originator's repeat, which needs relayOpaquePacket() to consult the TX queue. * fix(test): guard the harness include, drop a comment its test outlived Two review findings on the imported suite. The harness builds real PKI frames through CryptoEngine entry points a MESHTASTIC_EXCLUDE_PKI build does not declare, but it was included above the guard, so the empty-suite branch that exists for those builds could not compile. Move the include inside the guard and pull the base includes the stub branch needs above it. The phone-delivery case was dropped when the suite came across - this branch does not deliver unreadable frames to the phone - but its comment stayed behind and now described the test below it, which is about the signature policy. * test(rebroadcast_mode): make the from-known and channel-0 operands load-bearing The qualifier has three operands, and the suite only exercised one of them. Every existing case that a known party carries has a known DESTINATION: the remote-admin case marks both parties, the known-destination case marks the target. Rewriting the identity test to consult p->to alone passed all nine. And the only non-PKI-shaped frame in the suite was a broadcast, which !isBroadcast(p->to) rejects before p->channel is ever read, so deleting the channel gate passed all nine too. Add the two cases that close it: a known SOURCE with a destination we have never heard of, which must relay in both modes, and a known party on a channel hash we do not hold, which must not - that is someone else's channel traffic, addressed, not PKI. Measured both ways. With the from operand and the channel gate removed from relayOpaquePacket(), the two new cases fail and the other nine pass; with the shipping code, 11/11. Raised by CodeRabbit on #11898. --------- Co-authored-by: Ben Meadors --- src/mesh/NextHopRouter.cpp | 13 +- test/state-manifest.tsv | 1 + test/support/AuthPipelineHarness.h | 672 +++++++++++++++++++++++ test/test_rebroadcast_mode/test_main.cpp | 352 ++++++++++++ 4 files changed, 1037 insertions(+), 1 deletion(-) create mode 100644 test/support/AuthPipelineHarness.h create mode 100644 test/test_rebroadcast_mode/test_main.cpp diff --git a/src/mesh/NextHopRouter.cpp b/src/mesh/NextHopRouter.cpp index 1e7ddc9d4..77ebd796d 100644 --- a/src/mesh/NextHopRouter.cpp +++ b/src/mesh/NextHopRouter.cpp @@ -34,10 +34,21 @@ bool NextHopRouter::relayOpaquePacket(const meshtastic_MeshPacket *p) if (!iface || isToUs(p) || isFromUs(p) || p->id == 0 || p->hop_limit == 0 || !isRebroadcaster() || owner.is_licensed || !IS_ONE_OF(mode, meshtastic_Config_DeviceConfig_RebroadcastMode_ALL, meshtastic_Config_DeviceConfig_RebroadcastMode_ALL_SKIP_DECODING, - meshtastic_Config_DeviceConfig_RebroadcastMode_CORE_PORTNUMS_ONLY) || + meshtastic_Config_DeviceConfig_RebroadcastMode_CORE_PORTNUMS_ONLY, + meshtastic_Config_DeviceConfig_RebroadcastMode_LOCAL_ONLY, + meshtastic_Config_DeviceConfig_RebroadcastMode_KNOWN_ONLY) || (p->next_hop != NO_NEXT_HOP_PREFERENCE && p->next_hop != nodeDB->getLastByteOfNodeNum(getNodeNum()))) return false; + // LOCAL_ONLY and KNOWN_ONLY gate on identity, and the only opaque frame carrying its parties in + // the header is a PKI-shaped unicast: relay one just when a party is known, which is the rule + // RoutingModule applied before opaque frames stopped reaching modules. + if (IS_ONE_OF(mode, meshtastic_Config_DeviceConfig_RebroadcastMode_LOCAL_ONLY, + meshtastic_Config_DeviceConfig_RebroadcastMode_KNOWN_ONLY) && + !(p->channel == 0 && !isBroadcast(p->to) && + (nodeInfoLiteHasUser(nodeDB->getMeshNode(p->from)) || nodeInfoLiteHasUser(nodeDB->getMeshNode(p->to))))) + return false; + // Dedup opaque relays. Opaque frames deliberately never enter PacketHistory (so unauthenticated // traffic can't influence routing/ACK/next-hop) - but with NO dedup at all, a dense mesh re-relays // every copy of every frame, multiplying at each hop into an unbounded broadcast storm ("let hop diff --git a/test/state-manifest.tsv b/test/state-manifest.tsv index b37a5e581..e491f5c52 100644 --- a/test/state-manifest.tsv +++ b/test/state-manifest.tsv @@ -70,6 +70,7 @@ test_nodedb_v25_roundtrip writes=config.proto,module.proto,device.proto,channels test_packet_signing writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat errors=300 needs a NodeDB holding both peers' keys for the PKI encode/decode paths test_phone_api_config_dump writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto per-test NodeDB fixture backing full PhoneAPI want_config dumps; the constructor persists a default config/channel/node set in a fresh sandbox test_pki_admin_fallback writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto needs a NodeDB holding admin keys for the fallback paths +test_rebroadcast_mode writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat the ingress harness builds a NodeDB holding the relay peers' keys and our own identity, whose constructor persists a default set when the prefs directory is empty test_reliable_ack_matrix writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto constructs a NodeDB (whose constructor persists a default set when the prefs directory is empty) for the sender-key lookups in the ACK/NAK matrix test_stream_api writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto drives real PhoneAPI handshakes, which read and persist config and the node DB test_traceroute_nexthop writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto traceroute route selection reads the node DB diff --git a/test/support/AuthPipelineHarness.h b/test/support/AuthPipelineHarness.h new file mode 100644 index 000000000..0a3c91529 --- /dev/null +++ b/test/support/AuthPipelineHarness.h @@ -0,0 +1,672 @@ +#pragma once +// Ingress harness shared by the suites that push a packet through Router::perhapsHandleReceived() +// and observe what comes out the other side: a mock NodeDB with controllable keys and bits, a +// counting radio / routing module / module / MQTT, and builders for decoded, channel-encrypted and +// genuinely PKI-encrypted frames. One TU per suite includes this; the statics are per suite. +// +// Lifecycle: pipelineHarnessCreate() once from setup() before UNITY_BEGIN(); pipelineHarnessSetUp() +// at the top of setUp(); pipelineHarnessTearDown() at the top of tearDown(); pipelineHarnessDestroy() +// after UNITY_END(). Suites layer their own defaults (e.g. a signature policy) on top. +#include "MeshTypes.h" // include BEFORE TestUtil.h +#include "NodeStatus.h" +#include "TestUtil.h" +#include "UptimeClock.h" +#include "airtime.h" +#include "mesh/Channels.h" +#include "mesh/CryptoEngine.h" +#include "mesh/MeshRadio.h" +#include "mesh/MeshService.h" +#include "mesh/NodeDB.h" +#include "mesh/ReliableRouter.h" +#include "mesh/Router.h" +#include "mesh/SinglePortModule.h" +#include "modules/RoutingModule.h" +#include "mqtt/MQTT.h" +#include +#include +#include +#include +#include +#include +#include + +// Test fixture identifiers +// --------------------------------------------------------------------------- +static constexpr NodeNum LOCAL_NODE = 0x0A0A0A0A; +static constexpr NodeNum REMOTE_NODE = 0x0B0B0B0B; + +// A "small" broadcast payload whose signed encoding easily fits a LoRa frame, and an "oversized" +// one whose signed encoding does not, yet still encodes within a LoRa frame unsigned. +static constexpr size_t SMALL_PAYLOAD = 16; +static constexpr size_t OVERSIZED_PAYLOAD = 180; + +// --------------------------------------------------------------------------- +// MockNodeDB - inject nodes with controlled public keys / signer bits. +// Mirrors the pattern in test/test_hop_scaling. meshNodes/numMeshNodes are public on NodeDB. +// --------------------------------------------------------------------------- +class MockNodeDB : public NodeDB +{ + public: + void installDefaultsPreservingIdentity() { installDefaultConfig(true); } + + void clearTestNodes() + { + testNodes.clear(); + meshNodes = &testNodes; + numMeshNodes = 0; + } + + // Add a bare node and return a stable handle (fetch via getMeshNode so the pointer stays valid + // even if the vector reallocates after later adds). + void addNode(NodeNum num) + { + meshtastic_NodeInfoLite node = meshtastic_NodeInfoLite_init_zero; + node.num = num; + testNodes.push_back(node); + meshNodes = &testNodes; + numMeshNodes = testNodes.size(); + } + + void setPublicKey(NodeNum num, const uint8_t *pubKey) + { + meshtastic_NodeInfoLite *n = getMeshNode(num); + TEST_ASSERT_NOT_NULL(n); + n->public_key.size = 32; + memcpy(n->public_key.bytes, pubKey, 32); + } + + void setSignerBit(NodeNum num, bool value) + { + meshtastic_NodeInfoLite *n = getMeshNode(num); + TEST_ASSERT_NOT_NULL(n); + nodeInfoLiteSetBit(n, NODEINFO_BITFIELD_HAS_XEDDSA_SIGNED_MASK, value); + } + + void markHasUser(NodeNum num) + { + meshtastic_NodeInfoLite *n = getMeshNode(num); + TEST_ASSERT_NOT_NULL(n); + nodeInfoLiteSetBit(n, NODEINFO_BITFIELD_HAS_USER_MASK, true); + } + + /// A node with a User whose licensed flag is `licensed`; getLicenseStatus() then says so. + void markLicenseStatus(NodeNum num, bool licensed) + { + markHasUser(num); + nodeInfoLiteSetBit(getMeshNode(num), NODEINFO_BITFIELD_IS_LICENSED_MASK, licensed); + } + + void setLongName(NodeNum num, const char *name) + { + meshtastic_NodeInfoLite *n = getMeshNode(num); + TEST_ASSERT_NOT_NULL(n); + strncpy(n->long_name, name, sizeof(n->long_name) - 1); + n->long_name[sizeof(n->long_name) - 1] = '\0'; + } + + const char *longName(NodeNum num) + { + meshtastic_NodeInfoLite *n = getMeshNode(num); + TEST_ASSERT_NOT_NULL(n); + return n->long_name; + } + + std::vector testNodes; +}; + +static MockNodeDB *mockNodeDB = nullptr; + +/// Stands in for the radio: records what was sent instead of transmitting it, and models the TX +/// queue the router consults. `failSend` simulates a full queue. +class AuthPipelineRadio : public RadioInterface +{ + public: + ErrorCode send(meshtastic_MeshPacket *p) override + { + sendCalls++; + sent.push_back(*p); + packetPool.release(p); + return failSend ? ERRNO_DISABLED : ERRNO_OK; + } + bool cancelSending(NodeNum from, PacketId id) override + { + cancelCalls++; + releaseFromTxQueue(from, id); + return true; + } + bool findInTxQueue(NodeNum from, PacketId id) override + { + findCalls++; + return txQueueHolds(from, id); + } + bool removePendingTXPacket(NodeNum from, PacketId id, uint32_t) override + { + removeCalls++; + releaseFromTxQueue(from, id); + return true; + } + uint32_t getPacketTime(uint32_t, bool = false) override { return 7; } + + // The TX queue the router asks about. A test loads it to say "this one has not gone out yet"; + // cancelSending() and removePendingTXPacket() take entries back out, as the real queue does. + void holdInTxQueue(NodeNum from, PacketId id) + { + if (!txQueueHolds(from, id)) + txQueue.push_back({from, id}); + } + void releaseFromTxQueue(NodeNum from, PacketId id) + { + for (size_t i = 0; i < txQueue.size(); i++) { + if (txQueue[i].from == from && txQueue[i].id == id) { + txQueue.erase(txQueue.begin() + i); + return; + } + } + } + bool txQueueHolds(NodeNum from, PacketId id) const + { + for (const auto &e : txQueue) + if (e.from == from && e.id == id) + return true; + return false; + } + size_t txQueueSize() const { return txQueue.size(); } + + // What actually went out, in order, so a test can assert on the frame and not just the count. + const std::vector &sentPackets() const { return sent; } + const meshtastic_MeshPacket *lastSent() const { return sent.empty() ? nullptr : &sent.back(); } + /// How many of the sends carried this (from,id) - one relay of a frame heard twice, say. + uint32_t sentCountFor(NodeNum from, PacketId id) const + { + uint32_t n = 0; + for (const auto &p : sent) + if (getFrom(&p) == from && p.id == id) + n++; + return n; + } + + void reset() + { + sendCalls = cancelCalls = findCalls = removeCalls = 0; + failSend = false; + sent.clear(); + txQueue.clear(); + } + + bool failSend = false; + uint32_t sendCalls = 0; + uint32_t cancelCalls = 0; + uint32_t findCalls = 0; + uint32_t removeCalls = 0; + + private: + struct TxQueueEntry { + NodeNum from; + PacketId id; + }; + std::vector txQueue; + std::vector sent; +}; + +/// The production router with its protected state (history, pending retransmissions, upgrades) exposed. +class AuthPipelineRouter : public ReliableRouter +{ + public: + bool filter(meshtastic_MeshPacket *p) { return ReliableRouter::shouldFilterReceived(p); } + bool historyContains(const meshtastic_MeshPacket *p) { return wasSeenRecently(p, false); } + void remember(const meshtastic_MeshPacket *p) { wasSeenRecently(p, true); } + void forgetRelayer(uint8_t relay, PacketId id, NodeNum from) { removeRelayer(relay, id, from); } + bool handleUpgrade(meshtastic_MeshPacket *p) { return perhapsHandleUpgradedPacket(p); } + void addPending(const meshtastic_MeshPacket &p, uint32_t nextTx) + { + auto *copy = packetPool.allocCopy(p); + TEST_ASSERT_NOT_NULL(copy); + const GlobalPacketId key(copy); + pending.emplace(key, PendingPacket(copy, NUM_INTERMEDIATE_RETX)); + pending.at(key).nextTxMsec = nextTx; + } + uint32_t pendingNextTx(NodeNum from, PacketId id) + { + PendingPacket *entry = findPendingPacket(from, id); + return entry ? entry->nextTxMsec : 0; + } + uint8_t pendingTotalAttempts(NodeNum from, PacketId id) + { + PendingPacket *entry = findPendingPacket(from, id); + return entry ? entry->initialNumRetransmissions + 1 : 0; + } + size_t pendingCount() const { return pending.size(); } + /// Forget every opaque (from,id): the router outlives a test, so the dedup ring must not. + void clearOpaqueSeen() + { + for (auto &slot : opaqueSeen) { + slot.sender = 0; + slot.id = 0; + } + opaqueSeenNext = 0; + } + void clearPending() + { + for (auto &entry : pending) + packetPool.release(entry.second.packet); + pending.clear(); + } +}; + +/// Counts every ACK/NAK the router asks for instead of transmitting it. The opaque path sends none. +class AuthPipelineRoutingModule : public RoutingModule +{ + public: + void sendAckNak(meshtastic_Routing_Error, NodeNum, PacketId, ChannelIndex, uint8_t = 0, bool = false, + const meshtastic_MeshPacket * = nullptr) override + { + ackCalls++; + } + void reset() { ackCalls = 0; } + + uint32_t ackCalls = 0; +}; + +/// A POSITION_APP module that counts how often ingress reaches module dispatch. +class AuthPipelineModule : public SinglePortModule +{ + public: + AuthPipelineModule() : SinglePortModule("authPipeline", meshtastic_PortNum_POSITION_APP) {} + ProcessMessage handleReceived(const meshtastic_MeshPacket &) override + { + calls++; + return ProcessMessage::CONTINUE; + } + uint32_t calls = 0; +}; + +/// Exposes the uplink queue so a test can count what would have been published. +class AuthPipelineMqtt : public MQTT +{ + public: + int queueSize() { return mqttQueue.numUsed(); } + /// Fill the offline queue with decoded-traffic stand-ins, oldest first, so an eviction is visible. + void fillQueue() + { + for (int i = 0; mqttQueue.numFree() > 0; i++) { + auto *entry = new QueueEntry; + entry->topic = "sentinel/" + std::to_string(i); + mqttQueue.enqueue(entry, 0); + } + } + std::string oldestTopic() + { + QueueEntry *entry = mqttQueue.dequeuePtr(0); + if (!entry) + return ""; + std::string topic = entry->topic; + delete entry; + return topic; + } + void clearQueue() + { + while (QueueEntry *entry = mqttQueue.dequeuePtr(0)) + delete entry; + } +}; + +static AuthPipelineRouter *pipelineRouter = nullptr; +static AuthPipelineRadio *pipelineRadio = nullptr; +static AuthPipelineRoutingModule *pipelineRouting = nullptr; +static AuthPipelineModule *pipelineModule = nullptr; +static AuthPipelineMqtt *pipelineMqtt = nullptr; +static MeshService *pipelineService = nullptr; + +// --------------------------------------------------------------------------- +// Packet builders +// --------------------------------------------------------------------------- + +// Build a decoded packet with a deterministic payload of the requested size. +static meshtastic_MeshPacket makeDecoded(NodeNum from, NodeNum to, meshtastic_PortNum port, size_t payloadLen) +{ + meshtastic_MeshPacket p = meshtastic_MeshPacket_init_zero; + p.from = from; + p.to = to; + p.id = 0x12345678; + p.channel = 0; // primary channel index (perhapsEncode rewrites this to the channel hash) + p.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + p.decoded.portnum = port; + p.decoded.payload.size = payloadLen; + for (size_t i = 0; i < payloadLen; i++) + p.decoded.payload.bytes[i] = (uint8_t)(i & 0xff); + return p; +} + +// Sign a decoded packet with the CryptoEngine's current key - used to simulate a *remote* signer, +// because perhapsEncode only auto-signs packets that originate from us. +static meshtastic_MeshPacket channelEncode(meshtastic_MeshPacket p) +{ + uint8_t encoded[MAX_LORA_PAYLOAD_LEN + 1] = {}; + const size_t encodedSize = pb_encode_to_bytes(encoded, sizeof(encoded), &meshtastic_Data_msg, &p.decoded); + TEST_ASSERT_GREATER_THAN(0, encodedSize); + const int16_t hash = channels.setActiveByIndex(p.channel); + TEST_ASSERT_GREATER_OR_EQUAL(0, hash); + crypto->encryptPacket(p.from, p.id, encodedSize, encoded); + memcpy(p.encrypted.bytes, encoded, encodedSize); + p.encrypted.size = encodedSize; + p.channel = hash; + p.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; + return p; +} + +/// Set the receive-side signature policy for the next ingress. +static void setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy policy) +{ + config.security.packet_signature_policy = policy; +} + +/// Push a copy of `p` through Router::perhapsHandleReceived() as radio ingress. +static void runPipelineIngress(const meshtastic_MeshPacket &p) +{ + meshtastic_MeshPacket *copy = packetPool.allocCopy(p); + TEST_ASSERT_NOT_NULL(copy); + pipelineRouter->enqueueReceivedMessage(copy); + pipelineRouter->runOnce(); +} + +// --------------------------------------------------------------------------- +// PKI fixtures: two remote identities and frames encrypted between them +// --------------------------------------------------------------------------- + +/// A remote node the tests can encrypt from and to. +struct RelayIdentity { + NodeNum num; + uint8_t pub[32]; + uint8_t priv[32]; +}; + +// CryptoEngine::setDHPrivateKey takes a mutable pointer; the identities above are const. +static void useDHKey(const uint8_t *priv) +{ + uint8_t k[32]; + memcpy(k, priv, sizeof(k)); + crypto->setDHPrivateKey(k); +} + +/// A remote node with a fresh Curve25519 keypair. +static RelayIdentity makeIdentity(NodeNum num) +{ + RelayIdentity id; + id.num = num; + crypto->generateKeyPair(id.pub, id.priv); + return id; +} + +static constexpr NodeNum ADMIN_NODE = 0x0C0C0C0C; // the operator's node, sending remote admin +static constexpr NodeNum TARGET_NODE = 0x0D0D0D0D; // the node being administered + +/// A genuine PKI-encrypted packet on `port` from one remote identity to another, as it would be +/// heard off the air by a third node (us). The engine is left holding whatever DH key it held on +/// entry, so a caller may install its own key before or after building the frame. +static meshtastic_MeshPacket makePkiUnicastBetween(const RelayIdentity &from, const RelayIdentity &to, meshtastic_PortNum port, + PacketId id, bool wantAck = false) +{ + meshtastic_Data d = meshtastic_Data_init_zero; + d.portnum = port; + d.payload.size = SMALL_PAYLOAD; + for (size_t i = 0; i < SMALL_PAYLOAD; i++) + d.payload.bytes[i] = (uint8_t)(0xA0 + i); + uint8_t plain[MAX_LORA_PAYLOAD_LEN + 1]; + const size_t plainSize = pb_encode_to_bytes(plain, sizeof(plain), &meshtastic_Data_msg, &d); + TEST_ASSERT_GREATER_THAN(0, plainSize); + + meshtastic_MeshPacket p = meshtastic_MeshPacket_init_zero; + p.from = from.num; + p.to = to.num; + p.id = id; + p.channel = 0; // PKI packets carry channel hash 0 on the wire + p.hop_limit = 2; + p.hop_start = 3; + p.want_ack = wantAck; + p.transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA; + p.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; + + meshtastic_NodeInfoLite_public_key_t toKey = {32, {0}}; + memcpy(toKey.bytes, to.pub, 32); + // Borrow the sender's key for the encrypt only. private_key is public under PIO_UNIT_TESTING. + uint8_t savedPriv[32]; + memcpy(savedPriv, crypto->private_key, sizeof(savedPriv)); + useDHKey(from.priv); + TEST_ASSERT_TRUE(crypto->encryptCurve25519(p.to, p.from, toKey, p.id, plainSize, plain, p.encrypted.bytes)); + p.encrypted.size = plainSize + MESHTASTIC_PKC_OVERHEAD; + crypto->setDHPrivateKey(savedPriv); + return p; +} + +/// Every rebroadcast_mode, for cases that table the full matrix. +static const meshtastic_Config_DeviceConfig_RebroadcastMode ALL_MODES[] = { + meshtastic_Config_DeviceConfig_RebroadcastMode_ALL, + meshtastic_Config_DeviceConfig_RebroadcastMode_ALL_SKIP_DECODING, + meshtastic_Config_DeviceConfig_RebroadcastMode_CORE_PORTNUMS_ONLY, + meshtastic_Config_DeviceConfig_RebroadcastMode_KNOWN_ONLY, + meshtastic_Config_DeviceConfig_RebroadcastMode_LOCAL_ONLY, + meshtastic_Config_DeviceConfig_RebroadcastMode_NONE, +}; + +/// Mode name for assertion messages. +static const char *modeName(meshtastic_Config_DeviceConfig_RebroadcastMode m) +{ + switch (m) { + case meshtastic_Config_DeviceConfig_RebroadcastMode_ALL: + return "ALL"; + case meshtastic_Config_DeviceConfig_RebroadcastMode_ALL_SKIP_DECODING: + return "ALL_SKIP_DECODING"; + case meshtastic_Config_DeviceConfig_RebroadcastMode_CORE_PORTNUMS_ONLY: + return "CORE_PORTNUMS_ONLY"; + case meshtastic_Config_DeviceConfig_RebroadcastMode_KNOWN_ONLY: + return "KNOWN_ONLY"; + case meshtastic_Config_DeviceConfig_RebroadcastMode_LOCAL_ONLY: + return "LOCAL_ONLY"; + default: + return "NONE"; + } +} + +/// Empty the phone queue, returning how many frames it held. +static int drainPhoneQueue() +{ + int seen = 0; + while (meshtastic_MeshPacket *queued = pipelineService->getForPhone()) { + packetPool.release(queued); + seen++; + } + return seen; +} + +/// Drain the phone queue, asserting exactly `n` frames arrived and every one is still ciphertext. +static void expectEncryptedPhoneDeliveries(int n) +{ + int seen = 0; + while (meshtastic_MeshPacket *queued = pipelineService->getForPhone()) { + TEST_ASSERT_EQUAL(meshtastic_MeshPacket_encrypted_tag, queued->which_payload_variant); + TEST_ASSERT_EQUAL_MESSAGE(0, queued->channel, "an unreadable frame carries no channel information to the phone"); + packetPool.release(queued); + seen++; + } + TEST_ASSERT_EQUAL_MESSAGE(n, seen, "encrypted frames delivered to the phone"); +} + +/// Feed `p` through ingress under `mode` and assert it was, or was not, handed to the radio - and +/// that nothing else happened to it either way. +static void assertOpaqueRelay(const meshtastic_MeshPacket &p, meshtastic_Config_DeviceConfig_RebroadcastMode mode, + bool expectRelay, const char *why) +{ + pipelineRadio->reset(); + pipelineRouting->reset(); + pipelineModule->calls = 0; + config.device.rebroadcast_mode = mode; + meshtastic_MeshPacket copy = p; + copy.id += (uint32_t)mode; // a fresh id per mode so the opaque dedup does not decide the outcome + runPipelineIngress(copy); + char msg[200]; + snprintf(msg, sizeof(msg), "%s: %s", modeName(mode), why); + TEST_ASSERT_EQUAL_MESSAGE(expectRelay ? 1 : 0, pipelineRadio->sendCalls, msg); + TEST_ASSERT_EQUAL_MESSAGE(0, pipelineRouting->ackCalls, "an opaque packet not for us must never be ACKed"); + TEST_ASSERT_EQUAL_MESSAGE(0, pipelineModule->calls, "an opaque packet must not reach modules"); + TEST_ASSERT_NULL_MESSAGE(pipelineService->getForPhone(), "an opaque unicast for someone else is not the phone's business"); + TEST_ASSERT_FALSE_MESSAGE(pipelineRouter->historyContains(©), "an opaque packet must not enter PacketHistory"); +} + +/// A broadcast on a channel hash no local channel produces: opaque to us. +static meshtastic_MeshPacket makeUnknownChannelBroadcast(PacketId id) +{ + meshtastic_MeshPacket foreign = meshtastic_MeshPacket_init_zero; + foreign.from = ADMIN_NODE; + foreign.to = NODENUM_BROADCAST; + foreign.id = id; + foreign.channel = 0xFE; + foreign.hop_limit = 1; + foreign.hop_start = 2; + foreign.which_payload_variant = meshtastic_MeshPacket_encrypted_tag; + foreign.encrypted.size = 16; + memset(foreign.encrypted.bytes, 0x5A, foreign.encrypted.size); + return foreign; +} + +/// Give the local node a keypair in NodeDB so PKI frames addressed to it are decrypt candidates. +static RelayIdentity installOurIdentity() +{ + RelayIdentity us = makeIdentity(LOCAL_NODE); + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->setPublicKey(LOCAL_NODE, us.pub); + // NodeDB always holds a User for our own node, and the rebroadcast_mode predicates read that bit. + mockNodeDB->markHasUser(LOCAL_NODE); + return us; +} + +/// Put this node into licensed mode the way the device does: `owner` and our own NodeDB record agree, +/// so getLicenseStatus(us) says Licensed rather than NotLicensed. +static void markOurselvesLicensed() +{ + owner.is_licensed = true; + if (!mockNodeDB->getMeshNode(LOCAL_NODE)) + mockNodeDB->addNode(LOCAL_NODE); + mockNodeDB->markLicenseStatus(LOCAL_NODE, true); +} + +/// A decodable, unsigned channel broadcast from `from` to `to`, as a plain-text relay would see it. +static meshtastic_MeshPacket makeChannelBroadcastFrom(NodeNum from, NodeNum to, PacketId id) +{ + meshtastic_MeshPacket p = makeDecoded(from, to, meshtastic_PortNum_TEXT_MESSAGE_APP, SMALL_PAYLOAD); + p.id = id; + p.hop_limit = 1; + p.hop_start = 2; + p.transport_mechanism = meshtastic_MeshPacket_TransportMechanism_TRANSPORT_LORA; + return channelEncode(p); +} + +/// The second copy of `p` a neighbour would put back on the air: same (from,id), one hop spent. +static meshtastic_MeshPacket makeRelayedCopy(const meshtastic_MeshPacket &p) +{ + meshtastic_MeshPacket copy = p; + if (copy.hop_limit > 0) + copy.hop_limit--; + copy.relay_node = 0x42; + return copy; +} + +// --------------------------------------------------------------------------- +// Lifecycle +// --------------------------------------------------------------------------- + +static AirTime *harnessSavedAirTime = nullptr; +static meshtastic::NodeStatus *harnessSavedNodeStatus = nullptr; + +/// Build the router / radio / module / service / MQTT stack once per process. +static void pipelineHarnessCreate() +{ + initializeTestEnvironment(); + harnessSavedAirTime = airTime; + harnessSavedNodeStatus = nodeStatus; + airTime = new AirTime(); + nodeStatus = new meshtastic::NodeStatus(); + + config.lora.region = meshtastic_Config_LoRaConfig_RegionCode_US; + initRegion(); + pipelineRouter = new AuthPipelineRouter(); + auto pipelineRadioOwner = std::make_unique(); + pipelineRadio = pipelineRadioOwner.get(); + pipelineRouter->addInterface(std::move(pipelineRadioOwner)); + router = pipelineRouter; + routingModule = pipelineRouting = new AuthPipelineRoutingModule(); + pipelineModule = new AuthPipelineModule(); + service = pipelineService = new MeshService(); + mqtt = pipelineMqtt = new AuthPipelineMqtt(); +} + +/// Free the AirTime and NodeStatus the harness installed, then put back the originals. +static void pipelineHarnessDestroy() +{ + delete airTime; + delete nodeStatus; + airTime = harnessSavedAirTime; + nodeStatus = harnessSavedNodeStatus; +} + +/// Fresh NodeDB, zeroed config / owner (=> rebroadcast ALL, no private key), default channels, +/// every counter reset. The signature policy is left at zero (COMPATIBLE); suites set their own. +static void pipelineHarnessSetUp() +{ + service = pipelineService; + + // Construct the mock FIRST: the NodeDB constructor can reload persisted state from the + // host filesystem (portduino VFS) and repopulate the globals - a saved private key + // re-enables the PKI encrypt path and fails the unicast tests on hosts with leftover prefs. + mockNodeDB = new MockNodeDB(); + mockNodeDB->clearTestNodes(); +#if WARM_NODE_COUNT > 0 + mockNodeDB->warmStore.clear(); +#endif + nodeDB = mockNodeDB; + + // Clean global config/owner AFTER the ctor; zeroed config => rebroadcast ALL (no KNOWN_ONLY + // drop) and security.private_key.size == 0 (PKI encrypt path skipped => simple channel crypto). + config = meshtastic_LocalConfig_init_zero; + moduleConfig = meshtastic_LocalModuleConfig_init_zero; + owner = meshtastic_User_init_zero; + myNodeInfo.my_node_num = LOCAL_NODE; // drives isFromUs()/getFrom()/isToUs() + + // Working primary channel with the default PSK so encrypt/decrypt round-trips. + channels.initDefaults(); + channels.onConfigChanged(); + + // The router, radio, routing module and crypto engine are built once per process, so every + // piece of state they carry has to be cleared here or it decides the next test's outcome. + pipelineRouter->clearPending(); + pipelineRouter->clearOpaqueSeen(); + pipelineRouter->rxDupe = 0; + pipelineRouter->txRelayCanceled = 0; + pipelineRadio->reset(); + pipelineRouting->reset(); + pipelineModule->calls = 0; + pipelineMqtt->clearQueue(); + drainPhoneQueue(); + while (meshtastic_QueueStatus *queued = pipelineService->getQueueStatusForPhone()) + pipelineService->releaseQueueStatusToPool(queued); + resetRoutingAuthEvaluationCount(); // also invalidates the single-slot auth cache +#if !(MESHTASTIC_EXCLUDE_PKI) + // No DH key and no in-flight handshake key: a test that wants either installs its own. + uint8_t noKey[32] = {0}; + crypto->setDHPrivateKey(noKey); + crypto->clearPendingPublicKey(); + resetAdminKeyFallbackBudget(); // a suite that drains the bucket must not starve the next one +#endif +} + +/// Drop the NodeDB and put the clock and region back. Runs here, not at the end of a test body: +/// an assertion aborts the body, and these would otherwise leak into every later case. +static void pipelineHarnessTearDown() +{ + delete mockNodeDB; + mockNodeDB = nullptr; + nodeDB = nullptr; + Time::useRealClock(); + Time::resetMonotonicForTests(); + config.lora.region = meshtastic_Config_LoRaConfig_RegionCode_US; + initRegion(); +} diff --git a/test/test_rebroadcast_mode/test_main.cpp b/test/test_rebroadcast_mode/test_main.cpp new file mode 100644 index 000000000..ad8231eef --- /dev/null +++ b/test/test_rebroadcast_mode/test_main.cpp @@ -0,0 +1,352 @@ +// What this node carries for other nodes, per DeviceConfig.rebroadcast_mode - the one knob that +// governs relaying, for packets it can read and for packets it cannot (a PKI unicast between two +// other nodes, a channel it does not hold). The signature policy plays no part here and is varied +// across the cases to prove it. Each expected outcome is a contract, and the assertion messages +// say what breaks if it is not met. +// +// opaque, per mode ALL / ALL_SKIP_DECODING / CORE_PORTNUMS_ONLY carry; KNOWN_ONLY and LOCAL_ONLY +// carry a PKI-shaped unicast with one known party, and nothing else - not a +// unicast between two strangers, not an unreadable broadcast; NONE carries +// nothing. Both directions are load-bearing: dropping the modes from +// relayOpaquePacket()'s list fails the known-party cases, and dropping the +// identity/PKI-shape qualifier fails the stranger and foreign-mesh cases. +// licensed node never carries ciphertext; carries plaintext unless a party is known unlicensed +// header gates hop_limit 0, id 0, someone else's next_hop, CLIENT_MUTE each stop a relay +// +// Every case builds a real PKI frame, so the suite runs only where PKI is compiled in. + +#include "MeshTypes.h" // BEFORE TestUtil.h +#include "TestUtil.h" +#include + +#if !(MESHTASTIC_EXCLUDE_PKI) && !(MESHTASTIC_EXCLUDE_XEDDSA) + +// Inside the guard: the harness builds real PKI frames through CryptoEngine entry points that a +// PKI-excluded build does not declare. +#include "support/AuthPipelineHarness.h" + +void setUp(void) +{ + pipelineHarnessSetUp(); +} + +void tearDown(void) +{ + pipelineHarnessTearDown(); +} + +// Remote admin from the operator's node to a node behind us, both of them known to us. This is +// what a router in the field does all day. Anyone who makes this assertion fail in any mode but +// NONE is turning a stock ROUTER (whose default is CORE_PORTNUMS_ONLY) into a black hole for +// remote administration, direct messages and key verification, and owes an explanation for it. +void test_remote_admin_between_other_nodes_relays_in_every_mode(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT); + const RelayIdentity admin = makeIdentity(ADMIN_NODE); + const RelayIdentity target = makeIdentity(TARGET_NODE); + mockNodeDB->addNode(ADMIN_NODE); + mockNodeDB->setPublicKey(ADMIN_NODE, admin.pub); + mockNodeDB->markHasUser(ADMIN_NODE); + mockNodeDB->addNode(TARGET_NODE); + mockNodeDB->setPublicKey(TARGET_NODE, target.pub); + mockNodeDB->markHasUser(TARGET_NODE); + const meshtastic_MeshPacket adminPacket = + makePkiUnicastBetween(admin, target, meshtastic_PortNum_ADMIN_APP, 0xADA10001, /*wantAck=*/true); + + // Sanity: the frame really is opaque to us, not merely undecodable. + meshtastic_MeshPacket probe = adminPacket; + TEST_ASSERT_EQUAL(static_cast(RoutingAuthVerdict::OPAQUE_RELAY_ONLY), static_cast(passesRoutingAuthGate(&probe))); + + for (const auto mode : ALL_MODES) { + const bool expect = mode != meshtastic_Config_DeviceConfig_RebroadcastMode_NONE; + assertOpaqueRelay(adminPacket, mode, expect, + expect ? "a relay must carry remote admin it cannot read - justify any change to this" + : "NONE relays nothing"); + } + // NodeDB is untouched by all of it: no last_heard, no new entries. + TEST_ASSERT_EQUAL(0, mockNodeDB->getMeshNode(ADMIN_NODE)->last_heard); +} + +// The same admin packet between two nodes we have never heard of. Modes that key on identity +// (KNOWN_ONLY, LOCAL_ONLY) decline; the port-based and unconditional modes still carry it. +void test_pki_unicast_between_strangers_relays_unless_mode_needs_identity(void) +{ + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_COMPATIBLE); + const RelayIdentity admin = makeIdentity(ADMIN_NODE); + const RelayIdentity target = makeIdentity(TARGET_NODE); + const meshtastic_MeshPacket p = makePkiUnicastBetween(admin, target, meshtastic_PortNum_ADMIN_APP, 0xADA20002); + + for (const auto mode : ALL_MODES) { + const bool needsIdentity = IS_ONE_OF(mode, meshtastic_Config_DeviceConfig_RebroadcastMode_KNOWN_ONLY, + meshtastic_Config_DeviceConfig_RebroadcastMode_LOCAL_ONLY); + const bool expect = mode != meshtastic_Config_DeviceConfig_RebroadcastMode_NONE && !needsIdentity; + assertOpaqueRelay(p, mode, expect, + expect ? "a PKI unicast between strangers is still carried by a port- or unconditional-mode relay" + : "this mode only carries PKI traffic with a party we know"); + } +} + +// One party known is enough for KNOWN_ONLY / LOCAL_ONLY. The known party is the destination here, +// so the sender is a stranger and KNOWN_ONLY's decode short-circuit fires; the packet must still +// reach the relay decision. +void test_known_destination_satisfies_known_only_and_local_only(void) +{ + const RelayIdentity admin = makeIdentity(ADMIN_NODE); + const RelayIdentity target = makeIdentity(TARGET_NODE); + mockNodeDB->addNode(TARGET_NODE); + mockNodeDB->setPublicKey(TARGET_NODE, target.pub); + mockNodeDB->markHasUser(TARGET_NODE); + const meshtastic_MeshPacket p = makePkiUnicastBetween(admin, target, meshtastic_PortNum_TEXT_MESSAGE_APP, 0xADA30003); + + assertOpaqueRelay(p, meshtastic_Config_DeviceConfig_RebroadcastMode_KNOWN_ONLY, true, + "KNOWN_ONLY carries a PKI unicast to a node we know, whoever sent it"); + assertOpaqueRelay(p, meshtastic_Config_DeviceConfig_RebroadcastMode_LOCAL_ONLY, true, + "LOCAL_ONLY carries a PKI unicast to a node we know, whoever sent it"); +} + +// The mirror of the case above, and the only one that makes the `from` half of the identity test do +// any work: a known SENDER, a destination we have never heard of. A qualifier rewritten to consult +// only p->to passes every other case in this suite and fails this one. +void test_known_source_satisfies_known_only_and_local_only(void) +{ + const RelayIdentity admin = makeIdentity(ADMIN_NODE); + const RelayIdentity target = makeIdentity(TARGET_NODE); + mockNodeDB->addNode(ADMIN_NODE); + mockNodeDB->setPublicKey(ADMIN_NODE, admin.pub); + mockNodeDB->markHasUser(ADMIN_NODE); // the sender is known; the destination is not in the DB at all + const meshtastic_MeshPacket p = makePkiUnicastBetween(admin, target, meshtastic_PortNum_TEXT_MESSAGE_APP, 0xADA30013); + + assertOpaqueRelay(p, meshtastic_Config_DeviceConfig_RebroadcastMode_KNOWN_ONLY, true, + "KNOWN_ONLY carries a PKI unicast from a node we know, whoever it is addressed to"); + assertOpaqueRelay(p, meshtastic_Config_DeviceConfig_RebroadcastMode_LOCAL_ONLY, true, + "LOCAL_ONLY carries a PKI unicast from a node we know, whoever it is addressed to"); +} + +// The only case that makes the channel-0 half do any work. A unicast on a channel hash we do not +// hold is not PKI - it is someone else's channel traffic, addressed - so these modes decline it even +// though a party is known. The broadcast case cannot pin this: !isBroadcast() already rejects that. +void test_known_party_on_a_foreign_channel_is_declined(void) +{ + const RelayIdentity admin = makeIdentity(ADMIN_NODE); + const RelayIdentity target = makeIdentity(TARGET_NODE); + mockNodeDB->addNode(TARGET_NODE); + mockNodeDB->setPublicKey(TARGET_NODE, target.pub); + mockNodeDB->markHasUser(TARGET_NODE); + meshtastic_MeshPacket p = makePkiUnicastBetween(admin, target, meshtastic_PortNum_TEXT_MESSAGE_APP, 0xADA30023); + p.channel = 0x2B; // a channel hash we do not hold, so nothing about this frame says PKI + + assertOpaqueRelay(p, meshtastic_Config_DeviceConfig_RebroadcastMode_KNOWN_ONLY, false, + "KNOWN_ONLY declines an addressed frame on a foreign channel, known party or not"); + assertOpaqueRelay(p, meshtastic_Config_DeviceConfig_RebroadcastMode_LOCAL_ONLY, false, + "LOCAL_ONLY declines an addressed frame on a foreign channel, known party or not"); +} + +// An opaque *broadcast* (a channel we do not hold) is not PKI-shaped. CORE relays it - the +// port list cannot apply to a packet with no readable port - and only KNOWN/LOCAL/NONE decline. +void test_unknown_channel_broadcast_relays_in_core_portnums_only(void) +{ + const meshtastic_MeshPacket foreign = makeUnknownChannelBroadcast(0xADA40004); + for (const auto mode : ALL_MODES) { + const bool expect = IS_ONE_OF(mode, meshtastic_Config_DeviceConfig_RebroadcastMode_ALL, + meshtastic_Config_DeviceConfig_RebroadcastMode_ALL_SKIP_DECODING, + meshtastic_Config_DeviceConfig_RebroadcastMode_CORE_PORTNUMS_ONLY); + pipelineRadio->reset(); + config.device.rebroadcast_mode = mode; + meshtastic_MeshPacket copy = foreign; + copy.id += (uint32_t)mode; + runPipelineIngress(copy); + while (meshtastic_MeshPacket *queued = pipelineService->getForPhone()) // phone delivery: test_packet_signing C22 + packetPool.release(queued); + char msg[120]; + snprintf(msg, sizeof(msg), "%s: %s", modeName(mode), + expect ? "an unreadable broadcast is carried" : "an unreadable broadcast is not PKI-shaped, so declined"); + TEST_ASSERT_EQUAL_MESSAGE(expect ? 1 : 0, pipelineRadio->sendCalls, msg); + TEST_ASSERT_FALSE(pipelineRouter->historyContains(©)); + } +} + +// A licensed (ham) node must not relay traffic it cannot read - encryption is not permitted +// on its band, and it cannot tell what it is carrying. Every mode, no exceptions; its own inbound +// handling (phone delivery) is unaffected because that is about what was sent *to* it. +void test_licensed_node_never_relays_opaque_traffic(void) +{ + owner.is_licensed = true; + const RelayIdentity admin = makeIdentity(ADMIN_NODE); + const RelayIdentity target = makeIdentity(TARGET_NODE); + mockNodeDB->addNode(ADMIN_NODE); + mockNodeDB->markLicenseStatus(ADMIN_NODE, true); + mockNodeDB->addNode(TARGET_NODE); + mockNodeDB->markLicenseStatus(TARGET_NODE, true); + const meshtastic_MeshPacket p = makePkiUnicastBetween(admin, target, meshtastic_PortNum_ADMIN_APP, 0xADAB000B); + for (const auto mode : ALL_MODES) + assertOpaqueRelay(p, mode, false, "a licensed node does not carry ciphertext, whoever the parties are"); + + const meshtastic_MeshPacket foreign = makeUnknownChannelBroadcast(0xADAB001B); + for (const auto mode : ALL_MODES) { + pipelineRadio->reset(); + config.device.rebroadcast_mode = mode; + meshtastic_MeshPacket copy = foreign; + copy.id += (uint32_t)mode; + runPipelineIngress(copy); + drainPhoneQueue(); + TEST_ASSERT_EQUAL_MESSAGE(0, pipelineRadio->sendCalls, "a licensed node does not carry an unreadable broadcast either"); + } +} + +// What a licensed node does with traffic it *can* read: it will not relay for a party it knows to +// be unlicensed, in either direction; a licensed or unknown party is carried (RoutingModule). +void test_licensed_node_relays_decoded_unless_a_party_is_known_unlicensed(void) +{ + owner.is_licensed = true; + setPolicy(meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_COMPATIBLE); + constexpr NodeNum LICENSED_PEER = 0x0E0E0E0E, UNLICENSED_PEER = 0x0F0F0F0F, UNKNOWN_PEER = 0x01010101; + mockNodeDB->addNode(LICENSED_PEER); + mockNodeDB->markLicenseStatus(LICENSED_PEER, true); + mockNodeDB->addNode(UNLICENSED_PEER); + mockNodeDB->markLicenseStatus(UNLICENSED_PEER, false); + + struct Case { + NodeNum from, to; + bool relayed; + const char *why; + } cases[] = { + {LICENSED_PEER, NODENUM_BROADCAST, true, "broadcast from a licensed peer is carried"}, + {UNKNOWN_PEER, NODENUM_BROADCAST, true, "broadcast from a peer of unknown status is carried"}, + {UNLICENSED_PEER, NODENUM_BROADCAST, false, "broadcast from a known-unlicensed peer is not carried"}, + {LICENSED_PEER, UNLICENSED_PEER, false, "unicast to a known-unlicensed peer is not carried"}, + {LICENSED_PEER, UNKNOWN_PEER, true, "unicast to a peer of unknown status is carried"}, + }; + uint32_t n = 0; + for (const auto &c : cases) { + pipelineRadio->reset(); + pipelineRouting->reset(); + const meshtastic_MeshPacket p = makeChannelBroadcastFrom(c.from, c.to, 0xADAC0100 + ++n); + runPipelineIngress(p); + drainPhoneQueue(); + TEST_ASSERT_EQUAL_MESSAGE(c.relayed ? 1 : 0, pipelineRadio->sendCalls, c.why); + } +} + +// The header gates on the opaque path that never changed and must not: a spent hop budget, an +// id of 0, a next_hop naming someone else, and the CLIENT_MUTE role each stop a relay on their own. +void test_opaque_relay_header_gates_hold(void) +{ + const RelayIdentity admin = makeIdentity(ADMIN_NODE); + const RelayIdentity target = makeIdentity(TARGET_NODE); + const meshtastic_MeshPacket base = makePkiUnicastBetween(admin, target, meshtastic_PortNum_ADMIN_APP, 0xADAD000D); + config.device.rebroadcast_mode = meshtastic_Config_DeviceConfig_RebroadcastMode_ALL; + + meshtastic_MeshPacket spent = base; + spent.hop_limit = 0; + runPipelineIngress(spent); + TEST_ASSERT_EQUAL_MESSAGE(0, pipelineRadio->sendCalls, "hop_limit 0: nothing left to spend"); + + pipelineRadio->reset(); + meshtastic_MeshPacket noId = base; + noId.id = 0; + runPipelineIngress(noId); + TEST_ASSERT_EQUAL_MESSAGE(0, pipelineRadio->sendCalls, "id 0 cannot be deduplicated, so it is never relayed"); + + pipelineRadio->reset(); + meshtastic_MeshPacket notOurHop = base; + notOurHop.id++; + notOurHop.next_hop = (uint8_t)(nodeDB->getLastByteOfNodeNum(LOCAL_NODE) ^ 0x01); + runPipelineIngress(notOurHop); + TEST_ASSERT_EQUAL_MESSAGE(0, pipelineRadio->sendCalls, "next_hop names another relay"); + + pipelineRadio->reset(); + meshtastic_MeshPacket ourHop = base; + ourHop.id += 2; + ourHop.next_hop = nodeDB->getLastByteOfNodeNum(LOCAL_NODE); + runPipelineIngress(ourHop); + TEST_ASSERT_EQUAL_MESSAGE(1, pipelineRadio->sendCalls, "next_hop names us"); + + pipelineRadio->reset(); + config.device.role = meshtastic_Config_DeviceConfig_Role_CLIENT_MUTE; + meshtastic_MeshPacket muted = base; + muted.id += 3; + runPipelineIngress(muted); + TEST_ASSERT_EQUAL_MESSAGE(0, pipelineRadio->sendCalls, "CLIENT_MUTE relays nothing, opaque included"); +} + +// Hearing the same frame again is not a reason to carry it again - except from the originator, which +// only retransmits because it heard no rebroadcast of ours, and not even then while ours is queued. +void test_opaque_relay_carries_a_frame_once_unless_the_originator_repeats_it(void) +{ + const RelayIdentity admin = makeIdentity(ADMIN_NODE); + const RelayIdentity target = makeIdentity(TARGET_NODE); + const meshtastic_MeshPacket base = makePkiUnicastBetween(admin, target, meshtastic_PortNum_ADMIN_APP, 0xADAE000E); + config.device.rebroadcast_mode = meshtastic_Config_DeviceConfig_RebroadcastMode_ALL; + + runPipelineIngress(base); + runPipelineIngress(makeRelayedCopy(base)); + TEST_ASSERT_EQUAL_MESSAGE(1, pipelineRadio->sentCountFor(ADMIN_NODE, base.id), + "a neighbour's rebroadcast of a frame we carried is not carried again"); + + meshtastic_MeshPacket retx = base; + retx.hop_limit = retx.hop_start; + runPipelineIngress(retx); + TEST_ASSERT_EQUAL_MESSAGE(2, pipelineRadio->sentCountFor(ADMIN_NODE, base.id), + "the originator's own retransmission is carried again"); + + // A third case - our own queued copy suppressing the repeat - needs relayOpaquePacket() to + // consult the TX queue, which it does not do. Not this change's to add. +} + +// None of the above depends on packet_signature_policy: the remote-admin case ran STRICT, the +// strangers case COMPATIBLE. Here the same admin packet goes under every policy in turn. +void test_relay_decision_ignores_signature_policy(void) +{ + const RelayIdentity admin = makeIdentity(ADMIN_NODE); + const RelayIdentity target = makeIdentity(TARGET_NODE); + const meshtastic_MeshPacket p = makePkiUnicastBetween(admin, target, meshtastic_PortNum_ADMIN_APP, 0xADAA000A); + const meshtastic_Config_SecurityConfig_PacketSignaturePolicy policies[] = { + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_COMPATIBLE, + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_BALANCED, + meshtastic_Config_SecurityConfig_PacketSignaturePolicy_PACKET_SIGNATURE_POLICY_STRICT, + }; + uint32_t salt = 0; + for (const auto policy : policies) { + setPolicy(policy); + meshtastic_MeshPacket copy = p; + copy.id += 0x100 * ++salt; + assertOpaqueRelay(copy, meshtastic_Config_DeviceConfig_RebroadcastMode_CORE_PORTNUMS_ONLY, true, + "the signature policy governs what we admit, not what we carry"); + } +} + +void setup() +{ + pipelineHarnessCreate(); + UNITY_BEGIN(); + RUN_TEST(test_remote_admin_between_other_nodes_relays_in_every_mode); + RUN_TEST(test_pki_unicast_between_strangers_relays_unless_mode_needs_identity); + RUN_TEST(test_known_destination_satisfies_known_only_and_local_only); + RUN_TEST(test_known_source_satisfies_known_only_and_local_only); + RUN_TEST(test_known_party_on_a_foreign_channel_is_declined); + RUN_TEST(test_unknown_channel_broadcast_relays_in_core_portnums_only); + RUN_TEST(test_licensed_node_never_relays_opaque_traffic); + RUN_TEST(test_licensed_node_relays_decoded_unless_a_party_is_known_unlicensed); + RUN_TEST(test_opaque_relay_header_gates_hold); + RUN_TEST(test_opaque_relay_carries_a_frame_once_unless_the_originator_repeats_it); + RUN_TEST(test_relay_decision_ignores_signature_policy); + const int result = UNITY_END(); + pipelineHarnessDestroy(); + exit(result); +} + +void loop() {} + +#else // XEdDSA or PKI excluded + +void setUp(void) {} +void tearDown(void) {} +void setup() +{ + initializeTestEnvironment(); + UNITY_BEGIN(); + exit(UNITY_END()); +} +void loop() {} + +#endif From 332c4d7c6f29f067e61de42ceaf53ab6b11efa5d Mon Sep 17 00:00:00 2001 From: Tom <116762865+NomDeTom@users.noreply.github.com> Date: Sun, 20 Sep 2026 10:32:41 +0000 Subject: [PATCH 8/9] Narrow the ad-hoc NodeInfo greeting (#11897) * feat(nodedb): greet only while the node store is under half full The ad-hoc greeting in MeshService::handleFromRadio() was gated on !isFull(), so a node kept sending unsolicited NodeInfo right up to the last free slot - on a dense mesh that is the regime where the store is already churning and the greeting is least likely to buy a lasting entry. Add NodeDB::isHalfEmpty(), true only when strictly more than half the slots are free, and gate the greeting on it instead. The comparison is written as 2 * numMeshNodes < cap so a half-full store reads false with no integer rounding, and MAX_NUM_NODES is read into a local because portduino resolves it through a runtime call. The helper keeps the MINIMUM_SAFE_FREE_HEAP term that !isFull() used to contribute: low heap disqualifies the store regardless of occupancy, so a sparse database on a memory-starved device still does not transmit. Admission is untouched - updateFrom() and getOrCreateMeshNode() still fill to capacity. Only greeting stops early. * fix(nodeinfo): raise the minimum greeting window to 30 minutes The !shorterTimeout branch of NodeInfoModule::allocReply() used a 10-minute base, so a node that had just greeted one neighbour could greet the next ten minutes later. Raise the base to 30 minutes. This is the floor, not the window: getConfiguredOrDefaultMsScaled() still multiplies by the congestion coefficient for the roles that scale, so a busy mesh stretches it further. ROUTER/ROUTER_LATE and the tracker/sensor roles bypass the scaling and get a flat 30 minutes. The interactive paths are unaffected - they pass shorterTimeout and keep their own 60-second gate. The periodic broadcast is unaffected too: default_node_info_broadcast_secs is 3 hours with a 1-hour minimum, both clear of the new floor, so the timer is not swallowed by the throttle. * fix(nodeinfo): a send restarts the routine broadcast countdown sendOurNodeInfo() left the OSThread schedule alone, so an ad-hoc send had no effect on the periodic broadcast: run() anchors the next run at runned() + interval, and nothing re-anchored it when the send came from a greeting, a PKI decrypt failure or a completed key verification. The routine copy could follow minutes behind an ad-hoc one, putting two NodeInfos on the air for no gain. Call setIntervalFromNow() with the configured broadcast interval once the packet is queued, so the next periodic copy is a full interval from the send rather than from the last tick. It sits on the return-true path only: a send vetoed by allocReply() - throttle, airtime ceiling, reply suppression - must not be able to silence the routine broadcast. Calling it from inside runOnce() is harmless, since run() then applies the same interval from a last_run of effectively now. * test(nodeinfo): cover the send window, the countdown reset and the greeting gate Three behaviours from this branch had no coverage: isHalfEmpty()'s exclusive boundary, the 30-minute send floor, and the countdown reset on a send. isHalfEmpty() goes to test_nodedb_blocked, which already owns the full-store cases and clears the hot store per test. Three tests sweep the cap over the sizes real deployments have - portduino resolves MAX_NUM_NODES from General.MaxNodes on every read, so a predicate that cached it would greet at the wrong occupancy - and pin the band where admission outlives greeting. That suite had no tearDown; it has one now, restoring the cap so an assertion firing mid-sweep cannot leak a 2-node cap into the tests after it. test_nodeinfo_send_window is new because nothing in the tree stands up NodeInfoModule's send path. Six tests: the floor at 30 minutes with 10 refused, the interactive 60-second gate staying separate, the countdown re-armed by a broadcast and by an ad-hoc unicast, left alone by a refused send, and a preset change consumed only by a send that goes out. The scaling above 40 online nodes is deliberately not retested here - getConfiguredOrDefaultMsScaled() is test_default's contract, per preset and per role. These tests pin the base and leave the multiplier alone. NodeInfoModule gains two PIO_UNIT_TESTING accessors for the countdown: concurrency::OSThread is a private base, so a test shim cannot reach it and only the class itself can. They compile out of a shipping build. The heap term in isHalfEmpty()/isFull() stays uncovered: memGet.getFreeHeap() returns UINT32_MAX on portduino, so a native test could only pin a stub. * chore(trunk): exempt test_nodedb_blocked from the trufflehog Lob detector test_removeNodeByNum_presentNodeOnFullDb is exactly 35 characters after the test_ prefix, which is the length of a Lob API key, and trufflehog's detector matches the bare identifier. The name is years old; it surfaces now only because this branch touches the file, and the pre-push gate reports a finding in a changed file as new. Added to the ignore block that already carries the same detector's hex-literal false positives, with the reason stated alongside them. Nothing in that file is a credential. * fix(nodeinfo): exempt a licensed station from the floor, delay only on a real send Two review findings on the 30-minute window. Ham mode sets node_info_broadcast_secs to 600 s for the FCC minimum call-sign announcement (AdminModule.cpp). The new floor refused every one of those sends until 30 minutes had passed, so a licensed station's call sign went out three times less often than the regulation asks - a regression the old 10-minute base did not have. A licensed station now keeps its own interval whenever that is shorter than the floor. The exemption is exactly the licensed case because nothing else can get under the floor: a set-config clamps the field to an hour, and the userprefs path clamps identically. sendOurNodeInfo() ignored what sendToMesh() returned, so a packet the router declined - no interface, queue full - still re-armed the routine broadcast and still reported success, which let runOnce() consume a pending channel change for a send that never reached the air. Only ERRNO_OK and ERRNO_SHOULD_RELEASE now count; sendToMesh() has already released the packet in both cases. Both are pinned by tests that fail without them, measured: the licensed case fails at "11 min is past it, and the floor must not override it", the declined send at "a declined send is not a send". The licensed test carries an unlicensed control on the same configuration, so deleting the floor outright would not satisfy it. * test(nodeinfo): assert the deadline the scheduler reads, from an aged last_run The countdown cases asserted Thread::interval, which is not what schedules the next run: shouldRun() keys off _cached_next_run, and the two ways of writing it differ. setIntervalFromNow() recomputes it from now; Thread::setInterval() recomputes it from last_run. Swap the call in sendOurNodeInfo() for the latter and the period still reads three hours while the deadline lands wherever the last tick was - firing the routine copy right behind an ad-hoc send, the exact thing the reset exists to prevent. Every test passed. Assert the deadline instead, from a fixture where the two answers are distinguishable: ageLastRunForTests() calls Thread::runned() with an hour-old timestamp, the state a periodic thread is genuinely in between runs, so a deadline off last_run lands an hour early against a five second tolerance. Measured: with setInterval() in place of setIntervalFromNow(), the new case fails by 3600004 ms and the eight others pass, including the one asserting the period - which is what says the old assertion could not see this. runned() and _cached_next_run are protected in Thread and OSThread is a private base, so the hooks live on NodeInfoModule, with the two already there. Raised by Copilot on #11897. * fix(nodeinfo): a declined send must not start the throttle window either allocReply() stamped TransmitHistory when it built the packet, before anything had been sent. The previous commit made sendOurNodeInfo() report a router rejection instead of swallowing it, but the stamp was already written by then, so a packet that never reached the air still started the window - and with the floor now at 30 minutes, that silences the node for half an hour over a send that failed. allocReply() has two callers and only one of them can see the outcome: the module framework sends its own reply through currentReply, with no post-send hook a module can reach (MeshModule::sendResponse is not virtual). So the stamp stays there for that path, and sendOurNodeInfo() defers it across its own allocReply() call and stamps once the router has accepted the packet. deferHistoryStamp mirrors the shorterTimeout member alongside it - same call-scoped signal, same lifetime. test_sendWindow_aRejectedSendDoesNotStartTheWindow asserts both halves: no stamp after the rejection, and the retry immediately after goes out. The existing rejected-send case checked the first failure and the countdown only, which is how this survived it. 248/248 across every suite that touches NodeInfoModule (admin_session_repro, admin_radio, nodeinfo_send_window, traffic_management, fuzz_packets) plus transmit_history, whose subject this is. Raised by CodeRabbit on #11897. --- .trunk/trunk.yaml | 7 + src/mesh/MeshService.cpp | 2 +- src/mesh/NodeDB.cpp | 8 + src/mesh/NodeDB.h | 3 + src/modules/NodeInfoModule.cpp | 34 +- src/modules/NodeInfoModule.h | 14 + test/state-manifest.tsv | 1 + test/test_nodedb_blocked/test_main.cpp | 121 +++++- test/test_nodeinfo_send_window/test_main.cpp | 380 +++++++++++++++++++ 9 files changed, 564 insertions(+), 6 deletions(-) create mode 100644 test/test_nodeinfo_send_window/test_main.cpp diff --git a/.trunk/trunk.yaml b/.trunk/trunk.yaml index 588e9bb7f..34f1f3f20 100644 --- a/.trunk/trunk.yaml +++ b/.trunk/trunk.yaml @@ -183,10 +183,17 @@ lint: # numbers and request ids (0x0A0A0A0A, 0x0B0B0B0B, 0xABCD1234); its key # material is generated at runtime by crypto->generateKeyPair, so there is # no literal key in the file to leak. + # + # test_nodedb_blocked trips it on an identifier rather than on hex: one of + # its removeNodeByNum cases has a name exactly 35 characters long after the + # test_ prefix, which is the Lob key length, so the detector matches the + # bare word. Nothing in that file is a credential. Do not quote the name in + # full anywhere under lint - a comment containing it trips the same rule. - linters: [trufflehog] paths: - test/test_ack_proof/test_main.cpp - test/test_airtime/test_main.cpp + - test/test_nodedb_blocked/test_main.cpp - test/test_throttle/test_main.cpp - test/test_uptime_clock/test_main.cpp # The nightly index templates are halves of one page, not whole documents: diff --git a/src/mesh/MeshService.cpp b/src/mesh/MeshService.cpp index e3803c43a..9d7b71b5d 100644 --- a/src/mesh/MeshService.cpp +++ b/src/mesh/MeshService.cpp @@ -100,7 +100,7 @@ int MeshService::handleFromRadio(const meshtastic_MeshPacket *mp) // ignore our request for its NodeInfo } else if (mp->which_payload_variant == meshtastic_MeshPacket_decoded_tag && !nodeInfoLiteHasUser(nodeDB->getMeshNode(mp->from)) && nodeInfoModule && !isPreferredRebroadcaster && - !nodeDB->isFull()) { + nodeDB->isHalfEmpty()) { if (airTime->isTxAllowedChannelUtil(true)) { const int8_t hopsUsed = getHopsAway(*mp, config.lora.hop_limit); if (hopsUsed > (int32_t)(config.lora.hop_limit + 2)) { diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index 66f09dba6..b08d5bd84 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -4225,6 +4225,14 @@ bool NodeDB::isFull() return (numMeshNodes >= MAX_NUM_NODES) || (memGet.getFreeHeap() < MINIMUM_SAFE_FREE_HEAP); } +bool NodeDB::isHalfEmpty() const +{ + // MAX_NUM_NODES is a runtime call on portduino, so read it once. Strictly more than half the + // slots must be free, and low heap disqualifies the store just as it does in isFull(). + const size_t cap = (size_t)MAX_NUM_NODES; + return ((size_t)numMeshNodes * 2 < cap) && (memGet.getFreeHeap() >= MINIMUM_SAFE_FREE_HEAP); +} + uint32_t NodeDB::hotNodeLastHeard(NodeNum n) const { for (int i = 0; i < numMeshNodes; i++) diff --git a/src/mesh/NodeDB.h b/src/mesh/NodeDB.h index d36fd7ca0..c65bd6def 100644 --- a/src/mesh/NodeDB.h +++ b/src/mesh/NodeDB.h @@ -604,6 +604,9 @@ class NodeDB // returns true if the maximum number of nodes is reached or we are running low on memory bool isFull(); + // returns true only if more than half the node slots are still empty (and memory is not tight) + bool isHalfEmpty() const; + void clearLocalPosition(); void setLocalPosition(meshtastic_Position position, bool timeOnly = false) diff --git a/src/modules/NodeInfoModule.cpp b/src/modules/NodeInfoModule.cpp index 57a7540c4..644437788 100644 --- a/src/modules/NodeInfoModule.cpp +++ b/src/modules/NodeInfoModule.cpp @@ -101,9 +101,11 @@ bool NodeInfoModule::sendOurNodeInfo(NodeNum dest, bool wantReplies, uint8_t cha if (prevPacketId) // if we wrap around to zero, we'll simply fail to cancel in that rare case (no big deal) service->cancelSending(prevPacketId); shorterTimeout = _shorterTimeout; + deferHistoryStamp = true; DEBUG_HEAP_BEFORE; meshtastic_MeshPacket *p = allocReply(); DEBUG_HEAP_AFTER("NodeInfoModule::sendOurNodeInfo", p); + deferHistoryStamp = false; if (p) { // Check whether we didn't ignore it p->to = dest; @@ -123,8 +125,20 @@ bool NodeInfoModule::sendOurNodeInfo(NodeNum dest, bool wantReplies, uint8_t cha prevPacketId = p->id; - service->sendToMesh(p); + const ErrorCode res = service->sendToMesh(p); shorterTimeout = false; + // A rejected send never reached the air, so it neither defers the routine broadcast nor + // consumes a pending channel change. sendToMesh() has already released the packet. + if (res != ERRNO_OK && res != ERRNO_SHOULD_RELEASE) { + LOG_WARN("NodeInfo send rejected (err=%d)", res); + return false; + } + if (transmitHistory) + transmitHistory->setLastSentToMesh(meshtastic_PortNum_NODEINFO_APP); + // Our NodeInfo just went on the air, so the routine broadcast is due a full interval from now + // rather than from the last tick - an ad-hoc send otherwise leaves the periodic copy right behind it. + setIntervalFromNow( + Default::getConfiguredOrDefaultMs(config.device.node_info_broadcast_secs, default_node_info_broadcast_secs)); return true; } return false; @@ -157,8 +171,17 @@ meshtastic_MeshPacket *NodeInfoModule::allocReply() return NULL; } - // Use graduated scaling based on active mesh size (10 minute base, scales with congestion coefficient) - uint32_t timeoutMs = Default::getConfiguredOrDefaultMsScaled(0, 10 * 60, nodeStatus->getNumOnline()); + // Use graduated scaling based on active mesh size (30 minute base, scales with congestion coefficient) + uint32_t timeoutMs = Default::getConfiguredOrDefaultMsScaled(0, 30 * 60, nodeStatus->getNumOnline()); + // A licensed station's call-sign announcement is a regulatory interval, not a preference: ham mode + // sets node_info_broadcast_secs to 600 s, which a set-config would otherwise clamp to an hour. + // Never hold such a station past its own interval, whatever the floor and the scaling say. + if (owner.is_licensed) { + const uint32_t hamMs = + Default::getConfiguredOrDefaultMs(config.device.node_info_broadcast_secs, default_node_info_broadcast_secs); + if (hamMs < timeoutMs) + timeoutMs = hamMs; + } uint32_t lastNodeInfo = transmitHistory ? transmitHistory->getLastSentToMeshMillis(meshtastic_PortNum_NODEINFO_APP) : 0; if (!shorterTimeout && lastNodeInfo && Throttle::isWithinTimespanMs(lastNodeInfo, timeoutMs)) { LOG_DEBUG("Skip send NodeInfo since we sent it <%us ago", timeoutMs / 1000); @@ -180,7 +203,10 @@ meshtastic_MeshPacket *NodeInfoModule::allocReply() strcpy(u.id, nodeDB->getNodeId().c_str()); LOG_INFO("Send owner %s/%s/%s", u.id, u.long_name, u.short_name); - if (transmitHistory) + // The framework sends its own reply, so stamp here for that path. sendOurNodeInfo() stamps + // after the router accepts the packet instead - a send that never went out must not throttle + // the next one, and the floor it would sit out is 30 minutes. + if (transmitHistory && !deferHistoryStamp) transmitHistory->setLastSentToMesh(meshtastic_PortNum_NODEINFO_APP); return allocDataProtobuf(u); } diff --git a/src/modules/NodeInfoModule.h b/src/modules/NodeInfoModule.h index 6cdb8caa7..1c0933d00 100644 --- a/src/modules/NodeInfoModule.h +++ b/src/modules/NodeInfoModule.h @@ -30,6 +30,18 @@ class NodeInfoModule : public ProtobufModule, private concurren */ void triggerImmediateNodeInfoCheck(); +#ifdef PIO_UNIT_TESTING + /// Test-only reads of the routine-broadcast countdown a send re-arms. concurrency::OSThread is a + /// private base, so only this class can reach it - a test shim cannot. + unsigned long broadcastCountdownMsForTests() const { return interval; } + void armBroadcastCountdownForTests(unsigned long ms) { setIntervalFromNow(ms); } + /// The deadline the scheduler actually reads. interval alone cannot tell a deadline moved to + /// now from one recomputed off a stale last_run, which is the regression worth catching. + unsigned long broadcastDeadlineMsForTests() const { return _cached_next_run; } + /// Pretend the periodic thread last ran ageMs ago, so those two deadlines differ by ageMs. + void ageLastRunForTests(unsigned long ageMs) { runned(millis() - ageMs); } +#endif + protected: /** Called to handle a particular incoming message @@ -49,6 +61,8 @@ class NodeInfoModule : public ProtobufModule, private concurren private: bool shorterTimeout = false; + /// Set across sendOurNodeInfo()'s own allocReply(), so the transmit stamp waits for an accepted send. + bool deferHistoryStamp = false; bool suppressReplyForCurrentRequest = false; /// Sender -> uptime seconds (Time::getUptimeSecs()) at our last reply. Seconds, not millis: /// the suppression window is hours wide. See handleReceivedProtobuf(). diff --git a/test/state-manifest.tsv b/test/state-manifest.tsv index e491f5c52..5ad16db39 100644 --- a/test/state-manifest.tsv +++ b/test/state-manifest.tsv @@ -67,6 +67,7 @@ test_nodedb_legacy_migration writes=config.proto,module.proto,device.proto,chann test_nodedb_lora_slot writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto constructs a NodeDB, whose constructor persists a default set when the prefs directory is empty; the tests themselves only mutate config.lora in RAM and restore it in tearDown test_nodedb_save_retry writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto constructs a NodeDB (persists the default set on an empty prefs dir), then drives saveToDisk(SEGMENT_CONFIG) through the retry gate and rewrites config.proto in place to pin the no-format contract test_nodedb_v25_roundtrip writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat v25 persistence round-trips: every test saves nodes.proto and cold-boots a NodeDB whose constructor persists the default segments; warm.dat on the node-DB save cadence +test_nodeinfo_send_window writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,transmit_history.dat constructs a NodeDB for getNodeId()/updateFrom() on our own sends; every window probe stamps TransmitHistory, which persists the NodeInfo send time test_packet_signing writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat errors=300 needs a NodeDB holding both peers' keys for the PKI encode/decode paths test_phone_api_config_dump writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto per-test NodeDB fixture backing full PhoneAPI want_config dumps; the constructor persists a default config/channel/node set in a fresh sandbox test_pki_admin_fallback writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto needs a NodeDB holding admin keys for the fallback paths diff --git a/test/test_nodedb_blocked/test_main.cpp b/test/test_nodedb_blocked/test_main.cpp index 3825ed5ee..d2b7b72a3 100644 --- a/test/test_nodedb_blocked/test_main.cpp +++ b/test/test_nodedb_blocked/test_main.cpp @@ -14,6 +14,10 @@ #if WARM_NODE_COUNT > 0 #include "mesh/NodeDB.h" +#if defined(ARCH_PORTDUINO) +#include "platform/portduino/PortduinoGlue.h" +#endif +#include #include // Subclass shim: exposes the private maintenance paths (via the friend @@ -66,12 +70,17 @@ class NodeDBTestShim : public NodeDB // Index 0 is our own node; the eviction/migration scans treat it as self. void seedSelf() { push(0x0BADF00D, 0xFFFFFFFFu, false, false, /*withUser=*/true, /*withKey=*/false); } + + // isHalfEmpty() and isFull() read numMeshNodes against MAX_NUM_NODES and nothing else, so the + // occupancy tests set the count directly rather than allocating rows at every cap under test. + void setOccupancy(int n) { numMeshNodes = (pb_size_t)n; } }; namespace { NodeDBTestShim *db = nullptr; +int savedMaxNodes = 0; bool warmHasKey(NodeNum n) { @@ -84,8 +93,18 @@ bool warmHasKey(NodeNum n) void setUp(void) { db->clearHot(); +#if defined(ARCH_PORTDUINO) + savedMaxNodes = portduino_config.MaxNodes; +#endif +} +void tearDown(void) +{ +#if defined(ARCH_PORTDUINO) + // The occupancy sweeps below move the cap. Restore it here rather than at the end of each + // test, so an assertion that fires mid-sweep cannot leak a 2-node cap into the next test. + portduino_config.MaxNodes = savedMaxNodes; +#endif } -void tearDown(void) {} // Migration: a database from a larger-cap build trims to MAX_NUM_NODES; the // oldest non-protected nodes are demoted into the warm tier (keys preserved), @@ -318,6 +337,101 @@ static void test_removeNodeByNum_presentNodeOnFullDb(void) TEST_ASSERT_NOT_NULL(db->getMeshNode(8000 + MAX_NUM_NODES - 1)); // survivors kept } +#if defined(ARCH_PORTDUINO) +// NodeDB::isHalfEmpty() and the band it opens against isFull(). The ad-hoc greeting in +// MeshService::handleFromRadio() reads it before sending an unsolicited NodeInfo to a node it holds +// no user record for: greeting now stops at the half-way mark while admission continues to the cap, +// so there is a deliberate occupancy band in which the store still takes new nodes but no longer +// introduces itself to them. Before this the gate was !isFull(), and a node kept greeting up to the +// last free slot - the regime where the store is already churning and the entry a greeting buys is +// least likely to survive. +// +// Sweeping the cap matters because it is not a constant: on portduino MAX_NUM_NODES resolves to +// General.MaxNodes on every read, and a predicate that captured it once - a static, a value copied +// in the constructor - would greet at the wrong occupancy on every deployment that sets one. +// +// Not covered: the MINIMUM_SAFE_FREE_HEAP term both predicates carry. memGet.getFreeHeap() returns +// UINT32_MAX on portduino, so that branch is unreachable natively and is not faked. + +// The caps a real deployment has - STM32WL's 10, the nRF52840/ESP32 120, portduino/ESP32-S3 200 and +// 250 - plus odd caps, and 2 where an off-by-one stops being one slot and becomes the upper half. +static constexpr int kCaps[] = {2, 3, 10, 11, 120, 121, 200, 250}; + +static const char *occ(int cap, int n) +{ + static char buf[64]; + snprintf(buf, sizeof(buf), "cap=%d occupancy=%d", cap, n); + return buf; +} + +// Strictly more than half the slots must be free: exactly half full is not half empty, and at an +// odd cap the unsplittable slot counts as empty (2n < cap). Relaxing this to >= hands greeting one +// more slot at every cap. +static void test_halfEmpty_boundaryIsExclusiveAtEveryCap(void) +{ + for (int cap : kCaps) { + portduino_config.MaxNodes = cap; + TEST_ASSERT_EQUAL_INT_MESSAGE(cap, (int)MAX_NUM_NODES, "MAX_NUM_NODES must track General.MaxNodes at runtime"); + + const int halfWay = cap / 2; + + db->setOccupancy(0); + TEST_ASSERT_TRUE_MESSAGE(db->isHalfEmpty(), occ(cap, 0)); // a fresh node must greet + + db->setOccupancy(halfWay); + TEST_ASSERT_EQUAL_MESSAGE(2 * halfWay < cap, db->isHalfEmpty(), occ(cap, halfWay)); + + db->setOccupancy(halfWay + 1); + TEST_ASSERT_FALSE_MESSAGE(db->isHalfEmpty(), occ(cap, halfWay + 1)); + + db->setOccupancy(cap); + TEST_ASSERT_FALSE_MESSAGE(db->isHalfEmpty(), occ(cap, cap)); + TEST_ASSERT_TRUE_MESSAGE(db->isFull(), occ(cap, cap)); + } +} + +// The band is the point of the change: above the half-way mark and below the cap, admission +// continues (!isFull) while greeting has stopped (!isHalfEmpty). The two are never both true. If +// either predicate drifts the band closes, and greeting either runs to the last slot again or stops +// when admission does. +static void test_halfEmpty_theBandWhereAdmissionOutlivesGreeting(void) +{ + for (int cap : kCaps) { + portduino_config.MaxNodes = cap; + + int bandWidth = 0; + for (int n = 0; n <= cap; n++) { + db->setOccupancy(n); + TEST_ASSERT_FALSE_MESSAGE(db->isHalfEmpty() && db->isFull(), occ(cap, n)); + if (n < cap && !db->isHalfEmpty()) { + TEST_ASSERT_FALSE_MESSAGE(db->isFull(), occ(cap, n)); + bandWidth++; + } + } + TEST_ASSERT_EQUAL_INT_MESSAGE(cap - (cap / 2 + (cap % 2)), bandWidth, occ(cap, -1)); + } +} + +// The same occupancy changes answer when the cap moves underneath it, with no store change - what a +// General.MaxNodes edit plus a restart does, and what a cached cap gets wrong. +static void test_halfEmpty_followsACapChangedUnderneathIt(void) +{ + db->setOccupancy(70); + + portduino_config.MaxNodes = 120; + TEST_ASSERT_FALSE_MESSAGE(db->isHalfEmpty(), "70 of 120 is past the half-way mark"); + + portduino_config.MaxNodes = 200; + TEST_ASSERT_TRUE_MESSAGE(db->isHalfEmpty(), "70 of 200 leaves more than half free"); + + portduino_config.MaxNodes = 140; + TEST_ASSERT_FALSE_MESSAGE(db->isHalfEmpty(), "70 of 140 is exactly half full, which is not half empty"); + + portduino_config.MaxNodes = 141; + TEST_ASSERT_TRUE_MESSAGE(db->isHalfEmpty(), "70 of 141 leaves the spare slot free, so more than half"); +} +#endif // ARCH_PORTDUINO + NDB_TEST_ENTRY void setup() { initializeTestEnvironment(); @@ -335,6 +449,11 @@ NDB_TEST_ENTRY void setup() RUN_TEST(test_protectedCap_refusesBeyondLimit); RUN_TEST(test_removeNodeByNum_absentNodeOnFullDb); RUN_TEST(test_removeNodeByNum_presentNodeOnFullDb); +#if defined(ARCH_PORTDUINO) + RUN_TEST(test_halfEmpty_boundaryIsExclusiveAtEveryCap); + RUN_TEST(test_halfEmpty_theBandWhereAdmissionOutlivesGreeting); + RUN_TEST(test_halfEmpty_followsACapChangedUnderneathIt); +#endif exit(UNITY_END()); } NDB_TEST_ENTRY void loop() {} diff --git a/test/test_nodeinfo_send_window/test_main.cpp b/test/test_nodeinfo_send_window/test_main.cpp new file mode 100644 index 000000000..4a8b63751 --- /dev/null +++ b/test/test_nodeinfo_send_window/test_main.cpp @@ -0,0 +1,380 @@ +// NodeInfoModule's send window and the routine-broadcast countdown: allocReply(), +// sendOurNodeInfo() and runOnce() in src/modules/NodeInfoModule.cpp. +// +// Two contracts, both properties of the module rather than of the scaler it calls: +// +// 1. The non-interactive window has a 30 minute floor. allocReply() passes 30 * 60 as the base to +// Default::getConfiguredOrDefaultMsScaled(); what that base becomes at mesh sizes over 40 nodes, +// per modem preset and per role, is Default's own contract and is covered in test_default - +// these tests pin the base and leave the multiplier alone. The interactive path (shorterTimeout, +// used by a user-triggered send, a PKI decrypt failure and a completed key verification) keeps +// its separate 60 second gate and must not inherit the floor. +// +// 2. A send that goes out re-arms the routine broadcast, and a send that is refused does not. +// sendOurNodeInfo() calls setIntervalFromNow() with the configured broadcast interval once the +// packet is queued, so an ad-hoc send is not followed minutes later by the periodic copy. The +// reset sits on the return-true path deliberately: if a refused send could re-arm it, a node +// that keeps attempting greetings inside the window would defer its broadcast indefinitely and +// go silent - the opposite of the intent. +// +// A preset or channel change (radioGeneration) rides on the same path: runOnce() asks for replies +// while currentGeneration != radioGeneration and copies the generation across only on a true +// return, so a refused send has to leave the request pending for the next attempt. +// +// Regressions guarded: reverting the base to 10 * 60, the value this branch replaced; hoisting the +// setIntervalFromNow() call above the veto checks or onto the false path; and moving the generation +// copy out of `if (sendOurNodeInfo(...))`, which loses a preset change to a throttled send so the +// mesh is never asked to re-introduce itself. +// +// The window probes step an injected clock (Time::setTestMillis) rather than sleeping; +// TransmitHistory, which is where allocReply() reads "last sent" from, reads the same clock. +#include "MeshTypes.h" // BEFORE TestUtil.h +#include "TestUtil.h" +#include + +#if defined(ARCH_PORTDUINO) +#define NI_TEST_ENTRY extern "C" +#else +#define NI_TEST_ENTRY +#endif + +#include "Default.h" +#include "NodeStatus.h" +#include "UptimeClock.h" +#include "airtime.h" +#include "mesh/NodeDB.h" +#include "mesh/RadioInterface.h" +#include "mesh/Router.h" +#include "mesh/TransmitHistory.h" +#include "modules/NodeInfoModule.h" +#include "support/MockMeshService.h" +#include +#include + +// Exposes the protected periodic entry point. The countdown itself is read through NodeInfoModule's +// own ForTests accessors: OSThread is a private base, so a shim cannot reach it. Reading the interval +// rather than a deadline keeps these assertions off wall-clock millis(), which the test clock does not drive. +class NodeInfoModuleTestShim : public NodeInfoModule +{ + public: + using NodeInfoModule::runOnce; +}; + +namespace +{ + +// sendLocal() is not virtual and refuses to send with no interface attached, so the mock router +// carries a stub one. Only send() and getPacketTime() are pure virtual, and init() - which is what +// observes config and sleep notifications - is never called. +class StubRadioInterface : public RadioInterface +{ + public: + ErrorCode send(meshtastic_MeshPacket *p) override + { + packetPool.release(p); + return ERRNO_OK; + } + uint32_t getPacketTime(uint32_t totalPacketLen, bool received = false) override { return 100; } +}; + +class MockRouter : public Router +{ + public: + MockRouter() { addInterface(std::unique_ptr(new StubRadioInterface())); } + + // Router's constructor asserts cryptLock is null before allocating it, so a per-test router can + // only be rebuilt if the previous one hands the global back. + ~MockRouter() + { + delete cryptLock; + cryptLock = nullptr; + } + + ErrorCode send(meshtastic_MeshPacket *p) override + { + sentPackets.push_back(*p); + packetPool.release(p); // released here either way: the interface owns the packet it declined + return sendResult; + } + + ErrorCode sendResult = ERRNO_OK; + + // The broadcast loopback copy lands here; release rather than queue into fromRadioQueue, which + // nothing drains in tests. + void enqueueReceivedMessage(meshtastic_MeshPacket *p) override { packetPool.release(p); } + + std::vector sentPackets; +}; + +NodeInfoModuleTestShim *mod = nullptr; +MockMeshService *mockSvc = nullptr; +MockRouter *mockRouter = nullptr; +AirTime *testAirTime = nullptr; + +constexpr uint32_t kClockBaseMs = 60 * 60 * 1000; // an hour in, so no probe can underflow +constexpr uint32_t kThirtyMinMs = 30 * 60 * 1000; +constexpr uint32_t kThreeHoursMs = 3 * 60 * 60 * 1000; + +// Stamp "we sent a NodeInfo just now", jump the clock forward, and report whether another send is +// allowed. A permitted send re-stamps the history, which is why every probe stamps first. +bool sendAllowedAfterMs(uint32_t elapsedMs, bool shorterTimeout = false) +{ + transmitHistory->setLastSentToMesh(meshtastic_PortNum_NODEINFO_APP); + Time::advanceTestMillis(elapsedMs); + return mod->sendOurNodeInfo(NODENUM_BROADCAST, false, 0, shorterTimeout); +} + +} // namespace + +void setUp(void) +{ + Time::resetMonotonicForTests(); + Time::setTestMillis(kClockBaseMs); + Time::serviceMonotonic(); + + testAirTime = new AirTime(); + airTime = testAirTime; + + mockSvc = new MockMeshService(); + service = mockSvc; + + mockRouter = new MockRouter(); + router = mockRouter; + + if (transmitHistory) { + delete transmitHistory; + transmitHistory = nullptr; + } + transmitHistory = TransmitHistory::getInstance(); // fresh: loadFromDisk() is not called + + // The congestion coefficient is 1.0 at or below 40 online nodes, so the window under test is + // the bare floor. Nothing wires the node-status observer in a test build, but assert it rather + // than assume it - a non-zero count here would silently stretch every boundary below. + TEST_ASSERT_NOT_NULL(nodeStatus); + TEST_ASSERT_EQUAL_UINT16_MESSAGE(0, nodeStatus->getNumOnline(), "these boundaries assume an unscaled window"); + + config.device.role = meshtastic_Config_DeviceConfig_Role_CLIENT; + config.device.node_info_broadcast_secs = 0; // 0 selects the default, 3 hours + + owner.is_licensed = false; + strncpy(owner.long_name, "send window", sizeof(owner.long_name) - 1); + strncpy(owner.short_name, "sw", sizeof(owner.short_name) - 1); + + radioGeneration = 1; + mod = new NodeInfoModuleTestShim(); + nodeInfoModule = mod; + + // Settle the startup generation so no test inherits a pending "ask for replies", then drop the + // stamp that settling send left behind: every test starts unthrottled, and the ones that need a + // refusal arm the floor themselves. + mod->runOnce(); + mockRouter->sentPackets.clear(); + delete transmitHistory; + transmitHistory = nullptr; + transmitHistory = TransmitHistory::getInstance(); +} + +void tearDown(void) +{ + nodeInfoModule = nullptr; + delete mod; + mod = nullptr; + + // sendToMesh() copies a queue status to the phone on every send; toPhoneQueue takes ownership + // and nothing else drains it, so release them or LeakSanitizer aborts the run. + if (mockSvc) { + meshtastic_MeshPacket *p; + while ((p = mockSvc->getForPhone()) != nullptr) + mockSvc->releaseToPool(p); + } + + service = nullptr; + delete mockSvc; + mockSvc = nullptr; + + router = nullptr; + delete mockRouter; + mockRouter = nullptr; + + airTime = nullptr; + delete testAirTime; + testAirTime = nullptr; + + delete transmitHistory; + transmitHistory = nullptr; + + Time::useRealClock(); +} + +// The floor is 30 minutes, not the 10 it used to be: 10 and 29:59 are refused, 30:01 is not. +static void test_sendWindow_floorIsThirtyMinutes(void) +{ + TEST_ASSERT_FALSE_MESSAGE(sendAllowedAfterMs(10 * 60 * 1000), "10 min must be inside the window"); + TEST_ASSERT_FALSE_MESSAGE(sendAllowedAfterMs(kThirtyMinMs - 1000), "29:59 must be inside the window"); + TEST_ASSERT_TRUE_MESSAGE(sendAllowedAfterMs(kThirtyMinMs + 1000), "30:01 must be past the window"); +} + +// The interactive path keeps its own 60 second gate. Raising the routine floor must not have raised +// it, or a user-triggered send and a key verification would wait out half an hour. +static void test_sendWindow_interactiveSendKeepsItsSixtySecondGate(void) +{ + TEST_ASSERT_FALSE_MESSAGE(sendAllowedAfterMs(30 * 1000, /*shorterTimeout=*/true), "30 s is inside the 60 s gate"); + TEST_ASSERT_TRUE_MESSAGE(sendAllowedAfterMs(61 * 1000, /*shorterTimeout=*/true), "61 s is past the 60 s gate"); + TEST_ASSERT_TRUE_MESSAGE(sendAllowedAfterMs(5 * 60 * 1000, /*shorterTimeout=*/true), + "5 min must pass the interactive gate while still inside the routine floor"); +} + +// A send that goes out re-arms the countdown to a full interval - the default, and the configured +// value when there is one. Arming 1 ms first means only the reset can produce the expected value. +static void test_broadcastTimer_aSendRearmsTheRoutineCountdown(void) +{ + mod->armBroadcastCountdownForTests(1); + TEST_ASSERT_TRUE(mod->sendOurNodeInfo(NODENUM_BROADCAST, false, 0, false)); + TEST_ASSERT_EQUAL_UINT32(Default::getConfiguredOrDefaultMs(0, default_node_info_broadcast_secs), + (uint32_t)mod->broadcastCountdownMsForTests()); + TEST_ASSERT_EQUAL_UINT32(kThreeHoursMs, (uint32_t)mod->broadcastCountdownMsForTests()); + + config.device.node_info_broadcast_secs = 4 * 60 * 60; + mod->armBroadcastCountdownForTests(1); + Time::advanceTestMillis(kThirtyMinMs + 1000); + TEST_ASSERT_TRUE(mod->sendOurNodeInfo(NODENUM_BROADCAST, false, 0, false)); + TEST_ASSERT_EQUAL_UINT32(4 * 60 * 60 * 1000, (uint32_t)mod->broadcastCountdownMsForTests()); +} + +// interval is not what the scheduler reads: shouldRun() keys off _cached_next_run, and +// Thread::setInterval() recomputes that from last_run while setIntervalFromNow() recomputes it from +// now. Age last_run by an hour first and the two answers differ by an hour, so this case fails if +// the send ever re-arms the period without moving the deadline - which would fire the routine copy +// straight after an ad-hoc send, the exact thing the reset exists to prevent. +static void test_broadcastTimer_aSendMovesTheDeadlineNotJustThePeriod(void) +{ + const unsigned long ageMs = 60 * 60 * 1000; + mod->ageLastRunForTests(ageMs); + TEST_ASSERT_TRUE(mod->sendOurNodeInfo(NODENUM_BROADCAST, false, 0, false)); + + const unsigned long remaining = mod->broadcastDeadlineMsForTests() - millis(); + TEST_ASSERT_UINT32_WITHIN_MESSAGE(5000, kThreeHoursMs, (uint32_t)remaining, + "the next routine broadcast is due a full interval from the send, not from the last tick"); +} + +// An ad-hoc unicast - the shape of a greeting, a PKI decrypt failure or a completed key +// verification - re-arms the countdown just as a broadcast does. Without it the routine copy +// follows the ad-hoc one within minutes, putting two NodeInfos on the air for no gain. +static void test_broadcastTimer_anAdHocUnicastRearmsItToo(void) +{ + mod->armBroadcastCountdownForTests(1); + + TEST_ASSERT_TRUE(mod->sendOurNodeInfo(0x12345678, true, 0, false)); + TEST_ASSERT_EQUAL_UINT32(1, mockRouter->sentPackets.size()); + TEST_ASSERT_EQUAL_HEX32(0x12345678, mockRouter->sentPackets[0].to); + TEST_ASSERT_EQUAL_UINT32(kThreeHoursMs, (uint32_t)mod->broadcastCountdownMsForTests()); +} + +// A refused send must leave the countdown exactly where it was, or a node that keeps attempting +// greetings inside the window defers its routine broadcast forever. +static void test_broadcastTimer_aRefusedSendLeavesTheCountdownAlone(void) +{ + transmitHistory->setLastSentToMesh(meshtastic_PortNum_NODEINFO_APP); + Time::advanceTestMillis(60 * 1000); // a minute later: well inside the floor + + const unsigned long sentinel = 4321; + mod->armBroadcastCountdownForTests(sentinel); + + TEST_ASSERT_FALSE_MESSAGE(mod->sendOurNodeInfo(NODENUM_BROADCAST, false, 0, false), "the floor must refuse this send"); + TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size()); + TEST_ASSERT_EQUAL_UINT32(sentinel, (uint32_t)mod->broadcastCountdownMsForTests()); +} + +// A licensed station announces its call sign on a regulatory interval - ham mode sets +// node_info_broadcast_secs to 600 s for the FCC minimum - and the floor must not stretch that to 30 +// minutes. The unlicensed control below is the same configuration without the licence, so the +// assertion cannot pass by the floor quietly disappearing for everyone. +static void test_sendWindow_aLicensedStationKeepsItsCallSignInterval(void) +{ + config.device.node_info_broadcast_secs = 600; + + owner.is_licensed = true; + TEST_ASSERT_FALSE_MESSAGE(sendAllowedAfterMs(5 * 60 * 1000), "5 min is inside the station's own 10 min interval"); + TEST_ASSERT_TRUE_MESSAGE(sendAllowedAfterMs(11 * 60 * 1000), "11 min is past it, and the floor must not override it"); + + owner.is_licensed = false; + TEST_ASSERT_FALSE_MESSAGE(sendAllowedAfterMs(11 * 60 * 1000), "without a licence the 30 minute floor still applies"); +} + +// A send the router declines never reached the air. It must not defer the routine broadcast, and it +// must report failure so runOnce() does not treat a pending channel change as delivered. +static void test_broadcastTimer_aRejectedSendLeavesTheCountdownAlone(void) +{ + const unsigned long sentinel = 8765; + mod->armBroadcastCountdownForTests(sentinel); + mockRouter->sendResult = ERRNO_NO_INTERFACES; + + TEST_ASSERT_FALSE_MESSAGE(mod->sendOurNodeInfo(NODENUM_BROADCAST, false, 0, false), "a declined send is not a send"); + TEST_ASSERT_EQUAL_UINT32(sentinel, (uint32_t)mod->broadcastCountdownMsForTests()); +} + +// The countdown is only half of it: allocReply() used to stamp TransmitHistory when it built the +// packet, so a send the router then declined still started the window. With a 30 minute floor that +// silences the node for half an hour over a packet that never left. The retry immediately after must +// go out. +static void test_sendWindow_aRejectedSendDoesNotStartTheWindow(void) +{ + mockRouter->sendResult = ERRNO_NO_INTERFACES; + TEST_ASSERT_FALSE_MESSAGE(mod->sendOurNodeInfo(NODENUM_BROADCAST, false, 0, false), "the router declined this one"); + TEST_ASSERT_EQUAL_UINT32_MESSAGE(0, transmitHistory->getLastSentToMeshMillis(meshtastic_PortNum_NODEINFO_APP), + "a declined send must leave no transmit stamp behind"); + + mockRouter->sendResult = ERRNO_OK; + mockRouter->sentPackets.clear(); + TEST_ASSERT_TRUE_MESSAGE(mod->sendOurNodeInfo(NODENUM_BROADCAST, false, 0, false), + "the retry must not be throttled by the send that failed"); + TEST_ASSERT_EQUAL_UINT32(1, mockRouter->sentPackets.size()); +} + +// A preset or channel change bumps radioGeneration, and only a send that goes out consumes it: the +// refused attempt leaves the ask pending, the next successful one carries want_response, and the +// one after that does not ask again. +static void test_presetChange_isConsumedOnlyByASendThatGoesOut(void) +{ + radioGeneration++; + + // Arm the floor so the first attempt is refused, which is the case under test. + transmitHistory->setLastSentToMesh(meshtastic_PortNum_NODEINFO_APP); + Time::advanceTestMillis(60 * 1000); + + mod->runOnce(); + TEST_ASSERT_EQUAL_UINT32(0, mockRouter->sentPackets.size()); + + Time::advanceTestMillis(kThirtyMinMs + 1000); + mod->runOnce(); + TEST_ASSERT_EQUAL_UINT32(1, mockRouter->sentPackets.size()); + TEST_ASSERT_TRUE_MESSAGE(mockRouter->sentPackets[0].decoded.want_response, + "a refused send must not consume the preset change"); + TEST_ASSERT_EQUAL_HEX32(NODENUM_BROADCAST, mockRouter->sentPackets[0].to); + + Time::advanceTestMillis(kThirtyMinMs + 1000); + mod->runOnce(); + TEST_ASSERT_EQUAL_UINT32(2, mockRouter->sentPackets.size()); + TEST_ASSERT_FALSE_MESSAGE(mockRouter->sentPackets[1].decoded.want_response, + "a settled generation must not keep asking for replies"); +} + +NI_TEST_ENTRY void setup() +{ + initializeTestEnvironment(); + nodeDB = new NodeDB(); + + UNITY_BEGIN(); + RUN_TEST(test_sendWindow_floorIsThirtyMinutes); + RUN_TEST(test_sendWindow_interactiveSendKeepsItsSixtySecondGate); + RUN_TEST(test_broadcastTimer_aSendRearmsTheRoutineCountdown); + RUN_TEST(test_broadcastTimer_aSendMovesTheDeadlineNotJustThePeriod); + RUN_TEST(test_broadcastTimer_anAdHocUnicastRearmsItToo); + RUN_TEST(test_broadcastTimer_aRefusedSendLeavesTheCountdownAlone); + RUN_TEST(test_broadcastTimer_aRejectedSendLeavesTheCountdownAlone); + RUN_TEST(test_sendWindow_aRejectedSendDoesNotStartTheWindow); + RUN_TEST(test_sendWindow_aLicensedStationKeepsItsCallSignInterval); + RUN_TEST(test_presetChange_isConsumedOnlyByASendThatGoesOut); + exit(UNITY_END()); +} +NI_TEST_ENTRY void loop() {} From d971d507c472770eecb8c001467cddd94a6333fe Mon Sep 17 00:00:00 2001 From: Manuel <71137295+mverch67@users.noreply.github.com> Date: Sun, 20 Sep 2026 15:29:51 +0000 Subject: [PATCH 9/9] Thinknode M9 V2: keyboard and GPS (#11905) * thinknode v2 keyboard and GPS * update device-ui commit reference --- src/configuration.h | 3 +- src/detect/ScanI2CTwoWire.cpp | 3 +- src/input/STC8HKeyboard.h | 2 +- src/input/kbI2cBase.cpp | 8 ++-- .../ELECROW-ThinkNode-M9/platformio.ini | 12 ++++-- .../esp32s3/ELECROW-ThinkNode-M9/variant.cpp | 42 +++++++++++++++---- .../esp32s3/ELECROW-ThinkNode-M9/variant.h | 6 ++- 7 files changed, 55 insertions(+), 21 deletions(-) diff --git a/src/configuration.h b/src/configuration.h index 45e18bf6f..6d5587d88 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -265,7 +265,8 @@ along with this program. If not, see . #define BBQ10_KB_ADDR 0x1F #define MPR121_KB_ADDR 0x5A #define TCA8418_KB_ADDR 0x34 -#define TSTC8_KB_ADDR 0x6C // STC8H companion-MCU keypad on the ThinkNode-M9 +#define TSTC8_KB_V1_ADDR 0x6C // STC8H companion-MCU keypad on the ThinkNode-M9 V1 +#define TSTC8_KB_V2_ADDR 0x6D // STC8H companion-MCU keypad on the ThinkNode-M9 V2 // ----------------------------------------------------------------------------- // SENSOR diff --git a/src/detect/ScanI2CTwoWire.cpp b/src/detect/ScanI2CTwoWire.cpp index 767548998..26190462e 100644 --- a/src/detect/ScanI2CTwoWire.cpp +++ b/src/detect/ScanI2CTwoWire.cpp @@ -455,7 +455,8 @@ void ScanI2CTwoWire::scanPort(I2CPort port, uint8_t *address, uint8_t asize) type = BBQ10KB; logFoundDevice("BB Q10", (uint8_t)addr.address); break; - SCAN_SIMPLE_CASE(TSTC8_KB_ADDR, STC8HKB, "STC8H KB", (uint8_t)addr.address); + SCAN_SIMPLE_CASE(TSTC8_KB_V1_ADDR, STC8HKB, "STC8H KB", (uint8_t)addr.address); + SCAN_SIMPLE_CASE(TSTC8_KB_V2_ADDR, STC8HKB, "STC8H KB", (uint8_t)addr.address); SCAN_SIMPLE_CASE(ST7567_ADDRESS, SCREEN_ST7567, "ST7567", (uint8_t)addr.address); #ifdef HAS_NCP5623 SCAN_SIMPLE_CASE(NCP5623_ADDR, NCP5623, "NCP5623", (uint8_t)addr.address); diff --git a/src/input/STC8HKeyboard.h b/src/input/STC8HKeyboard.h index 7de3b3d28..ca2cb8ea4 100644 --- a/src/input/STC8HKeyboard.h +++ b/src/input/STC8HKeyboard.h @@ -53,7 +53,7 @@ class STC8HKeyboard private: void writeRegister(uint8_t reg, uint8_t val); - uint8_t _I2C_addr = TSTC8_KB_ADDR; + uint8_t _I2C_addr = TSTC8_KB_V1_ADDR; TwoWire *_pWire = &Wire; diff --git a/src/input/kbI2cBase.cpp b/src/input/kbI2cBase.cpp index d4851c7f6..2d47d7282 100644 --- a/src/input/kbI2cBase.cpp +++ b/src/input/kbI2cBase.cpp @@ -71,8 +71,8 @@ int32_t KbI2cBase::runOnce() // than the local Wire1 (e.g. SenseCAP Indicator) i2cBus = ScanI2CTwoWire::fetchI2CBus(cardkb_found); #if defined(ELECROW_ThinkNode_M9) - if (cardkb_found.address == TSTC8_KB_ADDR) { - Stc8HKeyBoard.begin(TSTC8_KB_ADDR, &Wire1); + if (cardkb_found.address == TSTC8_KB_V1_ADDR) { + Stc8HKeyBoard.begin(TSTC8_KB_V1_ADDR, &Wire1); } #endif if (cardkb_found.address == BBQ10_KB_ADDR) { @@ -91,8 +91,8 @@ int32_t KbI2cBase::runOnce() LOG_DEBUG("Use I2C Bus 0 (the first one)"); i2cBus = &Wire; #if defined(ELECROW_ThinkNode_M9) - if (cardkb_found.address == TSTC8_KB_ADDR) { - Stc8HKeyBoard.begin(TSTC8_KB_ADDR, &Wire); + if (cardkb_found.address == TSTC8_KB_V1_ADDR || cardkb_found.address == TSTC8_KB_V2_ADDR) { + Stc8HKeyBoard.begin(cardkb_found.address, &Wire); } #endif if (cardkb_found.address == BBQ10_KB_ADDR) { diff --git a/variants/esp32s3/ELECROW-ThinkNode-M9/platformio.ini b/variants/esp32s3/ELECROW-ThinkNode-M9/platformio.ini index dd7dc4a09..32ffda44e 100644 --- a/variants/esp32s3/ELECROW-ThinkNode-M9/platformio.ini +++ b/variants/esp32s3/ELECROW-ThinkNode-M9/platformio.ini @@ -35,7 +35,7 @@ build_flags = ; -D COMPASS_SENSOR_DEBUG=1 lib_deps = ${esp32s3_base.lib_deps} # renovate: datasource=custom depName=LovyanGFX packageName=lovyan03/library/LovyanGFX - lovyan03/LovyanGFX@1.2.26 + lovyan03/LovyanGFX@1.2.28 # renovate: datasource=git-refs depName=meshtastic-PCF8xRTC packageName=https://github.com/meshtastic/PCF8xRTC gitBranch=main https://github.com/meshtastic/PCF8xRTC/archive/efe1d9c92f79d8413f8ac4e9bdce0c48410600de.zip @@ -83,7 +83,8 @@ build_flags = -D LGFX_SCREEN_WIDTH=240 -D LGFX_SCREEN_HEIGHT=320 -D LGFX_INVERT_LIGHT=true -; -D MAP_FULL_REDRAW + -D MAP_FULL_REDRAW + -D FTP_BUF_SIZE=4096 -D MUI_WIFI_PS_MIN_MODEM -D DEFAULT_FTP_SERVER_NETWORK_TYPE_ESP32=NETWORK_ESP32 -D DEFAULT_STORAGE_TYPE_ESP32=STORAGE_SD @@ -91,9 +92,14 @@ build_flags = lib_deps = ${thinknode_m9_base.lib_deps} - https://github.com/meshtastic/device-ui/archive/70a9967f202a69390460c908a1321e475d3d4cdf.zip ; PR314 input-policy + https://github.com/meshtastic/device-ui/archive/24c8ea5da1537ce796069a5ccebc0c583cd9b23a.zip ; PR314 input-policy https://github.com/mverch67/MultiFTPServer/archive/0e854335b9916ed9f2d3bcfe68975ce746992ccd.zip custom_sdkconfig = ${esp32s3_base.custom_sdkconfig} ${device-ui_base.custom_sdkconfig} + +extra_scripts = + ${esp32s3_base.extra_scripts} + pre:extra_scripts/esp32_fatfs_exfat.py + post:extra_scripts/esp32_fatfs_exfat.py diff --git a/variants/esp32s3/ELECROW-ThinkNode-M9/variant.cpp b/variants/esp32s3/ELECROW-ThinkNode-M9/variant.cpp index 279dedeb5..fd802f752 100644 --- a/variants/esp32s3/ELECROW-ThinkNode-M9/variant.cpp +++ b/variants/esp32s3/ELECROW-ThinkNode-M9/variant.cpp @@ -3,6 +3,11 @@ #include "SPILock.h" #include "Wire.h" +#define KBD_ADDR_M9_VERSION1_0 0x6C +#define KBD_ADDR_M9_VERSION1_1 0x6D + +int m9Version = 0; + void earlyInitVariant() { pinMode(LORA_CS, OUTPUT); @@ -11,23 +16,42 @@ void earlyInitVariant() digitalWrite(SDCARD_CS, HIGH); pinMode(TFT_CS, OUTPUT); digitalWrite(TFT_CS, HIGH); + pinMode(PIN_GPS_EN, OUTPUT); + digitalWrite(PIN_GPS_EN, !GPS_EN_ACTIVE); + pinMode(GPS_RTC_INT, OUTPUT); + digitalWrite(GPS_RTC_INT, LOW); delay(100); } -void lateInitVariant() +void initVariant() { - // configure keyboard long-press time - const uint16_t ms = 700; - concurrency::LockGuard g(spiLock); - Wire.beginTransmission(0x6C); - Wire.write(0x03); - Wire.write((ms >> 8) & 0xFF); - Wire.write(ms & 0xFF); - Wire.endTransmission(); + // determine M9 version + Wire.begin(SDA, SCL); + Wire.beginTransmission(KBD_ADDR_M9_VERSION1_0); + if (Wire.endTransmission() == 0) { + m9Version = 1; + } + Wire.beginTransmission(KBD_ADDR_M9_VERSION1_1); + if (Wire.endTransmission() == 0) { + m9Version = 2; + } + + if (m9Version > 0) { + // configure keyboard long-press time + const uint16_t ms = 700; + Wire.beginTransmission(m9Version == 1 ? KBD_ADDR_M9_VERSION1_0 : KBD_ADDR_M9_VERSION1_1); + Wire.write(0x03); + Wire.write((ms >> 8) & 0xFF); + Wire.write(ms & 0xFF); + Wire.endTransmission(); + } + Wire.end(); } void variant_shutdown() { + pinMode(PIN_GPS_EN, OUTPUT); + digitalWrite(PIN_GPS_EN, !GPS_EN_ACTIVE); uint64_t gpioMask = (1ULL << KB_INT); gpio_pulldown_en((gpio_num_t)KB_INT); esp_sleep_enable_ext1_wakeup(gpioMask, ESP_EXT1_WAKEUP_ANY_HIGH); diff --git a/variants/esp32s3/ELECROW-ThinkNode-M9/variant.h b/variants/esp32s3/ELECROW-ThinkNode-M9/variant.h index 543a69089..1c50e13de 100644 --- a/variants/esp32s3/ELECROW-ThinkNode-M9/variant.h +++ b/variants/esp32s3/ELECROW-ThinkNode-M9/variant.h @@ -4,8 +4,6 @@ /*Power*/ #define VEXT_ENABLE 18 #define VEXT_ON_VALUE LOW -#define PIN_GPS_EN 11 -#define GPS_EN_ACTIVE LOW #define USE_POWERSAVE #define SLEEP_TIME 120 @@ -31,12 +29,16 @@ #define EXT_PWR_DETECT_VALUE LOW /*GPS*/ +extern int m9Version; #define HAS_GPS 1 #define GPS_BAUDRATE 115200 #define PIN_GPS_RESET 5 #define PIN_GPS_PPS 4 #define GPS_TX_PIN 3 #define GPS_RX_PIN 2 +#define GPS_RTC_INT 10 +#define PIN_GPS_EN 11 +#define GPS_EN_ACTIVE (m9Version > 1 ? HIGH : LOW) #define GPS_THREAD_INTERVAL 50 /*SPI*/