Skip to content

Conversation

thebentern
Copy link
Contributor

For consideration

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds channel-specific uplink and downlink control for UDP multicast communication by leveraging the existing channel configuration settings.

Key changes:

  • Adds channel validation for both UDP uplink and downlink operations
  • Modifies the onSend method signature to include channel index parameter
  • Updates Router.cpp to pass channel information to UDP handler

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/mesh/udp/UdpMulticastHandler.h Adds channel downlink/uplink validation and updates onSend method signature
src/mesh/Router.cpp Updates UDP handler calls to pass channel index and reorganizes UDP handling logic

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@thebentern thebentern added the enhancement New feature or request label Sep 27, 2025
@jp-bennett
Copy link
Collaborator

Only thing to consider is whether we would eventually want to control the UDP uplink separately from the MQTT.

@thebentern
Copy link
Contributor Author

Agreed. I think in the long term we can split this control out into it's own levers

Only thing to consider is whether we would eventually want to control the UDP uplink separately from the MQTT.

@thebentern
Copy link
Contributor Author

@GUVWAF added the default key fallback to this one

@thebentern thebentern requested a review from GUVWAF September 28, 2025 11:39
udpHandler->onSend(const_cast<meshtastic_MeshPacket *>(p));
// For packets that are already encrypted, we need to determine the channel from packet info
else if (udpHandler && config.network.enabled_protocols & meshtastic_Config_NetworkConfig_ProtocolFlags_UDP_BROADCAST &&
isFromUs(p)) {
Copy link
Member

Choose a reason for hiding this comment

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

isFromUs(p) is here still.

else if (udpHandler && config.network.enabled_protocols & meshtastic_Config_NetworkConfig_ProtocolFlags_UDP_BROADCAST &&
isFromUs(p)) {
// Check if any channel has uplink enabled (for PKI support)
bool uplinkEnabled = false;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe instead of this bool we can use an int8_t to store the channel index where uplink is enabled, and -1 if none was found such that we can eliminate the same loop hereafter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants