Update and improve serial protocol (#823)

* Update serial protocol

* Remove buffers reusage to avoid conflicts

.github/workflows/arduino_esp32.yaml

@@ -51,7 +51,7 @@ jobs:
 
           mkdir -p $ARDUINO_BASE
           cd $ARDUINO_BASE
-          platformio init -b esp32thing_plus -O "board_build.cmake_extra_args=-DZ_FEATURE_LINK_BLUETOOTH=1" -O "build_flags=-DZENOH_DEBUG=3 -DZENOH_COMPILER_GCC" -O "lib_ldf_mode=deep+"
+          platformio init -b esp32thing_plus -O "board_build.cmake_extra_args=-DZ_FEATURE_LINK_BLUETOOTH=1 -DZ_FEATURE_LINK_SERIAL=1" -O "build_flags=-DZENOH_DEBUG=3 -DZENOH_COMPILER_GCC" -O "lib_ldf_mode=deep+"
 
           cd $ARDUINO_BASE/lib
           ln -s $ZENOH_PICO_BASE

.github/workflows/espidf.yaml

@@ -51,7 +51,7 @@ jobs:
 
           mkdir -p $ESPIDF_BASE
           cd $ESPIDF_BASE
-          platformio init -b az-delivery-devkit-v4 --project-option="framework=espidf" --project-option="build_flags=-DZENOH_ESPIDF -DZENOH_DEBUG=3"
+          platformio init -b az-delivery-devkit-v4 -O "board_build.cmake_extra_args=-DZ_FEATURE_LINK_SERIAL=1" --project-option="framework=espidf" --project-option="build_flags=-DZENOH_ESPIDF -DZENOH_DEBUG=3"
 
           cd $ESPIDF_BASE/lib
           ln -s $ZENOH_PICO_BASE

CMakeLists.txt

@@ -343,7 +343,7 @@ file(GLOB_RECURSE Sources
   "src/session/*.c"
   "src/transport/*.c"
   "src/utils/*.c"
-  "src/system/platform_common.c"
+  "src/system/common/*.c"
 )
 
 if(WITH_ZEPHYR)

examples/rpi_pico/main.c

@@ -44,7 +44,10 @@ void print_ip_address() {
 
 void main_task(void *params) {
     (void)params;
+#ifndef NDEBUG
     vTaskDelay(pdMS_TO_TICKS(3000));
+#endif
+
 #if WIFI_SUPPORT_ENABLED
     if (cyw43_arch_init()) {
         printf("Failed to initialise\n");

include/zenoh-pico/collections/arc_slice.h

@@ -22,7 +22,7 @@
 
 #include "refcount.h"
 #include "slice.h"
-#include "zenoh-pico/system/platform_common.h"
+#include "zenoh-pico/system/common/platform.h"
 
 #ifdef __cplusplus
 extern "C" {

include/zenoh-pico/protocol/codec/serial.h

@@ -0,0 +1,33 @@
+//
+// Copyright (c) 2024 ZettaScale Technology
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License 2.0 which is available at
+// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+//
+// Contributors:
+//   ZettaScale Zenoh Team, <[email protected]>
+//
+
+#ifndef INCLUDE_ZENOH_PICO_PROTOCOL_CODEC_SERIAL_H
+#define INCLUDE_ZENOH_PICO_PROTOCOL_CODEC_SERIAL_H
+
+#include <stdint.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+size_t _z_serial_msg_serialize(uint8_t *dest, size_t dest_len, const uint8_t *src, size_t src_len, uint8_t header,
+                               uint8_t *tmp_buf, size_t tmp_buf_len);
+size_t _z_serial_msg_deserialize(const uint8_t *src, size_t src_len, uint8_t *dst, size_t dst_len, uint8_t *header,
+                                 uint8_t *tmp_buf, size_t tmp_buf_len);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* INCLUDE_ZENOH_PICO_PROTOCOL_CODEC_SERIAL_H */

include/zenoh-pico/protocol/definitions/serial.h

@@ -0,0 +1,61 @@
+//
+// Copyright (c) 2022 ZettaScale Technology
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License 2.0 which is available at
+// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+//
+// Contributors:
+//   ZettaScale Zenoh Team, <[email protected]>
+//
+
+#ifndef INCLUDE_ZENOH_PICO_PROTOCOL_DEFINITIONS_SERIAL_H
+#define INCLUDE_ZENOH_PICO_PROTOCOL_DEFINITIONS_SERIAL_H
+
+#include <stdint.h>
+
+#include "zenoh-pico/link/endpoint.h"
+#include "zenoh-pico/protocol/definitions/network.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/// ZSerial Frame Format
+///
+/// Using COBS
+///
+/// +-+-+----+------------+--------+-+
+/// |O|H|XXXX|ZZZZ....ZZZZ|CCCCCCCC|0|
+/// +-+----+------------+--------+-+
+/// |O| |Len |   Data     |  CRC32 |C|
+/// +-+-+-2--+----N-------+---4----+-+
+///
+/// Header: 1byte
+/// +---------------+
+/// |7|6|5|4|3|2|1|0|
+/// +---------------+
+/// |x|x|x|x|x|R|A|I|
+/// +---------------+
+///
+/// Flags:
+/// I - Init
+/// A - Ack
+/// R - Reset
+///
+/// Max Frame Size: 1510
+/// Max MTU: 1500
+/// Max On-the-wire length: 1516 (MFS + Overhead Byte (OHB) + Kind Byte + End of packet (EOP))
+
+#define _Z_FLAG_SERIAL_INIT 0x01
+#define _Z_FLAG_SERIAL_ACK 0x02
+#define _Z_FLAG_SERIAL_RESET 0x04
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* INCLUDE_ZENOH_PICO_PROTOCOL_DEFINITIONS_SERIAL_H*/

include/zenoh-pico/system/common/serial.h

@@ -0,0 +1,36 @@
+//
+// Copyright (c) 2024 ZettaScale Technology
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License 2.0 which is available at
+// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+//
+// Contributors:
+//   ZettaScale Zenoh Team, <[email protected]>
+//
+
+#ifndef ZENOH_PICO_SYSTEM_COMMON_SERIAL_H
+#define ZENOH_PICO_SYSTEM_COMMON_SERIAL_H
+
+#include <stdint.h>
+
+#include "zenoh-pico/system/common/platform.h"
+#include "zenoh-pico/utils/result.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+z_result_t _z_connect_serial(const _z_sys_net_socket_t sock);
+size_t _z_read_serial(const _z_sys_net_socket_t sock, uint8_t *ptr, size_t len);
+size_t _z_send_serial(const _z_sys_net_socket_t sock, const uint8_t *ptr, size_t len);
+size_t _z_read_exact_serial(const _z_sys_net_socket_t sock, uint8_t *ptr, size_t len);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* ZENOH_PICO_SYSTEM_COMMON_SERIAL_H */

include/zenoh-pico/system/link/serial.h

@@ -28,7 +28,7 @@ extern "C" {
 #if Z_FEATURE_LINK_SERIAL == 1
 
 #define _Z_SERIAL_MTU_SIZE 1500
-#define _Z_SERIAL_MFS_SIZE _Z_SERIAL_MTU_SIZE + 2 + 4  // MTU + Serial Len + Serial CRC32
+#define _Z_SERIAL_MFS_SIZE _Z_SERIAL_MTU_SIZE + 1 + 2 + 4  // MTU + Header + Serial Len + Serial CRC32
 #define _Z_SERIAL_MAX_COBS_BUF_SIZE \
     1516  // Max On-the-wire length for an MFS/MTU of 1510/1500 (MFS + Overhead Byte (OHB) + End of packet (EOP))
 
@@ -42,8 +42,8 @@ z_result_t _z_listen_serial_from_pins(_z_sys_net_socket_t *sock, uint32_t txpin,
 z_result_t _z_listen_serial_from_dev(_z_sys_net_socket_t *sock, char *dev, uint32_t baudrate);
 void _z_close_serial(_z_sys_net_socket_t *sock);
 size_t _z_read_exact_serial(const _z_sys_net_socket_t sock, uint8_t *ptr, size_t len);
-size_t _z_read_serial(const _z_sys_net_socket_t sock, uint8_t *ptr, size_t len);
-size_t _z_send_serial(const _z_sys_net_socket_t sock, const uint8_t *ptr, size_t len);
+size_t _z_read_serial_internal(const _z_sys_net_socket_t sock, uint8_t *header, uint8_t *ptr, size_t len);
+size_t _z_send_serial_internal(const _z_sys_net_socket_t sock, uint8_t header, const uint8_t *ptr, size_t len);
 
 #endif
 

include/zenoh-pico/system/platform.h

@@ -18,6 +18,6 @@
 #include <stdint.h>
 
 #include "zenoh-pico/config.h"
-#include "zenoh-pico/system/platform_common.h"
+#include "zenoh-pico/system/common/platform.h"
 
 #endif /* ZENOH_PICO_SYSTEM_PLATFORM_H */

include/zenoh-pico/system/platform/espidf.h

@@ -56,11 +56,7 @@ typedef struct {
         int _fd;
 #endif
 #if Z_FEATURE_LINK_SERIAL == 1
-        struct {
-            uart_port_t _serial;
-            uint8_t *before_cobs;
-            uint8_t *after_cobs;
-        };
+        uart_port_t _serial;
 #endif
     };
 } _z_sys_net_socket_t;

include/zenoh-pico/utils/logging.h

@@ -17,7 +17,7 @@
 
 #include <stdio.h>
 
-#include "zenoh-pico/system/platform_common.h"
+#include "zenoh-pico/system/common/platform.h"
 
 #ifdef __cplusplus
 extern "C" {

src/api/api.c

@@ -37,8 +37,8 @@
 #include "zenoh-pico/session/resource.h"
 #include "zenoh-pico/session/subscription.h"
 #include "zenoh-pico/session/utils.h"
+#include "zenoh-pico/system/common/platform.h"
 #include "zenoh-pico/system/platform.h"
-#include "zenoh-pico/system/platform_common.h"
 #include "zenoh-pico/transport/common/tx.h"
 #include "zenoh-pico/transport/multicast.h"
 #include "zenoh-pico/transport/unicast.h"

src/link/unicast/serial.c

@@ -20,6 +20,7 @@
 
 #include "zenoh-pico/config.h"
 #include "zenoh-pico/link/manager.h"
+#include "zenoh-pico/system/common/serial.h"
 #include "zenoh-pico/system/link/serial.h"
 #include "zenoh-pico/utils/pointers.h"
 

src/protocol/codec/serial.c

@@ -0,0 +1,117 @@
+//
+// Copyright (c) 2024 ZettaScale Technology
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License 2.0 which is available at
+// http://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
+// which is available at https://www.apache.org/licenses/LICENSE-2.0.
+//
+// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
+//
+// Contributors:
+//   ZettaScale Zenoh Team, <[email protected]>
+//
+
+#include <inttypes.h>
+#include <stdint.h>
+#include <string.h>
+
+#include "zenoh-pico/utils/checksum.h"
+#include "zenoh-pico/utils/encoding.h"
+#include "zenoh-pico/utils/logging.h"
+#include "zenoh-pico/utils/pointers.h"
+
+#define KIND_FIELD_LEN 1u
+#define LEN_FIELD_LEN 2u
+#define CRC32_LEN 4u
+
+size_t _z_serial_msg_serialize(uint8_t *dest, size_t dest_len, const uint8_t *src, size_t src_len, uint8_t header,
+                               uint8_t *tmp_buf, size_t tmp_buf_len) {
+    size_t expected_size = src_len + KIND_FIELD_LEN + LEN_FIELD_LEN + CRC32_LEN;
+    if (tmp_buf_len < expected_size) {
+        _Z_DEBUG("tmp buffer too small: %zu < %zu", tmp_buf_len, expected_size);
+        return SIZE_MAX;
+    }
+
+    uint32_t crc32 = _z_crc32(src, src_len);
+    uint8_t crc_bytes[CRC32_LEN] = {(uint8_t)(crc32 & 0xFF), (uint8_t)((crc32 >> 8) & 0xFF),
+                                    (uint8_t)((crc32 >> 16) & 0xFF), (uint8_t)((crc32 >> 24) & 0xFF)};
+
+    uint16_t wire_size = (uint16_t)src_len;
+    uint8_t size_bytes[LEN_FIELD_LEN] = {(uint8_t)(wire_size & 0xFF), (uint8_t)((wire_size >> 8) & 0xFF)};
+
+    uint8_t *tmp_buf_ptr = tmp_buf;
+
+    tmp_buf_ptr[0] = header;
+    tmp_buf_ptr += sizeof(header);
+
+    memcpy(tmp_buf_ptr, size_bytes, sizeof(size_bytes));
+    tmp_buf_ptr += sizeof(size_bytes);
+
+    memcpy(tmp_buf_ptr, src, src_len);
+    tmp_buf_ptr += src_len;
+
+    memcpy(tmp_buf_ptr, crc_bytes, sizeof(crc_bytes));
+    tmp_buf_ptr += sizeof(crc_bytes);
+
+    size_t total_len = _z_ptr_u8_diff(tmp_buf_ptr, tmp_buf);
+
+    size_t ret = _z_cobs_encode(tmp_buf, total_len, dest);
+    if (ret + 1 > dest_len) {
+        _Z_DEBUG("destination buffer too small");
+        return SIZE_MAX;
+    }
+
+    dest[ret] = 0x00;
+
+    return ret + 1u;
+}
+
+size_t _z_serial_msg_deserialize(const uint8_t *src, size_t src_len, uint8_t *dst, size_t dst_len, uint8_t *header,
+                                 uint8_t *tmp_buf, size_t tmp_buf_len) {
+    if (tmp_buf_len < src_len) {
+        _Z_DEBUG("tmp_buf too small");
+        return SIZE_MAX;
+    }
+
+    size_t decoded_size = _z_cobs_decode(src, src_len, tmp_buf);
+
+    if (decoded_size < KIND_FIELD_LEN + LEN_FIELD_LEN + CRC32_LEN) {
+        _Z_DEBUG("decoded frame too small");
+        return SIZE_MAX;
+    }
+
+    uint8_t *tmp_buf_ptr = tmp_buf;
+
+    *header = tmp_buf_ptr[0];
+    tmp_buf_ptr += sizeof(uint8_t);
+
+    uint16_t wire_size = tmp_buf_ptr[0] | (tmp_buf_ptr[1] << 8);
+    tmp_buf_ptr += sizeof(uint16_t);
+
+    size_t expected_size = wire_size + KIND_FIELD_LEN + LEN_FIELD_LEN + CRC32_LEN;
+    if (expected_size != decoded_size) {
+        _Z_DEBUG("wire size mismatch: %zu != %zu", expected_size, decoded_size);
+        return SIZE_MAX;
+    }
+
+    if (dst_len < wire_size) {
+        _Z_DEBUG("destination buffer too small: %zu < %u", dst_len, wire_size);
+        return SIZE_MAX;
+    }
+
+    if (wire_size != 0) {
+        memcpy(dst, tmp_buf_ptr, wire_size);
+        tmp_buf_ptr += wire_size;
+    }
+
+    uint32_t received_crc = tmp_buf_ptr[0] | (tmp_buf_ptr[1] << 8) | (tmp_buf_ptr[2] << 16) | (tmp_buf_ptr[3] << 24);
+
+    uint32_t computed_crc = _z_crc32(dst, wire_size);
+    if (received_crc != computed_crc) {
+        _Z_DEBUG("CRC mismatch. Received: 0x%08" PRIu32 ", Computed: 0x%08" PRIu32, received_crc, computed_crc);
+        return SIZE_MAX;
+    }
+
+    return wire_size;
+}