Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/test_native.yml
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,16 @@ jobs:
# the test artifact is unaffected.
platformio test -e coverage -v --junit-output-path testreport.xml 2>&1 | grep -v "[[:space:]]SKIPPED$"

- name: Event channel policy tests
run: platformio test -e coverage-event-policy -v --junit-output-path event-policy-testreport.xml

- name: Save test results
if: always() # run this step even if previous step failed
uses: actions/upload-artifact@v7
with:
name: platformio-test-report-${{ steps.version.outputs.long }}
overwrite: true
path: ./testreport.xml
path: ./*testreport.xml

- name: Capture coverage information
if: always() # run this step even if previous step failed
Expand Down
17 changes: 16 additions & 1 deletion src/mesh/Channels.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,21 @@ bool Channels::isWellKnownChannel(ChannelIndex chIndex)
return false;
}

bool Channels::isEventChannel(ChannelIndex chIndex)
{
#if USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL && defined(USERPREFS_CHANNEL_0_PSK)
static const uint8_t configuredEventPsk[] = USERPREFS_CHANNEL_0_PSK;
static_assert(sizeof(configuredEventPsk) == 16 || sizeof(configuredEventPsk) == 32,
"USERPREFS_CHANNEL_0_PSK must be an AES-128 or AES-256 key");
CryptoKey effectiveKey = getKey(chIndex);
return effectiveKey.length == sizeof(configuredEventPsk) &&
memcmp(effectiveKey.bytes, configuredEventPsk, sizeof(configuredEventPsk)) == 0;
#else
(void)chIndex;
return false;
#endif
}

bool Channels::hasDefaultChannel()
{
// If we don't use a preset or the default frequency slot, or we override the frequency, we don't have a default channel
Expand Down Expand Up @@ -563,4 +578,4 @@ bool Channels::setDefaultPresetCryptoForHash(ChannelHash channelHash)
int16_t Channels::setActiveByIndex(ChannelIndex channelIndex)
{
return setCrypto(channelIndex);
}
}
13 changes: 12 additions & 1 deletion src/mesh/Channels.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@
#include "mesh-pb-constants.h"
#include <Arduino.h>

#ifndef USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL
#define USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL USERPREFS_EVENT_MODE
#endif

#if USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL && !defined(USERPREFS_CHANNEL_0_PSK)
#error "USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL requires USERPREFS_CHANNEL_0_PSK"
#endif

/** A channel number (index into the channel table)
*/
typedef uint8_t ChannelIndex;
Expand Down Expand Up @@ -95,6 +103,9 @@ class Channels
// matches the current preset's name and PSK byte 1.
bool isWellKnownChannel(ChannelIndex chIndex);

// Returns true if this channel's effective key matches USERPREFS_CHANNEL_0_PSK.
bool isEventChannel(ChannelIndex chIndex);

// Returns true if we can be reached via a channel with the default settings given a region and modem preset
bool hasDefaultChannel();

Expand Down Expand Up @@ -164,4 +175,4 @@ bool channelFileUsesPublicKey(const meshtastic_ChannelFile &cf, ChannelIndex chI

static const uint8_t eventpsk[] = {0x38, 0x4b, 0xbc, 0xc0, 0x1d, 0xc0, 0x22, 0xd1, 0x81, 0xbf, 0x36,
0xb8, 0x61, 0x21, 0xe1, 0xfb, 0x96, 0xb7, 0x2e, 0x55, 0xbf, 0x74,
0x22, 0x7e, 0x9d, 0x6a, 0xfb, 0x48, 0xd6, 0x4c, 0xb1, 0xa1};
0x22, 0x7e, 0x9d, 0x6a, 0xfb, 0x48, 0xd6, 0x4c, 0xb1, 0xa1};
31 changes: 19 additions & 12 deletions src/mesh/FloodingRouter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ bool FloodingRouter::shouldFilterReceived(const meshtastic_MeshPacket *p)
LOG_DEBUG("Repeated reliable tx");
// Check if it's still in the Tx queue, if not, we have to relay it again
if (!findInTxQueue(p->from, p->id)) {
reprocessPacket(p);
perhapsRebroadcast(p);
if (reprocessPacket(p))
perhapsRebroadcast(p);
}
} else {
perhapsCancelDupe(p);
Expand All @@ -75,8 +75,8 @@ bool FloodingRouter::perhapsHandleUpgradedPacket(const meshtastic_MeshPacket *p)
LOG_DEBUG("Processing upgraded packet 0x%08x for rebroadcast with hop limit %d (dropping queued < %d)", p->id,
p->hop_limit, dropThreshold);

reprocessPacket(p);
perhapsRebroadcast(p);
if (reprocessPacket(p))
perhapsRebroadcast(p);

rxDupe++;
// We already enqueued the improved copy, so make sure the incoming packet stops here.
Expand All @@ -87,32 +87,39 @@ bool FloodingRouter::perhapsHandleUpgradedPacket(const meshtastic_MeshPacket *p)
return false;
}

