fix: address comments

ref:
2026-04-20 22:13:47 +00:00 · 2026-03-08 14:14:26 +01:00 · 2026-03-08 14:14:26 +01:00 · a21b83b127
commit a21b83b127
parent 0e98939987
7 changed files with 107 additions and 50 deletions
--- a/docs/companion_protocol.md
+++ b/docs/companion_protocol.md
@ -291,17 +291,21 @@ Bytes 7+: UTF-8 text bytes (variable length)
 **Command Format**:
 ```
 Byte 0: 0x3E
-Byte 1: Data Type (`txt_type`)
+Byte 1: Data Type (`data_type`)
 Byte 2: Channel Index (0-7)
 Bytes 3-6: Timestamp (32-bit little-endian Unix timestamp, seconds)
 Bytes 7+: Binary payload bytes (variable length)
 ```

 **Data Type / Transport Mapping**:
- `0xFF` (`TXT_TYPE_CUSTOM_BINARY`) must be used for custom-protocol binary datagrams.
+- `0xFF` (`DATA_TYPE_CUSTOM`) must be used for custom-protocol binary datagrams.
 - `0x00` (`TXT_TYPE_PLAIN`) is invalid for this command.
 - Values other than `0xFF` are reserved for official protocol extensions.

+**Limits**:
+- Maximum payload length is `163` bytes (`MAX_GROUP_DATA_LENGTH`).
+- Larger payloads are rejected with `PACKET_ERROR` / `ERR_CODE_ILLEGAL_ARG`.
+
 **Response**: `PACKET_OK` (0x00) on success

 ---
@ -322,6 +326,7 @@ Byte 0: 0x0A

 **Response**: 
 - `PACKET_CHANNEL_MSG_RECV` (0x08) or `PACKET_CHANNEL_MSG_RECV_V3` (0x11) for channel messages
+- `PACKET_CHANNEL_DATA_RECV` (0x1B) or `PACKET_CHANNEL_DATA_RECV_V3` (0x1C) for channel data
 - `PACKET_CONTACT_MSG_RECV` (0x07) or `PACKET_CONTACT_MSG_RECV_V3` (0x10) for contact messages
 - `PACKET_NO_MORE_MSGS` (0x0A) if no messages available

@ -391,11 +396,15 @@ Messages are received via the TX characteristic (notifications). The device send
   - `PACKET_CHANNEL_MSG_RECV` (0x08) - Standard format
   - `PACKET_CHANNEL_MSG_RECV_V3` (0x11) - Version 3 with SNR

-2. **Contact Messages**:
+2. **Channel Data**:
+   - `PACKET_CHANNEL_DATA_RECV` (0x1B) - Standard format
+   - `PACKET_CHANNEL_DATA_RECV_V3` (0x1C) - Version 3 with SNR
+
+3. **Contact Messages**:
   - `PACKET_CONTACT_MSG_RECV` (0x07) - Standard format
   - `PACKET_CONTACT_MSG_RECV_V3` (0x10) - Version 3 with SNR

-3. **Notifications**:
+4. **Notifications**:
   - `PACKET_MESSAGES_WAITING` (0x83) - Indicates messages are queued

 ### Contact Message Format
@ -489,37 +498,62 @@ Bytes 11+: Payload bytes
 **Payload Meaning**:
 - If `txt_type == 0x00`: payload is UTF-8 channel text.
 - If `txt_type != 0x00`: payload is binary (for example image/voice fragments) and must be treated as raw bytes.
