feat: Add support for PAYLOAD_TYPE_GRP_DATA by dz0ny · Pull Request #1928 · meshcore-dev/MeshCore

dz0ny · 2026-03-05T12:34:24Z

Docs changes are to reflect how it is currently in fw
This adds ability to send datagram data to everyone in channel

Copilot

Pull request overview

Adds channel-wide binary datagram support by introducing PAYLOAD_TYPE_GRP_DATA handling and exposing it through the companion radio example + protocol docs, enabling non-text payloads to be broadcast to all members of a group channel.

Changes:

Add a new onChannelDataRecv() hook and sendGroupData() API to the chat mesh helper layer.
Handle inbound PAYLOAD_TYPE_GRP_DATA packets and forward them to the new callback.
Update the companion radio example and companion protocol docs to support/send/parse channel payloads beyond plain UTF-8 text.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/helpers/BaseChatMesh.h	Adds `onChannelDataRecv()` virtual + `sendGroupData()` API.
src/helpers/BaseChatMesh.cpp	Routes `PAYLOAD_TYPE_GRP_DATA` to the new callback; implements `sendGroupData()`.
examples/companion_radio/MyMesh.h	Declares override for `onChannelDataRecv()`.
examples/companion_radio/MyMesh.cpp	Implements channel data receive framing + sends non-plain channel payloads via `sendGroupData()`.
docs/companion_protocol.md	Updates protocol docs to describe channel payload semantics and OK/error responses.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-05T12:40:43Z

src/helpers/BaseChatMesh.cpp

+  if (data_len < 0) return false;
+  if (data_len > MAX_PACKET_PAYLOAD - 5) data_len = MAX_PACKET_PAYLOAD - 5;
+
+  uint8_t temp[MAX_PACKET_PAYLOAD];
+  memcpy(temp, &timestamp, 4);
+  temp[4] = txt_type;
+  if (data_len > 0) memcpy(&temp[5], data, data_len);
+
+  auto pkt = createGroupDatagram(PAYLOAD_TYPE_GRP_DATA, channel, temp, 5 + data_len);


sendGroupData() clamps data_len to MAX_PACKET_PAYLOAD - 5, but Mesh::createGroupDatagram() will reject plaintext lengths above MAX_PACKET_PAYLOAD - CIPHER_BLOCK_SIZE (168 bytes). That means any data_len > 163 (since you add the 5-byte header) will still fail with pkt == NULL, and the caller reports a generic send failure. Clamp to the maximum plaintext length accepted by createGroupDatagram() (accounting for the 5-byte header), or explicitly detect/return a distinct error for oversize payloads so the failure mode is deterministic and debuggable.

liamcottle · 2026-03-05T12:55:26Z

Howdy, thanks for the PR!

We have a previously opened PR for similar functionality here #1530.

We left some review comments on it, but it was never followed up so didn't get merged.
Please have a read through for more info.

I noticed in this PR you have merged both group text and group data into the single CMD_SEND_GROUP_TEXT command, personally I'd prefer to have separate commands.

CMD_SEND_CHANNEL_TXT_MSG
CMD_SEND_CHANNEL_DATA

We also need to have a distinct payload type at the start of the raw group data as well, so we can reserve certain types for specific tasks.

I have some more info about this in the previous PR comments.

dz0ny · 2026-03-05T13:00:51Z

Ah, the SOS, reaction thing yep. Pushed changes, so one can specify type and then binary 0xff is for custom apps.

Docs changes are to reflect how it is currently in fw This adds ability to send datagram data to everyone in channel

ref:

liamcottle · 2026-03-06T08:23:16Z

examples/companion_radio/MyMesh.cpp

 #endif
 }

