Implement C library for UART communication with CRC16 support and str… by vshcryabets · Pull Request #57 · vshcryabets/V2StyxLib

vshcryabets · 2026-03-07T11:20:26Z

…eaming mode

Copilot

Pull request overview

This PR introduces a small C “UART protocol” support layer (CRC16 + framing flags) and wires it into the C++ L4 module, extending the RP2040 USB-UART channel to optionally prepend SOF markers and include a CRC16 in the transmitted frame.

Changes:

Added a new C static library (module_l4_c) providing v2styxlib_crc16_calculate() and v2styxlib_uart_configure_proto().
Extended ChannelUsbUart to support streaming mode (SOF markers) and optional CRC16 transmission.
Updated CMake to build/link the new C library and to make Catch2 optional.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
cpp/modules/l4/src/styxpico/ChannelUsbUart.cpp	Adds SOF + CRC16 framing on transmit and uses new C helpers
cpp/modules/l4/include/styxpico/ChannelUsbUart.h	Extends constructor to configure streaming/CRC parameters
cpp/modules/l4/include/data.h	Switches `Size` typedef to use the C-layer `BufferSize_t`
cpp/modules/l4/CMakeLists.txt	Links `module_l4_c`, gates platform sources, makes tests optional
cpp/CMakeLists.txt	Adds `ENABLE_CATCH2` and builds the C submodule
c/l4/include/Channel_c.h	New shared UART protocol/CRC API + configuration flags
c/l4/src/Channel_c.c	Implements CRC16 + protocol flag configuration
c/l4/include/ChannelUartStm8.h	Switches STM8 UART headers to use the shared C header
c/l4/src/ChannelUartStm8.c	Updates STM8 UART send framing (CRC optional)
c/l4/CMakeLists.txt	Defines the `module_l4_c` static library
c/l4/README.md	Documents protocol flags/types (needs minor corrections)

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

Copilot · 2026-03-07T11:27:24Z

+if (ENABLE_CATCH2)
+    add_executable(module_l4_tests
+        test/test_dummy.cpp)
+    if (USE_STD_MEMORY)
+        target_compile_definitions(module_l4_tests PRIVATE USE_STD_MEMORY)
+    endif()
+    if(CMAKE_CXX_STANDARD GREATER_EQUAL 23)
+        message("Build tests for C++23")
+        target_sources(module_l4_tests PRIVATE
+            test/test_ChannelUnixTcp.cpp
+            test/test_ChannelUnixPipeImpl.cpp
+        )
+    endif()


module_l4_tests is only created inside this if (ENABLE_CATCH2) block. Ensure there are no target_sources(...) / target_compile_features(...) calls for module_l4_tests outside this block (currently the C++23 section above references module_l4_tests before it exists, which breaks configuration). Those calls should be moved into this block after add_executable(module_l4_tests ...).

Copilot · 2026-03-07T11:27:25Z

+        // write size header; if CRC16 is enabled, include 2 extra bytes for CRC
        uint8_t packetSizeBuffer[4] = {0};
+        Size headerPayloadSize = (config.config & V2STYXLIB_SEND_CRC16) ? size + 2 : size;
        SizeResult headerSize = setPacketSize(
-            packetSizeHeader, 
-            packetSizeBuffer, 
+            packetSizeHeader,
+            packetSizeBuffer,
            sizeof(packetSizeBuffer),
-            size);
+            headerPayloadSize);


headerPayloadSize is computed in the Size type and uses size + 2 when CRC is enabled. If Size is narrow (it currently aliases BufferSize_t), this can overflow/wrap and produce an invalid length in the header. Compute the header length in a wider type and return an error if size + 2 exceeds the representable range for the configured header size.

Copilot · 2026-03-07T11:27:25Z

+        if (config.config & V2STYXLIB_STREAMING_MODE) {
+            u16buffer[0] = sofMarker1;
+            u16buffer[1] = sofMarker2;
+            fwrite(u16buffer, 1, 2, stdout);
+        }


The fwrite calls for SOF markers / size header / CRC are not checked for short writes or errors. If any of these writes fail, the receiver will desynchronize but sendBuffer() will still report success; check the return value of each write and return an appropriate error (e.g., ErrorCode::SendFailed) on failure.

Copilot · 2026-03-07T11:27:25Z

@@ -0,0 +1,34 @@
+# Channel.h


The README title references Channel.h, but this module introduces Channel_c.h. Renaming the heading (and any references) to match the actual header name will avoid confusion for users trying to locate the documented definitions.

Suggested change

# Channel.h

# Channel_c.h

Copilot · 2026-03-07T11:27:25Z

+uint16_t v2styxlib_crc16_calculate(const uint8_t *data, BufferSize_t length) {
+    uint16_t crc = V2STYXLIB_CRC16_INITIAL_VALUE;
+    for (BufferSize_t i = 0; i < length; i++) {
+        crc = v2styxlib_crc16_update(crc, data[i]);
+    }
+    return crc;
+}


The CRC16 implementation and protocol bit configuration are new core behaviors but currently have no unit tests. Since v2styxlib_crc16_calculate() is a pure function, consider adding a Catch2 test with one or two known test vectors (and a small test for v2styxlib_uart_configure_proto() bit toggling) to prevent accidental CRC/polynomial regressions.