-  For custom app datagrams sent via `CMD_SEND_CHANNEL_DATA`, `txt_type` must be `0xFF`.
+  For custom app datagrams sent via `CMD_SEND_CHANNEL_DATA`, `data_type` must be `0xFF`.
+
+### Channel Data Format
+
+**Standard Format** (`PACKET_CHANNEL_DATA_RECV`, 0x1B):
+```
+Byte 0: 0x1B (packet type)
+Byte 1: Channel Index (0-7)
+Byte 2: Path Length
+Byte 3: Data Type
+Bytes 4-7: Timestamp (32-bit little-endian)
+Bytes 8+: Payload bytes
+```
+
+**V3 Format** (`PACKET_CHANNEL_DATA_RECV_V3`, 0x1C):
+```
+Byte 0: 0x1C (packet type)
+Byte 1: SNR (signed byte, multiplied by 4)
+Bytes 2-3: Reserved
+Byte 4: Channel Index (0-7)
+Byte 5: Path Length
+Byte 6: Data Type
+Bytes 7-10: Timestamp (32-bit little-endian)
+Bytes 11+: Payload bytes
+```

 **Parsing Pseudocode**:
 ```python
-def parse_channel_message(data):
+def parse_channel_frame(data):
    packet_type = data[0]
    offset = 1
    
    # Check for V3 format
-    if packet_type == 0x11:  # V3
+    if packet_type in (0x11, 0x1C):  # V3
        snr_byte = data[offset]
        snr = ((snr_byte if snr_byte < 128 else snr_byte - 256) / 4.0)
        offset += 3  # Skip SNR + reserved
    
    channel_idx = data[offset]
    path_len = data[offset + 1]
-    txt_type = data[offset + 2]
+    item_type = data[offset + 2]
    timestamp = int.from_bytes(data[offset+3:offset+7], 'little')
    payload = data[offset+7:]
-    if txt_type == 0:
+    is_text = packet_type in (0x08, 0x11)
+    if is_text and item_type == 0:
        message = payload.decode('utf-8')
    else:
        message = None
    
    return {
        'channel_idx': channel_idx,
-        'txt_type': txt_type,
+        'item_type': item_type,
        'timestamp': timestamp,
        'payload': payload,
        'message': message,
-        'snr': snr if packet_type == 0x11 else None
+        'snr': snr if packet_type in (0x11, 0x1C) else None
    }
 ```

@ -556,6 +590,8 @@ Use `CMD_SEND_CHANNEL_TXT_MSG` for plain text, and `CMD_SEND_CHANNEL_DATA` for b
 | 0x10  | PACKET_CONTACT_MSG_RECV_V3 | Contact message (V3 with SNR) |
 | 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)    |
 | 0x80  | PACKET_ADVERTISEMENT       | Advertisement packet          |
 | 0x82  | PACKET_ACK                 | Acknowledgment                |
 | 0x83  | PACKET_MESSAGES_WAITING    | Messages waiting notification |
@ -775,7 +811,7 @@ BLE implementations enqueue and deliver one protocol frame per BLE write/notific
     - `SET_CHANNEL` → `PACKET_OK` or `PACKET_ERROR`
     - `CMD_SEND_CHANNEL_TXT_MSG` → `PACKET_OK` or `PACKET_ERROR`
     - `CMD_SEND_CHANNEL_DATA` → `PACKET_OK` or `PACKET_ERROR`
-     - `GET_MESSAGE` → `PACKET_CHANNEL_MSG_RECV`, `PACKET_CONTACT_MSG_RECV`, or `PACKET_NO_MORE_MSGS`
+     - `GET_MESSAGE` → `PACKET_CHANNEL_MSG_RECV`, `PACKET_CHANNEL_DATA_RECV`, `PACKET_CONTACT_MSG_RECV`, or `PACKET_NO_MORE_MSGS`
     - `GET_BATTERY` → `PACKET_BATTERY`

 4. **Timeout Handling**:
@ -855,8 +891,9 @@ response = wait_for_response(PACKET_OK)
 def on_notification_received(data):
    packet_type = data[0]
    
-    if packet_type == PACKET_CHANNEL_MSG_RECV or packet_type == PACKET_CHANNEL_MSG_RECV_V3:
-        message = parse_channel_message(data)
+    if packet_type in (PACKET_CHANNEL_MSG_RECV, PACKET_CHANNEL_MSG_RECV_V3,
+                       PACKET_CHANNEL_DATA_RECV, PACKET_CHANNEL_DATA_RECV_V3):
+        message = parse_channel_frame(data)
        handle_channel_message(message)
    elif packet_type == PACKET_MESSAGES_WAITING:
        # Poll for messages
