Feat/add uart support #25

Dzarda7 · 2025-11-19T08:14:11Z

Description

This PR adds support for UART reception and processing of SLIP command. It was tested and works on the following targets:

esp32 v3.1
esp32c2 v2.0 (v1.0 does not work!)
esp32c3 v1.0
esp32c6 v0.1
esp32h2 v0.1
esp32s3 v0.2
esp32s2 v1.0
esp32c5 v1.0 and v1.1
esp32p4 v1.2 (need new target for >=v3.0)

Along the way, other stuff was fixed and resolved to enable this like not linking standard libraries which caused build with warnings for newer targets. Erase of bss section and account for checksum in SLIP header.

Testing

Tested on mentioned targets using esptool flash-id command. Works, some targets fail because flash chip is not attached, but stub responds correctly.

Checklist

Before submitting a Pull Request, please ensure the following:

🚨 This PR does not introduce breaking changes.
All CI checks (GH Actions) pass.
Documentation is updated as needed.
Tests are updated or added as necessary.
Code is well-commented, especially in complex areas.
Git history is clean — commits are squashed to the minimum necessary.

github-actions · 2025-11-19T08:14:45Z

	Messages
📖	You might consider squashing your 4 commits (simplifying branch history).

👋 Hello Dzarda7, we appreciate your contribution to this project!

Click to see more instructions ...

This automated output is generated by the PR linter DangerJS, which checks if your Pull Request meets the project's requirements and helps you fix potential issues.

DangerJS is triggered with each push event to a Pull Request and modify the contents of this comment.

Please consider the following:
- Danger mainly focuses on the PR structure and formatting and can't understand the meaning behind your code or changes.
- Danger is not a substitute for human code reviews; it's still important to request a code review from your colleagues.
- Addressing info messages (📖) is strongly recommended; they're less critical but valuable.
- To manually retry these Danger checks, please navigate to the Actions tab and re-run last Danger workflow.

Review and merge process you can expect ...

We do welcome contributions in the form of bug reports, feature requests and pull requests.

1. An internal issue has been created for the PR, we assign it to the relevant engineer.
2. They review the PR and either approve it or ask you for changes or clarifications.
3. Once the GitHub PR is approved we do the final review, collect approvals from core owners and make sure all the automated tests are passing.
- At this point we may do some adjustments to the proposed change, or extend it by adding tests or documentation.
4. If the change is approved and passes the tests it is merged into the default branch.

Generated by 🚫 dangerJS against aea8a71

Dzarda7 · 2025-11-19T10:01:40Z

@dobairoland @radimkarnis @jedi7 This is working implementation for UART. I do not expect many changes in the API on esp-stub-lib side, but it waits for merging as some things needs to be decided. But this can be used for testing. Also review can be done I believe.

jedi7 · 2025-11-19T11:02:51Z

src/main.c