Copilot · 2026-03-07T11:27:26Z

+        // send packet size + 2 bytes for CRC16
+        UART1->DR = length + 2;
+        // then CRC16
+        uint16_t crc = v2styxlib_uart_crc16_calculate(buffer, length);


v2styxlib_uart_crc16_calculate is still referenced, but the implementation was removed/moved to Channel_c.c. This will fail to compile/link for the STM8 UART sender; call the new v2styxlib_crc16_calculate (or provide a compatibility wrapper) instead.

Suggested change

uint16_t crc = v2styxlib_uart_crc16_calculate(buffer, length);

uint16_t crc = v2styxlib_crc16_calculate(buffer, length);

Copilot · 2026-03-07T11:27:26Z

    using StyxDate = uint32_t;
    using StyxBuffer = uint8_t *;
-    using Size = uint16_t;
+    using Size = BufferSize_t; // Alias for BufferSize_t defined in Channel_c.h


Changing Size from uint16_t to BufferSize_t (defaults to uint8_t) effectively caps all payload sizes to 255 bytes across the entire C++ stack and makes the 2- and 4-byte packet header modes unable to represent larger sizes. This conflicts with existing Unix channel configurations that use iounit values like 8192 and will also make setPacketSize() comparisons/encoding incorrect; keep Size as a wider integer (e.g., uint16_t/uint32_t/size_t) and keep BufferSize_t limited to the C UART layer if needed.

Suggested change

using Size = BufferSize_t; // Alias for BufferSize_t defined in Channel_c.h

using Size = uint16_t; // Use a wider type; BufferSize_t remains for low-level UART

Copilot · 2026-03-07T11:27:26Z


        // write buffer
-        result = fwrite(buffer, 1, size, stdout);
+        ssize_t result = fwrite(buffer, 1, size, stdout);


fwrite() returns size_t, but the result is stored in ssize_t. On some toolchains (incl. embedded/newlib configurations) ssize_t may not be available without additional headers, and the signed conversion is unnecessary. Prefer size_t for the return value and handle the error/short-write case explicitly.

Suggested change

ssize_t result = fwrite(buffer, 1, size, stdout);

size_t result = fwrite(buffer, 1, size, stdout);

// explicitly handle potential short write or error

if (result < size) {

// partial write; caller will see the actual number of bytes written

}

Copilot · 2026-03-07T11:27:26Z

+When set, UART operates in streaming mode:
+- SOF (Start of Frame) markers are sent before each data frame.
+- The sender does not wait for responses between transmissions.
+


Channel_c.h introduces the V2STYXLIB_SEND_CRC16 flag, but the README currently documents streaming mode and SOF markers only. Add a short section describing V2STYXLIB_SEND_CRC16 (default value/bit, and how it changes the frame layout) to keep the documentation complete and consistent with the header.

Suggested change

## `V2STYXLIB_SEND_CRC16`

Flag bit indicating CRC16 is appended to each transmitted frame.

Default: `0x02`.

When set, a 2-byte CRC16 checksum (using `V2STYXLIB_CRC16_POLY` and `V2STYXLIB_CRC16_INITIAL_VALUE`) is added at the end of every frame, after the payload (and any header fields) and excluding any SOF markers. This extends the frame layout by two bytes and enables end-to-end error detection.

Implement C library for UART communication with CRC16 support and str…

2da96d7

…eaming mode

vshcryabets requested a review from Copilot March 7, 2026 11:20

vshcryabets self-assigned this Mar 7, 2026

Copilot started reviewing on behalf of vshcryabets March 7, 2026 11:20 View session

Copilot AI reviewed Mar 7, 2026

View reviewed changes

Fix comments

eed65af

vshcryabets merged commit 34dac0f into master Mar 7, 2026
1 check passed

vshcryabets deleted the feature/rp2040-c-lib branch March 7, 2026 12:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement C library for UART communication with CRC16 support and str…#57

Implement C library for UART communication with CRC16 support and str…#57
vshcryabets merged 2 commits into
masterfrom
feature/rp2040-c-lib

vshcryabets commented Mar 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Copilot AI Mar 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	uint16_t crc = v2styxlib_uart_crc16_calculate(buffer, length);
	uint16_t crc = v2styxlib_crc16_calculate(buffer, length);

	using Size = BufferSize_t; // Alias for BufferSize_t defined in Channel_c.h
	using Size = uint16_t; // Use a wider type; BufferSize_t remains for low-level UART

-        ssize_t result = fwrite(buffer, 1, size, stdout);
+        size_t result = fwrite(buffer, 1, size, stdout);
+        // explicitly handle potential short write or error
+        if (result < size) {
+            // partial write; caller will see the actual number of bytes written
+        }

+## `V2STYXLIB_SEND_CRC16`
+Flag bit indicating CRC16 is appended to each transmitted frame.
+Default: `0x02`.
+When set, a 2-byte CRC16 checksum (using `V2STYXLIB_CRC16_POLY` and `V2STYXLIB_CRC16_INITIAL_VALUE`) is added at the end of every frame, after the payload (and any header fields) and excluding any SOF markers. This extends the frame layout by two bytes and enables end-to-end error detection.

Conversation

vshcryabets commented Mar 7, 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 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants