feat: Add support for PAYLOAD_TYPE_GRP_DATA

[dz0ny]

• 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.

[liamcottle]

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]

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

[dz0ny]

@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]

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]

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.

[dz0ny]

Addressed the current review items in 2fe3c36

[dz0ny]

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.

Sounds a bit premature optimization. App author can wire their own type in paylaod and this would be the only such type while the protocol uses uint8_t.

[liamcottle]

old check was if len > 5 (disallowing exactly 5) and new logic is len < 5, which means exactly 5 is now allowed? Will this be problematic?

[liamcottle]

Addressed the current review items in 2fe3c36

Looking good! Thank you for the updates :)

[liamcottle]

Sounds a bit premature optimization. App author can wire their own type in paylaod and this would be the only such type while the protocol uses uint8_t.

After discussions with Scott, I think we should go with uint16_t so we can maintain a registry of unique app/developer/purpose IDs in the project files.

Meshtastic does similar with their portnums, which goes up to 500+ IDs. uint8_t would limit us to ~256.
https://github.com/meshtastic/protobufs/blob/master/meshtastic/portnums.proto

Ideally, in your MeshCore SAR app, the MeshCore project would allocate you a "unique ID" from the uint16_t ~65,535 possible types pool. And then you have free reign to allocate your own internal IDs for your own custom payload types in your raw payload data, without other custom applications causing you trouble.

It was noted that if everyone is using 0xFF for custom data, then you will end up with other applications sending unexpected data to your application. For example if you expect 0xFF (custom data) and the next byte to be 0x01 for "image data", someone using 0xFF for custom data in a different application would trigger your image handler if their first custom byte happens to be 0x01. So, we need to be able to separate other applications from each other.

I also think it would be beneficial to provide a larger pool of development/prototype ids, such as 0xFF00 - 0xFFFF, which would help to avoid these issues during testing. Once the project allocates a unique ID, you wouldn't have garbage data from other apps using the same development IDs.

I would suggest to go with the larger pool from day one. I'd expect we would eat up ~256 unique application/type ids pretty quickly at the rate we are seeing networks grow, and new custom applications come about.

One could argue that we could just use a custom channel key to segregate applications, such as "#meshcore-sar", but I can see usefulness in being able to send "well known" payload types to any channel. For example, we could have a new "share contact" payload, and allow that to be sent to any channel.

[ripplebiz]

Could you please change the base branch to 'dev'?

[ripplebiz]

Looks good, but I'd rather drop the enforced 'timestamp' field in all the methods (and the new companion protocol parts), and just have a data byte array (with explicit data_len). If various data_types need a timestamp they can just include one.
And, also widen data_type to uint16_t (and remove the concept of CUSTOM)

[dz0ny]

@ripplebiz changed to dev, you can merge now.