+    if ((intr_flags & UART_INTR_RXFIFO_FULL) || (intr_flags & UART_INTR_RXFIFO_TOUT)) {
+        uint32_t count = stub_target_uart_get_rxfifo_count(UART_NUM_0);
+
+        for (uint32_t i = 0; i < count; i++) {


jedi7 · 2025-11-19T11:03:59Z

src/main.c

+
+        for (uint32_t i = 0; i < count; i++) {
+            uint8_t byte = stub_target_uart_read_rxfifo_byte(UART_NUM_0);
+            stub_lib_uart_tx_one_char(byte);


this is little confusing, we are doing echo? or what is the purpose?

jedi7 · 2025-11-19T11:08:45Z

src/main.c

+            slip_recv_byte(byte);
+
+            if (slip_is_frame_complete() || slip_is_frame_error()) {
+                break;


it will be nice to have a comment like:
"cannot receive next frame till the current frame is processed"
also there should be mechanism which will prevent receiving next frame (blocking interrupt?) or allow to cache next frame.

jedi7 · 2025-11-19T11:09:23Z

src/main.c

+        }
+
+        if (slip_is_frame_error()) {
+            slip_recv_reset();


can we send an error to user?

jedi7 · 2025-11-19T11:11:56Z

src/slip.c


+    // If frame is already complete or has error, ignore new bytes until reset
+    if (s_recv_ctx.frame_complete || s_recv_ctx.frame_error) {
+        return;


this can lead to frame errors at higher speeds. we should buffer that.

jedi7 · 2025-11-19T11:12:51Z

src/slip.c

+        // Add escaped byte to buffer
+        if (s_recv_ctx.frame_length < s_recv_ctx.buffer_size) {
+            s_recv_ctx.buffer[s_recv_ctx.frame_length] = byte;
+            s_recv_ctx.frame_length++;


nit: ++s_recv_ctx.frame_length

Copilot

Pull Request Overview

This PR implements UART reception and SLIP protocol command processing for ESP stub firmware, enabling the stub to receive and decode commands from the host. Key changes include implementing the SLIP decoder state machine with buffer management, adding UART interrupt handling, and fixing BSS initialization and checksum handling issues.

Key changes:

Implements complete SLIP frame reception with state machine, error handling, and buffer management
Adds UART interrupt handler to receive bytes and process them through SLIP decoder
Fixes command handler to account for checksum field in SLIP header and replaces inline register functions with macros

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/slip.h	Adds function declarations for frame reception status checking and data retrieval
src/slip.c	Implements SLIP decoder state machine with buffering, error detection, and frame completion tracking
src/main.c	Adds BSS initialization, UART setup with interrupt handler, and main loop for processing received frames
src/command_handler.c	Replaces inline register access functions with macros and accounts for checksum in SLIP header parsing
src/CMakeLists.txt	Adds command_handler.c to build sources
esp-stub-lib	Updates submodule to version with UART support

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

Copilot · 2025-11-19T18:16:58Z

src/main.c

+
+        for (uint32_t i = 0; i < count; i++) {
+            uint8_t byte = stub_target_uart_read_rxfifo_byte(UART_NUM_0);
+            stub_lib_uart_tx_one_char(byte);


This line echoes every received byte back to UART, which appears to be debug code left in by mistake. In production, the stub should not echo received bytes as this will interfere with the SLIP protocol communication between the host and stub.

Suggested change

stub_lib_uart_tx_one_char(byte);

Copilot · 2025-11-19T18:16:58Z

src/slip.c

+            s_recv_ctx.frame_error = true;
+            state = STATE_NO_FRAME;
+        }
        break;


The STATE_ESCAPING case has duplicate buffer overflow handling logic that's identical to the STATE_IN_FRAME case (lines 117-124). Consider extracting this into a helper function to reduce duplication and improve maintainability.

Copilot · 2025-11-19T18:16:59Z

src/slip.c


 void slip_recv_byte(uint8_t byte)
 {
    static slip_state_t state = STATE_NO_FRAME;


The static state variable is not reset when slip_recv_reset() is called, which could leave the decoder in an inconsistent state. If a frame error occurs mid-escape sequence (STATE_ESCAPING), the state will remain in that state even after reset, causing the next frame to be decoded incorrectly. Consider resetting state to STATE_NO_FRAME in slip_recv_reset().

dobairoland · 2025-11-19T18:53:55Z

LGTM in general. Thanks!

radimkarnis

Nice job, this LGTM, thank you!

This commit also starts using read and write reg macros from esp-stub-lib

Dzarda7 self-assigned this Nov 19, 2025

Dzarda7 marked this pull request as draft November 19, 2025 08:18

Dzarda7 force-pushed the feat/add_uart_support branch from c082f6b to e6fdc45 Compare November 19, 2025 09:59

Dzarda7 requested review from dobairoland, jedi7 and radimkarnis and removed request for dobairoland, jedi7 and radimkarnis November 19, 2025 09:59

jedi7 reviewed Nov 19, 2025

View reviewed changes

dobairoland requested a review from Copilot November 19, 2025 18:14

Copilot AI reviewed Nov 19, 2025

View reviewed changes

radimkarnis approved these changes Nov 21, 2025

View reviewed changes

Dzarda7 force-pushed the feat/add_uart_support branch from e6fdc45 to 1ef9839 Compare November 26, 2025 09:07

Dzarda7 added 4 commits November 26, 2025 13:36

fix: Account for checksum field in command header

2a69fd9

This commit also starts using read and write reg macros from esp-stub-lib

fix: Erase bss section at the beginning

6cd0867

feat: Add slip functions for receive

38e47fb

feat: Add UART support for slip frame receive

aea8a71

Dzarda7 force-pushed the feat/add_uart_support branch from 1ef9839 to aea8a71 Compare November 26, 2025 12:36

Feat/add uart support #25

Are you sure you want to change the base?

Feat/add uart support #25

Conversation

Dzarda7 commented Nov 19, 2025

Description

Related

Testing

Checklist

Uh oh!

github-actions bot commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Dzarda7 commented Nov 19, 2025

Uh oh!

jedi7 Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

jedi7 Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

jedi7 Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

jedi7 Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

jedi7 Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

jedi7 Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

dobairoland commented Nov 19, 2025

Uh oh!

radimkarnis left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Nov 19, 2025 •

edited

Loading