From 14eaa5587d571b76326f0e63c33c6d72d1f6fa38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Tue, 1 Sep 2026 11:52:40 +0000 Subject: [PATCH] Honor mute when waking the screen for a received message (#11688) * fix(ui): honor mute when waking the screen for a received message TextMessageModule fired powerFSM.trigger(EVENT_RECEIVED_MSG) for every text packet, gated only by shouldWakeOnReceivedMessage(), which checks external notification, device role and battery level but never the mute flags. A muted channel therefore suppressed the banner and still lit the screen. MessageRenderer::handleNewMessage() only computed mute for MessageType::BROADCAST, so a DM from a muted node produced a banner and a wake. Add isMutedForPacket() in Channels: a DM addressed to us reads the sender's NodeInfoLite mute bit, every other packet reads the mute bit of the channel it arrived on. This is the predicate ExternalNotificationModule already applied to the buzzer, vibra and LED outputs, hoisted so all three call sites share it. Bell and alert messages still break through mute on both paths, unchanged. No protobuf or config change: ChannelSettings.module_settings.is_muted and the NodeInfoLite mute bit already exist and are already settable from the device menu and via AdminMessage.toggle_muted_node. Closes #11674 * fix(ui): let an alert break through mute on the screen wake path In COLOR display mode TextMessageModule skips handleNewMessage(), so powerFSM.trigger(EVENT_RECEIVED_MSG) is the only wake an alert gets. Gating it on mute alone dropped that wake for a bell on a muted channel. Add MeshService::isAlertPayload(): an ASCII BEL in the payload while at least one alert_bell_* output is enabled. The wake gate is now "not muted, or an alert". MessageRenderer uses the same predicate instead of its own inline bell scan, which also lifts that scan's arbitrary 100 byte cap. Rename three test cases. Their names carried exactly 35 characters after the test_ prefix, which matches the Lob API key format and tripped trufflehog in the trunk check gate. --- src/graphics/draw/MessageRenderer.cpp | 28 +-- src/mesh/Channels.cpp | 9 + src/mesh/Channels.h | 4 + src/mesh/MeshService.cpp | 15 ++ src/mesh/MeshService.h | 4 + src/modules/ExternalNotificationModule.cpp | 10 +- src/modules/TextMessageModule.cpp | 7 +- test/state-manifest.tsv | 1 + test/test_muted_source/test_main.cpp | 239 +++++++++++++++++++++ 9 files changed, 285 insertions(+), 32 deletions(-) create mode 100644 test/test_muted_source/test_main.cpp diff --git a/src/graphics/draw/MessageRenderer.cpp b/src/graphics/draw/MessageRenderer.cpp index 2e023dc843..284bfc6c4f 100644 --- a/src/graphics/draw/MessageRenderer.cpp +++ b/src/graphics/draw/MessageRenderer.cpp @@ -3,6 +3,8 @@ #include "MessageRenderer.h" // Core includes +#include "Channels.h" +#include "MeshService.h" #include "MessageStore.h" #include "NodeDB.h" #include "UIRenderer.h" @@ -1138,13 +1140,7 @@ void handleNewMessage(OLEDDisplay *display, const StoredMessage &sm, const mesht // still happens so a message can light the screen back up. const bool menuShowing = NotificationRenderer::isMenuShowing(); - // Determine if message belongs to a muted channel - bool isChannelMuted = false; - if (sm.type == MessageType::BROADCAST) { - const meshtastic_Channel channel = channels.getByIndex(packet.channel ? packet.channel : channels.getPrimaryIndex()); - if (channel.settings.has_module_settings && channel.settings.module_settings.is_muted) - isChannelMuted = true; - } + const bool isMuted = isMutedForPacket(packet); // Banner logic const meshtastic_NodeInfoLite *node = nodeDB->getMeshNode(packet.from); @@ -1164,21 +1160,9 @@ void handleNewMessage(OLEDDisplay *display, const StoredMessage &sm, const mesht char truncatedLongName[64]; graphics::UIRenderer::truncateStringWithEmotes(display, longName, truncatedLongName, sizeof(truncatedLongName), availWidth); - const char *msgRaw = reinterpret_cast(packet.decoded.payload.bytes); char banner[256]; - bool isAlert = false; - - // Check if alert detection is enabled via external notification module - if (moduleConfig.external_notification.alert_bell || moduleConfig.external_notification.alert_bell_vibra || - moduleConfig.external_notification.alert_bell_buzzer) { - for (size_t i = 0; i < packet.decoded.payload.size && i < 100; i++) { - if (msgRaw[i] == '\x07') { - isAlert = true; - break; - } - } - } + const bool isAlert = MeshService::isAlertPayload(packet); if (isAlert) { if (truncatedLongName[0]) @@ -1186,8 +1170,8 @@ void handleNewMessage(OLEDDisplay *display, const StoredMessage &sm, const mesht else strcpy(banner, "Alert Received"); } else { - // Skip muted channels unless it's an alert - if (isChannelMuted) + // Skip muted channels/senders unless it's an alert + if (isMuted) return; if (truncatedLongName[0]) { diff --git a/src/mesh/Channels.cpp b/src/mesh/Channels.cpp index 770213b4a7..ec1a418eae 100644 --- a/src/mesh/Channels.cpp +++ b/src/mesh/Channels.cpp @@ -584,3 +584,12 @@ int16_t Channels::setActiveByIndex(ChannelIndex channelIndex) { return setCrypto(channelIndex); } + +bool isMutedForPacket(const meshtastic_MeshPacket &mp) +{ + if (!isBroadcast(mp.to) && isToUs(&mp)) + return nodeInfoLiteIsMuted(nodeDB->getMeshNode(mp.from)); + + const meshtastic_Channel &ch = channels.getByIndex(mp.channel ? mp.channel : channels.getPrimaryIndex()); + return ch.settings.has_module_settings && ch.settings.module_settings.is_muted; +} diff --git a/src/mesh/Channels.h b/src/mesh/Channels.h index 27833130c0..a7bbd2277a 100644 --- a/src/mesh/Channels.h +++ b/src/mesh/Channels.h @@ -160,6 +160,10 @@ extern Channels channels; static const uint8_t defaultpsk[] = {0xd4, 0xf1, 0xbb, 0x3a, 0x20, 0x29, 0x07, 0x59, 0xf0, 0xbc, 0xff, 0xab, 0xcf, 0x4e, 0x69, 0x01}; +/// True if the user muted the source of this packet: the sender for a DM addressed to us, +/// otherwise the channel it arrived on. +bool isMutedForPacket(const meshtastic_MeshPacket &mp); + /// True if a getKey()-resolved key offers no privacy: length 0 (off) or the public defaultpsk family. Pure; for tests. bool cryptoKeyIsPublic(const CryptoKey &key); diff --git a/src/mesh/MeshService.cpp b/src/mesh/MeshService.cpp index cfe213ff9b..707d292a94 100644 --- a/src/mesh/MeshService.cpp +++ b/src/mesh/MeshService.cpp @@ -445,6 +445,21 @@ bool MeshService::trySendPosition(NodeNum dest, bool wantReplies) return false; } +// ASCII BEL, the in-band alert marker. Numeric so no control byte sits in the source, and +// file-local because ASCII_BELL is already a macro in Screen.cpp and ExternalNotificationModule.cpp. +static const uint8_t kAsciiBell = 7; + +bool MeshService::isAlertPayload(const meshtastic_MeshPacket &p) +{ + if (!moduleConfig.external_notification.alert_bell && !moduleConfig.external_notification.alert_bell_vibra && + !moduleConfig.external_notification.alert_bell_buzzer) + return false; + for (pb_size_t i = 0; i < p.decoded.payload.size; i++) + if (p.decoded.payload.bytes[i] == kAsciiBell) + return true; + return false; +} + // Re-decode nested string-bearing payloads before local phone delivery so PB_VALIDATE_UTF8 rejects // malformed NodeInfo/Waypoint data a strict phone decoder could crash on. Mesh relay is unaffected. bool MeshService::phonePayloadIsDecodable(const meshtastic_Data &d) diff --git a/src/mesh/MeshService.h b/src/mesh/MeshService.h index 7adcdb7c6d..fb93370cc2 100644 --- a/src/mesh/MeshService.h +++ b/src/mesh/MeshService.h @@ -101,6 +101,10 @@ class MeshService p->decoded.portnum == meshtastic_PortNum_ALERT_APP; } + /// True if the sender flagged this text as an alert: an ASCII BEL in the payload while at least + /// one alert_bell_* output is enabled. Alerts deliberately break through a mute. + static bool isAlertPayload(const meshtastic_MeshPacket &p); + /// Returns false when a decoded NodeInfo/Waypoint payload fails nested protobuf decode (invalid /// UTF-8 under PB_VALIDATE_UTF8, etc.); other portnums pass through. Callers gate on the variant. static bool phonePayloadIsDecodable(const meshtastic_Data &decoded); diff --git a/src/modules/ExternalNotificationModule.cpp b/src/modules/ExternalNotificationModule.cpp index 0c97bf3575..ef0627f06b 100644 --- a/src/modules/ExternalNotificationModule.cpp +++ b/src/modules/ExternalNotificationModule.cpp @@ -14,6 +14,7 @@ * @date [Insert Date] */ #include "ExternalNotificationModule.h" +#include "Channels.h" #include "MeshService.h" #include "NodeDB.h" #include "Router.h" @@ -421,15 +422,8 @@ ProcessMessage ExternalNotificationModule::handleReceived(const meshtastic_MeshP } } - const meshtastic_NodeInfoLite *sender = nodeDB->getMeshNode(mp.from); - meshtastic_Channel ch = channels.getByIndex(mp.channel ? mp.channel : channels.getPrimaryIndex()); - - // If we receive a broadcast message, apply channel mute setting - // If we receive a direct message and the receipent is us, apply DM mute setting - // Else we just handle it as not muted. const bool isDmToUs = !isBroadcast(mp.to) && isToUs(&mp); - bool is_muted = isDmToUs ? nodeInfoLiteIsMuted(sender) - : (ch.settings.has_module_settings && ch.settings.module_settings.is_muted); + const bool is_muted = isMutedForPacket(mp); const bool buzzerModeIsDirectOnly = (config.device.buzzer_mode == meshtastic_Config_DeviceConfig_BuzzerMode_DIRECT_MSG_ONLY); diff --git a/src/modules/TextMessageModule.cpp b/src/modules/TextMessageModule.cpp index 818e39a948..843311a125 100644 --- a/src/modules/TextMessageModule.cpp +++ b/src/modules/TextMessageModule.cpp @@ -1,4 +1,5 @@ #include "TextMessageModule.h" +#include "Channels.h" #include "MeshService.h" #include "MessageStore.h" #include "NodeDB.h" @@ -34,8 +35,10 @@ ProcessMessage TextMessageModule::handleReceived(const meshtastic_MeshPacket &mp auto *display = screen ? screen->getDisplayDevice() : nullptr; graphics::MessageRenderer::handleNewMessage(display, *sm, mp); }) - // Only trigger screen wake if configuration allows it - if (shouldWakeOnReceivedMessage()) { + // Only trigger screen wake if configuration allows it and the channel/sender isn't muted. + // An alert breaks through the mute: in COLOR display mode handleNewMessage() above never runs, + // so this trigger is the only wake an alert would get. + if (shouldWakeOnReceivedMessage() && (!isMutedForPacket(mp) || MeshService::isAlertPayload(mp))) { powerFSM.trigger(EVENT_RECEIVED_MSG); } diff --git a/test/state-manifest.tsv b/test/state-manifest.tsv index 4c8f56bbb1..f2ef9ff4c1 100644 --- a/test/state-manifest.tsv +++ b/test/state-manifest.tsv @@ -58,6 +58,7 @@ test_hop_start_policy writes=config.proto,module.proto,device.proto,channels.pro test_mesh_beacon writes=module.proto exercises the beacon's module-config save path test_mesh_module writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat module framework tests construct a NodeDB test_mqtt writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto errors=1000..12000 constructs a NodeDB for node lookups in the MQTT paths +test_muted_source writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto constructs a NodeDB (isToUs needs nodeDB->getNodeNum(), and the DM branch looks the sender up), whose constructor persists a default set when the prefs directory is empty test_nexthop_routing writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto next-hop selection reads and updates the node DB test_nodedb_blocked state=per-suite writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto,warm.dat saturates the DB with MAX_NUM_NODES-2 favourited nodes to test the protected cap; a later test's removeNodeByNum() persists that state, and the cap test depends on the fill from the test before it test_nodedb_boot_recovery state=per-suite writes=config.proto,module.proto,device.proto,channels.proto,nodes.proto deliberate boot-recovery ladder: corrupts/deletes/restores the pref files and reboots a NodeDB per test to pin the DECODE_FAILED identity freeze, so each test observes the previous test's on-disk state diff --git a/test/test_muted_source/test_main.cpp b/test/test_muted_source/test_main.cpp new file mode 100644 index 0000000000..9cc2eedeef --- /dev/null +++ b/test/test_muted_source/test_main.cpp @@ -0,0 +1,239 @@ +// isMutedForPacket() source resolution - src/mesh/Channels.cpp. A DM addressed to us reads the +// sender's mute bit; every other packet reads the mute bit of the channel it arrived on. +#include "MeshTypes.h" // Include BEFORE TestUtil.h (provides NodeNum, isToUs, isBroadcast) +#include "TestUtil.h" +#include + +#include "mesh/Channels.h" +#include "mesh/MeshService.h" +#include "mesh/NodeDB.h" +#include +#include + +static constexpr NodeNum kLocalNode = 0x11111111; +static constexpr NodeNum kPeer = 0x22222222; +static constexpr NodeNum kThirdParty = 0x33333333; +static constexpr NodeNum kStranger = 0x44444444; // deliberately never added to the DB + +// isToUs() reads nodeDB->getNodeNum() and the DM branch looks the sender up, so a real NodeDB +// must be live. +static NodeDB *testNodeDB = nullptr; + +static meshtastic_MeshPacket makePacket(NodeNum from, NodeNum to, uint8_t channel) +{ + meshtastic_MeshPacket p = meshtastic_MeshPacket_init_zero; + p.from = from; + p.to = to; + p.channel = channel; + p.which_payload_variant = meshtastic_MeshPacket_decoded_tag; + p.decoded.portnum = meshtastic_PortNum_TEXT_MESSAGE_APP; + return p; +} + +static void setSlot(ChannelIndex idx, meshtastic_Channel_Role role, bool muted) +{ + meshtastic_Channel &ch = channels.getByIndex(idx); + ch.index = idx; + ch.has_settings = true; + ch.role = role; + ch.settings.has_module_settings = true; + ch.settings.module_settings.is_muted = muted; +} + +// Append straight into the hot store: getOrCreateMeshNode() would drag in the cap and +// eviction machinery, which this predicate has nothing to do with. +static void setNodeMuted(NodeNum num, bool muted) +{ + meshtastic_NodeInfoLite *n = nodeDB->getMeshNode(num); + if (!n) { + nodeDB->meshNodes->resize(nodeDB->numMeshNodes + 1); + n = &nodeDB->meshNodes->at(nodeDB->numMeshNodes++); + memset(n, 0, sizeof(*n)); + n->num = num; + } + nodeInfoLiteSetBit(n, NODEINFO_BITFIELD_IS_MUTED_MASK, muted); +} + +// --------------------------------------------------------------------------- +// Broadcast: the arrival channel decides +// --------------------------------------------------------------------------- + +void test_broadcast_on_unmuted_channel_is_not_muted() +{ + TEST_ASSERT_FALSE(isMutedForPacket(makePacket(kPeer, NODENUM_BROADCAST, 0))); +} + +void test_broadcast_on_muted_channel() +{ + setSlot(0, meshtastic_Channel_Role_PRIMARY, true); + TEST_ASSERT_TRUE(isMutedForPacket(makePacket(kPeer, NODENUM_BROADCAST, 0))); +} + +// A channel with no module_settings at all has never been muted. +void test_channel_without_module_settings_is_not_muted() +{ + meshtastic_Channel &ch = channels.getByIndex(0); + ch.settings.has_module_settings = false; + ch.settings.module_settings.is_muted = true; // stale payload behind the presence flag + TEST_ASSERT_FALSE(isMutedForPacket(makePacket(kPeer, NODENUM_BROADCAST, 0))); +} + +// Mute is per channel, not global: a muted secondary must not silence the others. +void test_broadcast_reads_its_own_channel() +{ + setSlot(2, meshtastic_Channel_Role_SECONDARY, true); + setSlot(1, meshtastic_Channel_Role_SECONDARY, false); + TEST_ASSERT_TRUE(isMutedForPacket(makePacket(kPeer, NODENUM_BROADCAST, 2))); + TEST_ASSERT_FALSE(isMutedForPacket(makePacket(kPeer, NODENUM_BROADCAST, 1))); +} + +// channel == 0 means "the primary", which is not always slot 0. +void test_channel_zero_resolves_to_primary_slot() +{ + setSlot(0, meshtastic_Channel_Role_SECONDARY, false); + setSlot(3, meshtastic_Channel_Role_PRIMARY, true); + channels.onConfigChanged(); + TEST_ASSERT_EQUAL_UINT8(3, channels.getPrimaryIndex()); + TEST_ASSERT_TRUE(isMutedForPacket(makePacket(kPeer, NODENUM_BROADCAST, 0))); +} + +// --------------------------------------------------------------------------- +// DM addressed to us: the sender decides +// --------------------------------------------------------------------------- + +void test_dm_to_us_from_muted_sender() +{ + setNodeMuted(kPeer, true); + TEST_ASSERT_TRUE(isMutedForPacket(makePacket(kPeer, kLocalNode, 0))); +} + +void test_dm_to_us_from_unmuted_sender_is_not_muted() +{ + setNodeMuted(kPeer, false); + TEST_ASSERT_FALSE(isMutedForPacket(makePacket(kPeer, kLocalNode, 0))); +} + +// The discriminator: a DM must not inherit its channel's mute state. +void test_dm_to_us_ignores_channel_mute() +{ + setSlot(0, meshtastic_Channel_Role_PRIMARY, true); + setNodeMuted(kPeer, false); + TEST_ASSERT_FALSE(isMutedForPacket(makePacket(kPeer, kLocalNode, 0))); +} + +// A sender we have never heard of has no mute bit to read. +void test_dm_to_us_from_unknown_sender_is_not_muted() +{ + TEST_ASSERT_FALSE(isMutedForPacket(makePacket(kStranger, kLocalNode, 0))); +} + +// Not addressed to us: overheard traffic falls back to the channel, sender mute is irrelevant. +void test_dm_to_third_party_uses_channel() +{ + setSlot(0, meshtastic_Channel_Role_PRIMARY, true); + setNodeMuted(kPeer, false); + TEST_ASSERT_TRUE(isMutedForPacket(makePacket(kPeer, kThirdParty, 0))); + + setSlot(0, meshtastic_Channel_Role_PRIMARY, false); + setNodeMuted(kPeer, true); + TEST_ASSERT_FALSE(isMutedForPacket(makePacket(kPeer, kThirdParty, 0))); +} + +// --------------------------------------------------------------------------- +// Alert payloads, which break through a mute +// --------------------------------------------------------------------------- + +// ASCII BEL, the in-band alert marker. Numeric so no control byte sits in the source. +static const uint8_t kAsciiBell = 7; + +static meshtastic_MeshPacket withText(meshtastic_MeshPacket p, const char *text, bool bell) +{ + p.decoded.payload.size = (pb_size_t)strlen(text); + memcpy(p.decoded.payload.bytes, text, p.decoded.payload.size); + if (bell) + p.decoded.payload.bytes[p.decoded.payload.size++] = kAsciiBell; + return p; +} + +void test_bell_is_an_alert_when_a_bell_output_is_on() +{ + moduleConfig.external_notification.alert_bell = true; + TEST_ASSERT_TRUE(MeshService::isAlertPayload(withText(makePacket(kPeer, NODENUM_BROADCAST, 0), "wake up", true))); +} + +void test_bell_is_not_an_alert_when_every_bell_output_is_off() +{ + TEST_ASSERT_FALSE(MeshService::isAlertPayload(withText(makePacket(kPeer, NODENUM_BROADCAST, 0), "wake up", true))); +} + +void test_plain_text_is_never_an_alert() +{ + moduleConfig.external_notification.alert_bell = true; + TEST_ASSERT_FALSE(MeshService::isAlertPayload(withText(makePacket(kPeer, NODENUM_BROADCAST, 0), "wake up", false))); +} + +// The wake gate is "not muted, or an alert": a bell must survive a muted channel. +void test_alert_survives_a_muted_channel() +{ + moduleConfig.external_notification.alert_bell = true; + setSlot(0, meshtastic_Channel_Role_PRIMARY, true); + const meshtastic_MeshPacket p = withText(makePacket(kPeer, NODENUM_BROADCAST, 0), "wake up", true); + TEST_ASSERT_TRUE(isMutedForPacket(p)); + TEST_ASSERT_TRUE(!isMutedForPacket(p) || MeshService::isAlertPayload(p)); +} + +// --------------------------------------------------------------------------- +// Unity lifecycle +// --------------------------------------------------------------------------- + +void setUp(void) +{ + if (!testNodeDB) + testNodeDB = new NodeDB(); // its constructor overwrites my_node_num, so claim ours after + + config = meshtastic_LocalConfig_init_zero; + moduleConfig = meshtastic_LocalModuleConfig_init_zero; + myNodeInfo.my_node_num = kLocalNode; + nodeDB = testNodeDB; + + // Start from an empty hot store so kStranger is genuinely unknown. + nodeDB->meshNodes->clear(); + nodeDB->numMeshNodes = 0; + + memset(&channelFile, 0, sizeof(channelFile)); + channels.initDefaults(); + channels.onConfigChanged(); +} + +void tearDown(void) {} + +void setup() +{ + initializeTestEnvironment(); + + UNITY_BEGIN(); + + printf("\n=== Broadcast: channel mute ===\n"); + RUN_TEST(test_broadcast_on_unmuted_channel_is_not_muted); + RUN_TEST(test_broadcast_on_muted_channel); + RUN_TEST(test_channel_without_module_settings_is_not_muted); + RUN_TEST(test_broadcast_reads_its_own_channel); + RUN_TEST(test_channel_zero_resolves_to_primary_slot); + + printf("\n=== Direct message: sender mute ===\n"); + RUN_TEST(test_dm_to_us_from_muted_sender); + RUN_TEST(test_dm_to_us_from_unmuted_sender_is_not_muted); + RUN_TEST(test_dm_to_us_ignores_channel_mute); + RUN_TEST(test_dm_to_us_from_unknown_sender_is_not_muted); + RUN_TEST(test_dm_to_third_party_uses_channel); + + printf("\n=== Alerts break through mute ===\n"); + RUN_TEST(test_bell_is_an_alert_when_a_bell_output_is_on); + RUN_TEST(test_bell_is_not_an_alert_when_every_bell_output_is_off); + RUN_TEST(test_plain_text_is_never_an_alert); + RUN_TEST(test_alert_survives_a_muted_channel); + + exit(UNITY_END()); +} + +void loop() {}