+void MyMesh::onChannelDataRecv(const mesh::GroupChannel &channel, mesh::Packet *pkt, uint32_t timestamp, uint8_t txt_type,


please refactor txt_type to data_type, since this isn't intended for text based data at this time.

liamcottle · 2026-03-06T08:24:23Z

examples/companion_radio/MyMesh.cpp

+                               const uint8_t *data, size_t data_len) {
+  int i = 0;
+  if (app_target_ver >= 3) {
+    out_frame[i++] = RESP_CODE_CHANNEL_MSG_RECV_V3;


we should probably use a new RESP_CODE_CHANNEL_DATA_RECV

liamcottle · 2026-03-06T08:25:31Z

examples/companion_radio/MyMesh.cpp

+  i += 4;
+
+  size_t available = MAX_FRAME_SIZE - i;
+  if (data_len > available) data_len = available;


I would probably prefer to have an error returned if provided data is too long, rather than truncating and sending anyway

liamcottle · 2026-03-06T08:27:16Z

examples/companion_radio/MyMesh.cpp

        writeOKFrame();
      } else {
-        writeErrFrame(ERR_CODE_NOT_FOUND); // bad channel_idx
+        writeErrFrame(ERR_CODE_TABLE_FULL);


we should probably revert all the changes made to CMD_SEND_CHANNEL_TXT_MSG as they're not relevant for the new CMD_SEND_CHANNEL_DATA. Changes to the error codes and lookup should be a separate PR :)

liamcottle · 2026-03-06T08:27:33Z

examples/companion_radio/MyMesh.cpp

    }
+  } else if (cmd_frame[0] == CMD_SEND_CHANNEL_DATA) { // send GroupChannel datagram
+    int i = 1;
+    uint8_t txt_type = cmd_frame[i++];


Please refactor txt_type to data_type

liamcottle · 2026-03-06T09:00:41Z

src/helpers/BaseChatMesh.cpp

+
+  auto pkt = createGroupDatagram(PAYLOAD_TYPE_GRP_DATA, channel, temp, 5 + data_len);
+  if (pkt) {
+    sendFloodScoped(channel, pkt);


currently group text messages are always flood routed to the whole network, I wonder if we should allow for the ability for group data to have an optional path supplied, to allow for targeted group data to a specific area via specific repeaters... maybe something for @ripplebiz to comment on?

I think as an optional param, not required. Or maybe hop based, so only allow one hop.But yea heavily depends on use case and how to prevent spamming.

Yeah definitely optional, just thinking it could be useful to be able to send some sort of flood advert locally, but without propagating to the entire network, since you could pick the direct path.

liamcottle · 2026-03-06T09:14:51Z

src/helpers/BaseChatMesh.h

  virtual uint32_t calcDirectTimeoutMillisFor(uint32_t pkt_airtime_millis, uint8_t path_len) const = 0;
  virtual void onSendTimeout() = 0;
  virtual void onChannelMessageRecv(const mesh::GroupChannel& channel, mesh::Packet* pkt, uint32_t timestamp, const char *text) = 0;
+  virtual void onChannelDataRecv(const mesh::GroupChannel& channel, mesh::Packet* pkt, uint32_t timestamp, uint8_t txt_type,


please change txt_type to data_type

liamcottle · 2026-03-06T09:15:08Z

src/helpers/BaseChatMesh.h

  int  sendMessage(const ContactInfo& recipient, uint32_t timestamp, uint8_t attempt, const char* text, uint32_t& expected_ack, uint32_t& est_timeout);
  int  sendCommandData(const ContactInfo& recipient, uint32_t timestamp, uint8_t attempt, const char* text, uint32_t& est_timeout);
  bool sendGroupMessage(uint32_t timestamp, mesh::GroupChannel& channel, const char* sender_name, const char* text, int text_len);
+  bool sendGroupData(uint32_t timestamp, mesh::GroupChannel& channel, uint8_t txt_type, const uint8_t* data, int data_len);


please change txt_type to data_type

liamcottle · 2026-03-06T09:15:49Z

src/helpers/TxtDataHelpers.h

 #define TXT_TYPE_PLAIN          0    // a plain text message
 #define TXT_TYPE_CLI_DATA       1    // a CLI command
 #define TXT_TYPE_SIGNED_PLAIN   2    // plain text, signed by sender
+#define TXT_TYPE_CUSTOM_BINARY  0xFF // custom app binary payload (group/channel datagrams)


since this will be a new type dedicated to group data, instead of group text, we should prob rename to DATA_TYPE_CUSTOM

liamcottle · 2026-03-06T09:18:25Z

src/helpers/BaseChatMesh.cpp

+  if (data_len < 0) return false;
+  // createGroupDatagram() accepts at most (MAX_PACKET_PAYLOAD - CIPHER_BLOCK_SIZE)
+  // plaintext bytes; subtract our 5-byte {timestamp, txt_type} header.
+  const int max_group_data_len = (MAX_PACKET_PAYLOAD - CIPHER_BLOCK_SIZE) - 5;


we should probably move max_group_data_len to a constant defined in Meshcore.h near MAX_PACKET_PAYLOAD. probably named MAX_GROUP_DATA_LENGTH

ref:

dz0ny · 2026-03-08T13:20:48Z

@liamcottle Should users be allowed to send packets on idx=0 (the public channel)? I think maybe not the 0xff type, but the others should be allowed. What do you think?

I know users want SOS, REACTION types, so those should be allowed. Just custom protocols probably not.

liamcottle

Looking good, thanks for your changes so far. Please see some extra comments.

liamcottle · 2026-03-09T07:50:18Z

docs/companion_protocol.md


-2. **Contact Messages**:
+2. **Channel Data**:
+   - `PACKET_CHANNEL_DATA_RECV` (0x1B) - Standard format


We can probably just have a single type PACKET_CHANNEL_DATA_RECV which also includes the SNR and reserved bytes. The reason we have two types for text message packets, is legacy reasons. We didn't have a way to add SNR data to existing response, because there was no data length byte. So we couldn't add it to the end of the payload.

liamcottle · 2026-03-09T07:51:01Z

docs/companion_protocol.md

 | 0x11  | PACKET_CHANNEL_MSG_RECV_V3 | Channel message (V3 with SNR) |
 | 0x12  | PACKET_CHANNEL_INFO        | Channel information           |
+| 0x1B  | PACKET_CHANNEL_DATA_RECV   | Channel data (standard)       |
+| 0x1C  | PACKET_CHANNEL_DATA_RECV_V3| Channel data (V3 with SNR)    |


Suggested to remove PACKET_CHANNEL_DATA_RECV_V3 and only have PACKET_CHANNEL_DATA_RECV. SNR would be included in PACKET_CHANNEL_DATA_RECV.

liamcottle · 2026-03-09T07:51:20Z

examples/companion_radio/MyMesh.cpp

 #define RESP_CODE_AUTOADD_CONFIG      25
 #define RESP_ALLOWED_REPEAT_FREQ      26
+#define RESP_CODE_CHANNEL_DATA_RECV   27
+#define RESP_CODE_CHANNEL_DATA_RECV_V3 28


No need for _V3 based on previous review comments

liamcottle · 2026-03-09T07:51:47Z

examples/companion_radio/MyMesh.cpp

  int i = 0;
  if (app_target_ver >= 3) {
-    out_frame[i++] = RESP_CODE_CHANNEL_MSG_RECV_V3;
+    out_frame[i++] = RESP_CODE_CHANNEL_DATA_RECV_V3;


Rename RESP_CODE_CHANNEL_DATA_RECV_V3 to RESP_CODE_CHANNEL_DATA_RECV.

liamcottle · 2026-03-09T07:52:30Z

examples/companion_radio/MyMesh.cpp

+void MyMesh::onChannelDataRecv(const mesh::GroupChannel &channel, mesh::Packet *pkt, uint32_t timestamp, uint8_t data_type,
                               const uint8_t *data, size_t data_len) {
  int i = 0;
  if (app_target_ver >= 3) {


No need for the target version check, since these are brand new request/response codes, which can be checked against a protocol version bump later on.

liamcottle · 2026-03-09T07:54:07Z

examples/companion_radio/MyMesh.cpp

+  }
  int copy_len = (int)data_len;
  if (copy_len > 0) {
    memcpy(&out_frame[i], data, copy_len);


I feel like we should also have a data_length byte that's in the payload so we can in a future update, add extra data at the end if there is room for it. This will help prevent the issue where we needed new packet format versions to add the SNR to txt messages.

liamcottle · 2026-03-09T08:01:47Z

src/helpers/BaseChatMesh.cpp

  memcpy(temp, &timestamp, 4);
-  temp[4] = txt_type;
+  temp[4] = data_type;
  if (data_len > 0) memcpy(&temp[5], data, data_len);


As mentioned in previous review comment, wondering if we should have a data_length byte as well.

This would allow adding extra data at the end later on, as clients shouldn't be reading more than data_length. This should also fix the issue where packets have extra zero padding passed back to the client, since the firmware has no idea what the actual data length is. This is a problem we currently have with cayenne lpp. Where there's extra zeros at the end, and clients have to do funky things to know to stop reading.

I would prefer to do this at the sendGroupData level, e.g temp[5] = data_len; otherwise we would have to add this to all other payload types. Probably best to just make it standard that we know the length up front.

liamcottle · 2026-03-09T08:04:11Z

Should users be allowed to send packets on idx=0 (the public channel)? I think maybe not the 0xff type, but the others should be allowed. What do you think?

I'll discuss this one with Scott.

liamcottle · 2026-03-09T08:22:14Z

We should probably go with uint16_t for data_type, instead of uint8_t so we have a larger pool of data types to hand out to the community for custom applications etc. ~256 might be too limiting, and we'd have to be careful how many IDs we give out. An ID could be given to an application/organisation/person, rather than a specific purpose, and the app author can embed their own internal type inside their data payload.

Copilot AI review requested due to automatic review settings March 5, 2026 12:34

Copilot started reviewing on behalf of dz0ny March 5, 2026 12:34 View session

Copilot AI reviewed Mar 5, 2026

View reviewed changes

dz0ny force-pushed the feat/grp-data-upstream branch from a54a351 to 965c40c Compare March 5, 2026 12:52

dz0ny added 2 commits March 5, 2026 14:04

feat: Add support for PAYLOAD_TYPE_GRP_DATA

a78bae9

Docs changes are to reflect how it is currently in fw This adds ability to send datagram data to everyone in channel

feat: Require 0xFF for custom payloads

d448c1e

ref:

dz0ny force-pushed the feat/grp-data-upstream branch from 965c40c to d448c1e Compare March 5, 2026 13:05

Merge branch 'main' into feat/grp-data-upstream

d67d308

liamcottle requested changes Mar 6, 2026

View reviewed changes

fix: address comments

cd7557d

ref:

liamcottle requested changes Mar 9, 2026

View reviewed changes

Conversation

dz0ny commented Mar 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

Uh oh!

liamcottle commented Mar 5, 2026

Uh oh!

dz0ny commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liamcottle Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dz0ny commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

liamcottle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liamcottle commented Mar 9, 2026

Uh oh!

liamcottle commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dz0ny commented Mar 5, 2026 •

edited

Loading

liamcottle Mar 6, 2026 •

edited

Loading

dz0ny commented Mar 8, 2026 •

edited

Loading

liamcottle commented Mar 9, 2026 •

edited

Loading