void FloodingRouter::reprocessPacket(const meshtastic_MeshPacket *p)
bool FloodingRouter::reprocessPacket(const meshtastic_MeshPacket *p)
{
if (nodeDB)
nodeDB->updateFrom(*p);

#if !MESHTASTIC_EXCLUDE_TRACEROUTE
if (traceRouteModule && p->which_payload_variant != meshtastic_MeshPacket_decoded_tag) {
// If we got a packet that is not decoded, try to decode it so we can check for traceroute.
bool shouldDecode = false;
#if USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL
shouldDecode = true;
#elif !MESHTASTIC_EXCLUDE_TRACEROUTE
shouldDecode = traceRouteModule != nullptr;
#endif
if (shouldDecode && p->which_payload_variant != meshtastic_MeshPacket_decoded_tag) {
// Duplicate handling can rebroadcast before the normal receive path. The
// event policy needs the portnum even when TraceRoute is excluded.
auto decodedState = perhapsDecode(const_cast<meshtastic_MeshPacket *>(p));
if (decodedState == DecodeState::DECODE_SUCCESS) {
// parsing was successful, print for debugging
printPacket("reprocessPacket(DUP)", p);
} else {
// Fatal decoding error, we can't do anything with this packet
LOG_WARN(
"FloodingRouter::reprocessPacket: Fatal decode error (state=%d, id=0x%08x, from=%u), can't check for traceroute",
static_cast<int>(decodedState), p->id, getFrom(p));
return;
LOG_WARN("FloodingRouter::reprocessPacket: decode error (state=%d, id=0x%08x, from=%u)",
static_cast<int>(decodedState), p->id, getFrom(p));
return true;
Comment on lines 109 to +112

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Format from as a node ID.

Use 0x%08x rather than decimal %u.

-            LOG_WARN("FloodingRouter::reprocessPacket: decode error (state=%d, id=0x%08x, from=%u)",
+            LOG_WARN("FloodingRouter::reprocessPacket: decode error (state=%d, id=0x%08x, from=0x%08x)",

As per coding guidelines, “Format node and packet IDs as 0x%08x.”

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Fatal decoding error, we can't do anything with this packet
LOG_WARN(
"FloodingRouter::reprocessPacket: Fatal decode error (state=%d, id=0x%08x, from=%u), can't check for traceroute",
static_cast<int>(decodedState), p->id, getFrom(p));
return;
LOG_WARN("FloodingRouter::reprocessPacket: decode error (state=%d, id=0x%08x, from=%u)",
static_cast<int>(decodedState), p->id, getFrom(p));
return true;
// Fatal decoding error, we can't do anything with this packet
LOG_WARN("FloodingRouter::reprocessPacket: decode error (state=%d, id=0x%08x, from=0x%08x)",
static_cast<int>(decodedState), p->id, getFrom(p));
return true;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mesh/FloodingRouter.cpp` around lines 109 - 112, Update the LOG_WARN call
in FloodingRouter::reprocessPacket to format the getFrom(p) node ID with the
hexadecimal 0x%08x specifier instead of decimal %u, keeping the existing
arguments and message context unchanged.

Source: Coding guidelines

}
}

#if !MESHTASTIC_EXCLUDE_TRACEROUTE
if (traceRouteModule && p->which_payload_variant == meshtastic_MeshPacket_decoded_tag &&
p->decoded.portnum == meshtastic_PortNum_TRACEROUTE_APP) {
traceRouteModule->processUpgradedPacket(*p);
}
#endif
return true;
}

bool FloodingRouter::roleAllowsCancelingDupe(const meshtastic_MeshPacket *p)
Expand Down
4 changes: 2 additions & 2 deletions src/mesh/FloodingRouter.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ class FloodingRouter : public Router
bool perhapsHandleUpgradedPacket(const meshtastic_MeshPacket *p);

/* Call when we receive a packet that needs some reprocessing, but afterwards should be filtered */
void reprocessPacket(const meshtastic_MeshPacket *p);
bool reprocessPacket(const meshtastic_MeshPacket *p);

// Return false for roles like ROUTER which should always rebroadcast even when we've heard another rebroadcast of
// the same packet
Expand All @@ -75,4 +75,4 @@ class FloodingRouter : public Router

// Return true if we are a rebroadcaster
bool isRebroadcaster();
};
};
16 changes: 12 additions & 4 deletions src/mesh/NextHopRouter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,16 @@ bool NextHopRouter::shouldFilterReceived(const meshtastic_MeshPacket *p)
LOG_INFO("Fallback to flooding from relay_node=0x%x", p->relay_node);
// Check if it's still in the Tx queue, if not, we have to relay it again
if (!findInTxQueue(p->from, p->id)) {
reprocessPacket(p);
perhapsRebroadcast(p);
if (reprocessPacket(p))
perhapsRebroadcast(p);
}
} else {
bool isRepeated = getHopsAway(*p) == 0;
// If repeated and not in Tx queue anymore, try relaying again, or if we are the destination, send the ACK again
if (isRepeated) {
if (!findInTxQueue(p->from, p->id)) {
reprocessPacket(p);
if (!perhapsRebroadcast(p) && isToUs(p) && p->want_ack) {
if (reprocessPacket(p) && !isBlockedEventCoordinatePacket(p) && !perhapsRebroadcast(p) && isToUs(p) &&
p->want_ack) {
sendAckNak(meshtastic_Routing_Error_NONE, getFrom(p), p->id, p->channel, 0);
}
}
Expand Down Expand Up @@ -151,6 +151,14 @@ void NextHopRouter::sniffReceived(const meshtastic_MeshPacket *p, const meshtast
/* Check if we should be rebroadcasting this packet if so, do so. */
bool NextHopRouter::perhapsRebroadcast(const meshtastic_MeshPacket *p)
{
#if USERPREFS_BLOCK_POSITION_ON_EVENT_CHANNEL
// Never relay coordinate-bearing packets on the event ("everyone") channel.
// Closes the reliable-retransmit-dupe path that runs before handleReceived().
if (isBlockedEventCoordinatePacket(p)) {
return false;
}
#endif

// Check if traffic management wants to exhaust this packet's hops
bool exhaustHops = false;
#if HAS_TRAFFIC_MANAGEMENT
Expand Down
10 changes: 10 additions & 0 deletions src/mesh/PhoneAPI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1764,6 +1764,16 @@ bool PhoneAPI::handleToRadioPacket(meshtastic_MeshPacket &p)
}
#endif

// Reject before recording duplicate or per-port cooldown state, so a blocked
// attempt cannot throttle a valid private-channel position retry.
if (isBlockedEventCoordinatePacket(&p)) {
LOG_DEBUG("Suppress phone coordinate send on event (everyone) channel");
meshtastic_QueueStatus qs = router->getQueueStatus();
service->sendQueueStatusToPhone(qs, 0, p.id);
sendNotification(meshtastic_LogRecord_Level_WARNING, p.id, "Location sharing is disabled on this channel");
return false;
}

#if defined(ARCH_PORTDUINO)
// For use with the simulator, we should not ignore duplicate packets from the phone
if (SimRadio::instance == nullptr)
Expand Down
4 changes: 4 additions & 0 deletions src/mesh/PositionPrecision.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ uint32_t getPositionPrecisionForChannel(const meshtastic_Channel &channel)

uint32_t getPositionPrecisionForChannel(uint8_t channelIndex)
{
// Event-channel privacy takes precedence over every stored precision and key policy.
if (channels.isEventChannel(channelIndex))
return 0;

const meshtastic_Channel &ch = channels.getByIndex(channelIndex);
if (ch.role == meshtastic_Channel_Role_DISABLED)
return 0;
Expand Down
8 changes: 7 additions & 1 deletion src/mesh/ReliableRouter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@
*/
ErrorCode ReliableRouter::send(meshtastic_MeshPacket *p)
{
if (isBlockedEventCoordinatePacket(p)) {
LOG_DEBUG("Suppress reliable coordinate send on event (everyone) channel");
packetPool.release(p);
return meshtastic_Routing_Error_NOT_AUTHORIZED;
}

if (p->want_ack) {
DEBUG_HEAP_BEFORE;
auto copy = packetPool.allocCopy(*p);
Expand Down Expand Up @@ -196,4 +202,4 @@ bool ReliableRouter::shouldSuccessAckWithWantAck(const meshtastic_MeshPacket *p)
}

return false;
}
}
Loading
Loading