--- a/examples/companion_radio/MyMesh.cpp
+++ b/examples/companion_radio/MyMesh.cpp
@ -92,6 +92,8 @@
 #define RESP_CODE_STATS               24   // v8+, second byte is stats type
 #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

 #define SEND_TIMEOUT_BASE_MILLIS        500
 #define FLOOD_SEND_TIMEOUT_FACTOR       16.0f
@ -205,7 +207,8 @@ void MyMesh::updateContactFromFrame(ContactInfo &contact, uint32_t& last_mod, co
 }

 bool MyMesh::Frame::isChannelMsg() const {
-  return buf[0] == RESP_CODE_CHANNEL_MSG_RECV || buf[0] == RESP_CODE_CHANNEL_MSG_RECV_V3;
+  return buf[0] == RESP_CODE_CHANNEL_MSG_RECV || buf[0] == RESP_CODE_CHANNEL_MSG_RECV_V3 ||
+         buf[0] == RESP_CODE_CHANNEL_DATA_RECV || buf[0] == RESP_CODE_CHANNEL_DATA_RECV_V3;
 }

 void MyMesh::addToOfflineQueue(const uint8_t frame[], int len) {
@ -565,27 +568,30 @@ void MyMesh::onChannelMessageRecv(const mesh::GroupChannel &channel, mesh::Packe
 #endif
 }

-void MyMesh::onChannelDataRecv(const mesh::GroupChannel &channel, mesh::Packet *pkt, uint32_t timestamp, uint8_t txt_type,
+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) {
-    out_frame[i++] = RESP_CODE_CHANNEL_MSG_RECV_V3;
+    out_frame[i++] = RESP_CODE_CHANNEL_DATA_RECV_V3;
    out_frame[i++] = (int8_t)(pkt->getSNR() * 4);
    out_frame[i++] = 0; // reserved1
    out_frame[i++] = 0; // reserved2
  } else {
-    out_frame[i++] = RESP_CODE_CHANNEL_MSG_RECV;
+    out_frame[i++] = RESP_CODE_CHANNEL_DATA_RECV;
  }

  uint8_t channel_idx = findChannelIdx(channel);
  out_frame[i++] = channel_idx;
  out_frame[i++] = pkt->isRouteFlood() ? pkt->path_len : 0xFF;
-  out_frame[i++] = txt_type;
+  out_frame[i++] = data_type;
  memcpy(&out_frame[i], &timestamp, 4);
  i += 4;

  size_t available = MAX_FRAME_SIZE - i;
-  if (data_len > available) data_len = available;
+  if (data_len > available) {
+    MESH_DEBUG_PRINTLN("onChannelDataRecv(): payload_len=%d exceeds frame space=%d, truncating", (uint32_t)data_len, (uint32_t)available);
+    data_len = available;
+  }
  int copy_len = (int)data_len;
  if (copy_len > 0) {
    memcpy(&out_frame[i], data, copy_len);
@ -1069,29 +1075,27 @@ void MyMesh::handleCmdFrame(size_t len) {
    }
  } else if (cmd_frame[0] == CMD_SEND_CHANNEL_TXT_MSG) { // send GroupChannel text msg
    int i = 1;
-    uint8_t txt_type = cmd_frame[i++];
+    uint8_t txt_type = cmd_frame[i++]; // should be TXT_TYPE_PLAIN
    uint8_t channel_idx = cmd_frame[i++];
    uint32_t msg_timestamp;
    memcpy(&msg_timestamp, &cmd_frame[i], 4);
    i += 4;
    const char *text = (char *)&cmd_frame[i];
-    int text_len = (len > (size_t)i) ? (int)(len - i) : 0;

    if (txt_type != TXT_TYPE_PLAIN) {
      writeErrFrame(ERR_CODE_UNSUPPORTED_CMD);
    } else {
      ChannelDetails channel;
-      if (!getChannel(channel_idx, channel)) {
-        writeErrFrame(ERR_CODE_NOT_FOUND); // bad channel_idx
-      } else if (sendGroupMessage(msg_timestamp, channel.channel, _prefs.node_name, text, text_len)) {
+      bool success = getChannel(channel_idx, channel);
+      if (success && sendGroupMessage(msg_timestamp, channel.channel, _prefs.node_name, text, len - i)) {
        writeOKFrame();
      } else {
-        writeErrFrame(ERR_CODE_TABLE_FULL);
+        writeErrFrame(ERR_CODE_NOT_FOUND); // bad channel_idx
      }
    }
  } else if (cmd_frame[0] == CMD_SEND_CHANNEL_DATA) { // send GroupChannel datagram
    int i = 1;
-    uint8_t txt_type = cmd_frame[i++];
+    uint8_t data_type = cmd_frame[i++];
    uint8_t channel_idx = cmd_frame[i++];
    uint32_t msg_timestamp;
    memcpy(&msg_timestamp, &cmd_frame[i], 4);
@ -1102,9 +1106,12 @@ void MyMesh::handleCmdFrame(size_t len) {
    ChannelDetails channel;
    if (!getChannel(channel_idx, channel)) {
      writeErrFrame(ERR_CODE_NOT_FOUND); // bad channel_idx
-    } else if (txt_type != TXT_TYPE_CUSTOM_BINARY) {
+    } else if (data_type != DATA_TYPE_CUSTOM) {
      writeErrFrame(ERR_CODE_UNSUPPORTED_CMD);
-    } else if (sendGroupData(msg_timestamp, channel.channel, txt_type, payload, payload_len)) {
+    } else if (payload_len > MAX_GROUP_DATA_LENGTH) {
+      MESH_DEBUG_PRINTLN("CMD_SEND_CHANNEL_DATA payload too long: %d > %d", payload_len, MAX_GROUP_DATA_LENGTH);
+      writeErrFrame(ERR_CODE_ILLEGAL_ARG);
+    } else if (sendGroupData(msg_timestamp, channel.channel, data_type, payload, payload_len)) {
      writeOKFrame();
    } else {
      writeErrFrame(ERR_CODE_TABLE_FULL);
--- a/examples/companion_radio/MyMesh.h
+++ b/examples/companion_radio/MyMesh.h
@ -137,7 +137,7 @@ protected:
                           const uint8_t *sender_prefix, const char *text) override;
  void onChannelMessageRecv(const mesh::GroupChannel &channel, mesh::Packet *pkt, uint32_t timestamp,
                            const char *text) override;
-  void onChannelDataRecv(const mesh::GroupChannel &channel, mesh::Packet *pkt, uint32_t timestamp, uint8_t txt_type,
+  void onChannelDataRecv(const mesh::GroupChannel &channel, mesh::Packet *pkt, uint32_t timestamp, uint8_t data_type,
                         const uint8_t *data, size_t data_len) override;

  uint8_t onContactRequest(const ContactInfo &contact, uint32_t sender_timestamp, const uint8_t *data,
--- a/src/MeshCore.h
+++ b/src/MeshCore.h
@ -17,6 +17,7 @@
 #define PATH_HASH_SIZE       1

 #define MAX_PACKET_PAYLOAD  184
+#define MAX_GROUP_DATA_LENGTH  (MAX_PACKET_PAYLOAD - CIPHER_BLOCK_SIZE - 5)
 #define MAX_PATH_SIZE        64
 #define MAX_TRANS_UNIT      255

@ -100,4 +101,4 @@ public:
  }
 };

-}
+}
--- a/src/helpers/BaseChatMesh.cpp
+++ b/src/helpers/BaseChatMesh.cpp
@ -353,10 +353,18 @@ int BaseChatMesh::searchChannelsByHash(const uint8_t* hash, mesh::GroupChannel d
 #endif

 void BaseChatMesh::onGroupDataRecv(mesh::Packet* packet, uint8_t type, const mesh::GroupChannel& channel, uint8_t* data, size_t len) {
-  if (len < 5) return;
+  if (len < 5) {
+    MESH_DEBUG_PRINTLN("onGroupDataRecv: dropping short group payload len=%d", (uint32_t)len);
+    return;
+  }
+
+  uint8_t data_type = data[4];
+  if (type == PAYLOAD_TYPE_GRP_TXT) {
+    if ((data_type >> 2) != 0) {
+      MESH_DEBUG_PRINTLN("onGroupDataRecv: dropping unsupported group text type=%d", (uint32_t)data_type);
+      return;
+    }

-  uint8_t txt_type = data[4];
-  if (type == PAYLOAD_TYPE_GRP_TXT && (txt_type >> 2) == 0) {  // 0 = plain text msg
    uint32_t timestamp;
    memcpy(&timestamp, data, 4);

@ -368,7 +376,7 @@ void BaseChatMesh::onGroupDataRecv(mesh::Packet* packet, uint8_t type, const mes
  } else if (type == PAYLOAD_TYPE_GRP_DATA) {
    uint32_t timestamp;
    memcpy(&timestamp, data, 4);
-    onChannelDataRecv(channel, packet, timestamp, txt_type, &data[5], len - 5);
+    onChannelDataRecv(channel, packet, timestamp, data_type, &data[5], len - 5);
  }
 }

@ -460,24 +468,28 @@ bool BaseChatMesh::sendGroupMessage(uint32_t timestamp, mesh::GroupChannel& chan
  return false;
 }

-bool BaseChatMesh::sendGroupData(uint32_t timestamp, mesh::GroupChannel& channel, uint8_t txt_type, const uint8_t* data, int data_len) {
-  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;
-  if (data_len > max_group_data_len) data_len = max_group_data_len;
+bool BaseChatMesh::sendGroupData(uint32_t timestamp, mesh::GroupChannel& channel, uint8_t data_type, const uint8_t* data, int data_len) {
+  if (data_len < 0) {
+    MESH_DEBUG_PRINTLN("sendGroupData: invalid negative data_len=%d", data_len);
+    return false;
+  }
+  if (data_len > MAX_GROUP_DATA_LENGTH) {
+    MESH_DEBUG_PRINTLN("sendGroupData: data_len=%d exceeds max=%d", data_len, MAX_GROUP_DATA_LENGTH);
+    return false;
+  }

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

  auto pkt = createGroupDatagram(PAYLOAD_TYPE_GRP_DATA, channel, temp, 5 + data_len);
-  if (pkt) {
-    sendFloodScoped(channel, pkt);
-    return true;
+  if (pkt == NULL) {
+    MESH_DEBUG_PRINTLN("sendGroupData: unable to create group datagram, data_len=%d", data_len);
+    return false;
  }
-  return false;
+  sendFloodScoped(channel, pkt);
+  return true;
 }

 bool BaseChatMesh::shareContactZeroHop(const ContactInfo& contact) {
--- a/src/helpers/BaseChatMesh.h
+++ b/src/helpers/BaseChatMesh.h
@ -111,7 +111,7 @@ protected:
  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,
+  virtual void onChannelDataRecv(const mesh::GroupChannel& channel, mesh::Packet* pkt, uint32_t timestamp, uint8_t data_type,
                                 const uint8_t* data, size_t data_len) {}
  virtual uint8_t onContactRequest(const ContactInfo& contact, uint32_t sender_timestamp, const uint8_t* data, uint8_t len, uint8_t* reply) = 0;
  virtual void onContactResponse(const ContactInfo& contact, const uint8_t* data, uint8_t len) = 0;
@ -150,7 +150,7 @@ public:
  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);
+  bool sendGroupData(uint32_t timestamp, mesh::GroupChannel& channel, uint8_t data_type, const uint8_t* data, int data_len);
  int  sendLogin(const ContactInfo& recipient, const char* password, uint32_t& est_timeout);
  int  sendAnonReq(const ContactInfo& recipient, const uint8_t* data, uint8_t len, uint32_t& tag, uint32_t& est_timeout);
  int  sendRequest(const ContactInfo& recipient, uint8_t req_type, uint32_t& tag, uint32_t& est_timeout);
--- a/src/helpers/TxtDataHelpers.h
+++ b/src/helpers/TxtDataHelpers.h
@ -6,7 +6,7 @@
 #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)
+#define DATA_TYPE_CUSTOM        0xFF // custom app binary payload (group/channel datagrams)

 class StrHelper {
 public: