Skip to content

Conversation

@RT-Trace
Copy link
Contributor

@RT-Trace RT-Trace commented Oct 30, 2025

virtual terminal v1.0.0版本,支持rt-trace工具使用

Summary by CodeRabbit

  • New Features

    • Added Virtual Terminal module with formatted output (printf-style support)
    • Added bidirectional buffer I/O operations for upstream and downstream communication
    • Added single-character read/write functionality
    • Added terminal console mode with I/O redirection capabilities
    • Added character device driver for system integration
  • Documentation

    • Localized README to Chinese with expanded setup guidance and feature descriptions

@Rbb666 Rbb666 closed this Nov 7, 2025
@Rbb666 Rbb666 reopened this Nov 7, 2025
@Rbb666 Rbb666 requested a review from Copilot November 7, 2025 03:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces a Virtual Terminal component for RT-Thread, enabling I/O redirection, bidirectional data transmission, and command interaction through RT-Tunnel. The component provides a character device interface ("vterm") for redirecting console and FinSH shell I/O, supports command registration, and includes formatted output capabilities.

  • Implements virtual terminal device driver with RT-Thread device model integration
  • Provides bidirectional data tunneling with upstream/downstream buffers
  • Adds command registration system and input assistance functionality

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 18 comments.

Show a summary per file
File Description
drv_Vterm.c Implements vterm device driver with open/read/write operations, I/O switching, and command registration
RT_Vterm.c Core virtual terminal module with tunnel-based data transmission functions
RT_Verm_print.c Formatted output implementation (printf/vprintf) for virtual terminal
RT_Vterm.h Public API declarations for virtual terminal operations and data structures
SConscript Build script for the virtual terminal package
RT_Vterm_examples/SConscript Build script for example code
RT_Vterm_examples/RT_Vterm_example_basic.c Basic usage examples demonstrating read/write and printf operations
README.md Chinese documentation describing features, initialization sequence, and usage

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

@coderabbitai
Copy link

coderabbitai bot commented Nov 7, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

This PR introduces a complete Virtual Terminal module (RT_Vterm) for RT-Thread featuring dual-tunnel upstream/downstream I/O, printf-style formatted output with buffering, single-character and buffered data operations, FinSH shell integration via a device driver, and example usage demonstrations. Documentation is localized to Chinese.

Changes

Cohort / File(s) Summary
Documentation & Build Setup
README.md, SConscript, RT_Vterm_examples/SConscript
README localized to Chinese with expanded feature descriptions and initialization flow; top-level SConscript aggregates sources from current and subdirectories; example SConscript collects .c files in test group.
Core Virtual Terminal API
RT_Vterm.h, RT_Vterm.c
Header declares extern tunnel handles, I/O wrappers (Write, Read, PutChar), status queries (HasData, HasKey), blocking/non-blocking reads, and initialization; implementation acquires/configures tunnels as VTTU/VTTD, manages upstream/downstream buffer operations, and provides defensive null-checks.
Printf Formatting
RT_Verm_print.c
Adds buffered printf implementation (RT_Vterm_printf, RT_Vterm_vprintf) with support for format specifiers (%c, %d, %u, %x/%X, %s, %p, %%), numeric padding/alignment, LF→CRLF conversion, and overflow handling.
Device Driver & FinSH Integration
drv_Vterm.c
Registers "vterm" character device with read/write handlers; implements vterm_console to redirect console and FinSH to vterm; provides restore_original for restoration; includes idle-hook polling (vterm_check) to forward data to FinSH RX semaphore.
Example Usage
RT_Vterm_examples/RT_Vterm_example_basic.c
Demonstrates basic I/O via vterm_example_basic (write/read/check buffers) and vterm_example_printf (formatted output across multiple types).

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant Vterm as RT_Vterm
    participant Tunnel as RT_tunnel
    participant Driver as vterm Driver
    participant FinSH as FinSH Shell

    App->>Vterm: RT_Vterm_Init()
    Vterm->>Tunnel: Acquire up_tunnel (write)
    Vterm->>Tunnel: Acquire down_tunnel (read)
    Tunnel-->>Vterm: Tunnels configured

    Note over App,FinSH: Upstream Data Flow
    App->>Vterm: RT_Vterm_Write(data)
    Vterm->>Tunnel: Write to up_tunnel
    Tunnel-->>Driver: Data available

    Note over App,FinSH: Downstream Data Flow
    Driver->>Vterm: RT_Vterm_Read()
    Vterm->>Tunnel: Read from down_tunnel
    Tunnel-->>Vterm: Data retrieved
    Vterm-->>Driver: Return data

    Note over App,FinSH: Console Redirection
    App->>Driver: vterm_console()
    Driver->>Driver: Save original console
    Driver->>FinSH: Redirect FinSH to vterm
    Driver->>App: Return confirmation

    Note over App,FinSH: Polling Integration
    Driver->>Driver: vterm_check (idle hook)
    Driver->>Vterm: RT_Vterm_HasData()
    alt Data Available
        Driver->>FinSH: Release RX semaphore
        FinSH->>FinSH: Process input
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • RT_Verm_print.c: Full printf implementation with numeric formatting, padding logic, buffer management, and edge case handling (zero-padding, sign display, alternate forms).
  • RT_Vterm.c: Tunnel lifecycle management, multiple I/O abstractions (blocking/non-blocking), and defensive null-check patterns across 13+ exported functions.
  • drv_Vterm.c: Device driver integration with FinSH shell redirection, LF→CRLF conversion, semaphore signaling, and idle-hook polling—requires understanding of RT-Thread architecture.
  • SConscript files: Recursive directory traversal and build aggregation logic.
  • Cross-file integration: Tunnel handles shared globally; device driver depends on Vterm public API; printf backs formatted output.

Poem

🐰 A tunnel tale in code so fine,
Where printf flows through buffers line,
FinSH now speaks through virtual veils,
Data floats on dual-rail trails,
RT-Thread's heart now types with grace!

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'virtual terminal v1.0.0' is vague and generic. While it references the project name and version, it fails to summarize the actual changes (new module implementation, printf functionality, driver integration, documentation localization). The version number alone doesn't convey meaningful information about what was added or changed. Replace with a more descriptive title that highlights the main change, such as 'Add RT_Vterm virtual terminal module with printf and I/O tunnels' or 'Implement virtual terminal driver with buffered printf support'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 96.55% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 9

♻️ Duplicate comments (8)
RT_Vterm_examples/RT_Vterm_example_basic.c (1)

1-1: Fix include casing to match header name.

RT_Vterm.h exists with capital “RT”, so including "rt_Vterm.h" fails on case-sensitive builds. This was already raised and remains unresolved. Please align the include with the actual filename.

-#include "rt_Vterm.h"
+#include "RT_Vterm.h"
RT_Verm_print.c (2)

1-1: Match the public header’s case.

The header is named RT_Vterm.h; using "rt_Vterm.h" breaks builds on case-sensitive filesystems. Prior feedback already covered this—please correct it.

-#include "rt_Vterm.h"
+#include "RT_Vterm.h"

498-504: Final flush must check the write result and avoid double counting.

BufferDesc.ReturnValue already includes buffered characters. Adding BufferDesc.Cnt again inflates the returned length, and ignoring the RT_Vterm_Write result hides write failures. Bail out on error and leave the count untouched.

-        if (BufferDesc.Cnt != 0u)
-        {
-            RT_Vterm_Write(acBuffer, BufferDesc.Cnt);
-        }
-        BufferDesc.ReturnValue += (int)BufferDesc.Cnt;
+        if (BufferDesc.Cnt != 0u)
+        {
+            if (RT_Vterm_Write(acBuffer, BufferDesc.Cnt) != BufferDesc.Cnt)
+            {
+                BufferDesc.ReturnValue = -1;
+            }
+        }
drv_Vterm.c (4)

1-1: Case-sensitive filename mismatch in include statement.

The include uses "rt_Vterm.h" but the actual filename is RT_Vterm.h. This will fail on case-sensitive filesystems (Linux).

Apply this diff:

-#include "rt_Vterm.h"
+#include "RT_Vterm.h"

63-79: Function always reports success even when writes fail.

The function returns size regardless of whether RT_Vterm_PutChar succeeds. If the buffer is full, this creates a mismatch between reported and actual bytes written.

Note: This was previously acknowledged and deferred to a future version.


123-123: Documentation mentions incorrect device name.

The comment says "vterm_buffer" but the code uses "vterm" (lines 133-134).

Apply this diff:

- *               2. Redirects console and FinSH output to "vterm_buffer" device
+ *               2. Redirects console and FinSH output to "vterm" device

164-169: Documentation incorrectly describes function as thread entry.

The documentation describes this as a thread entry function with parameter p and mentions "every 10ms" polling, but the function takes no parameters and is registered as an idle hook (line 198), not a dedicated thread.

Apply this diff:

-/**
- * @brief        Thread entry for polling Vterm data and notifying FinSH
- *
- * @param[in]    p  Unused (thread parameter placeholder)
- *
- * @note         1. Runs indefinitely, checking for downstream data every 10ms
- *               2. Triggers FinSH data processing via finsh_rx_ind when data is available
- */
+/**
+ * @brief        Idle hook for polling Vterm data and notifying FinSH
+ *
+ * @note         1. Called periodically by the idle hook mechanism
+ *               2. Triggers FinSH data processing via finsh_rx_ind when data is available
+ */
RT_Vterm.c (1)

33-43: Missing null checks for tunnel allocation.

The function calls Get_Free_Tunnel() without checking if it returns NULL. If tunnel allocation fails, accessing up_tunnel->ID or down_tunnel->ID will cause a null pointer dereference.

Apply this diff:

 int RT_Vterm_Init(void)
 {
     up_tunnel   = Get_Free_Tunnel();
     down_tunnel = Get_Free_Tunnel();
+    
+    if (!up_tunnel || !down_tunnel)
+    {
+        return -RT_ENOMEM;
+    }
+    
     Set_Tunnel_Operation(up_tunnel, tunnel_write);
     Set_Tunnel_Operation(down_tunnel, tunnel_read);
     up_tunnel->ID   = 0x56545455; // VTTU
     down_tunnel->ID = 0x56545444; // VTTD
 
     return RT_EOK;
 }
🧹 Nitpick comments (1)
RT_Vterm.c (1)

155-166: Consider adding timeout parameter for blocking wait.

The function blocks indefinitely until data arrives. In embedded systems, infinite blocking can be problematic if data never arrives or if called from inappropriate contexts.

Consider adding an optional timeout parameter in a future version:

int RT_Vterm_WaitKey_Timeout(uint32_t timeout_ms)
{
    int  r;
    char c;
    uint32_t elapsed = 0;
    
    while (elapsed < timeout_ms || timeout_ms == 0)
    {
        r = RT_Vterm_Read(&c, 1);
        if (r == 1)
            return (unsigned char)c;
        rt_thread_mdelay(1);
        elapsed++;
    }
    return -1;  // Timeout
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f692e7e and c1409b5.

📒 Files selected for processing (8)
  • README.md (1 hunks)
  • RT_Verm_print.c (1 hunks)
  • RT_Vterm.c (1 hunks)
  • RT_Vterm.h (1 hunks)
  • RT_Vterm_examples/RT_Vterm_example_basic.c (1 hunks)
  • RT_Vterm_examples/SConscript (1 hunks)
  • SConscript (1 hunks)
  • drv_Vterm.c (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
RT_Verm_print.c (1)
RT_Vterm.c (1)
  • RT_Vterm_Write (58-63)
RT_Vterm.h (2)
RT_Vterm.c (13)
  • RT_Vterm_Write (58-63)
  • RT_Vterm_WriteDownBuffer (75-80)
  • RT_Vterm_ReadUpBuffer (92-97)
  • RT_Vterm_Read (109-114)
  • RT_Vterm_PutChar (186-189)
  • RT_Vterm_PutCharSkip (196-201)
  • RT_Vterm_HasData (120-125)
  • RT_Vterm_HasDataUp (131-136)
  • RT_Vterm_HasKey (142-145)
  • RT_Vterm_WaitKey (155-166)
  • RT_Vterm_GetKey (172-179)
  • RT_Vterm_GetAvailWriteSpace (207-212)
  • RT_Vterm_Init (33-43)
RT_Verm_print.c (2)
  • RT_Vterm_printf (521-531)
  • RT_Vterm_vprintf (334-508)
drv_Vterm.c (1)
RT_Vterm.c (3)
  • RT_Vterm_PutChar (186-189)
  • RT_Vterm_Read (109-114)
  • RT_Vterm_HasData (120-125)
RT_Vterm_examples/RT_Vterm_example_basic.c (2)
RT_Vterm.c (3)
  • RT_Vterm_Write (58-63)
  • RT_Vterm_HasData (120-125)
  • RT_Vterm_Read (109-114)
RT_Verm_print.c (1)
  • RT_Vterm_printf (521-531)
🪛 Clang (14.0.6)
RT_Verm_print.c

[error] 1-1: 'rt_Vterm.h' file not found

(clang-diagnostic-error)

RT_Vterm.h

[error] 4-4: 'RT_tunnel.h' file not found

(clang-diagnostic-error)

RT_Vterm_examples/RT_Vterm_example_basic.c

[error] 1-1: 'rt_Vterm.h' file not found

(clang-diagnostic-error)

🔇 Additional comments (2)
RT_Vterm.h (2)

160-160: Verify whether the missing implementation is intentional.

Comprehensive codebase search confirms the function Start_Vterm_Input_Assist() is declared at RT_Vterm.h:160 with no implementation present in any .c, .cpp, .h, or .hpp files. Determine whether this is:

  • Intentional (external linking, weak symbol, or generated elsewhere)
  • Missing implementation that requires implementation

155-155: Function Vterm_Cmd_Register lacks implementation.

Verification confirms the function is declared in RT_Vterm.h (line 155) but has no implementation in the codebase. The function does not appear in RT_Vterm.c or any other source files. Determine whether this is intentional (stub/planned feature) or requires implementation.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (3)
RT_Vterm_examples/RT_Vterm_example_basic.c (3)

1-1: Fix the include case to match the actual filename.

The header file is RT_Vterm.h (capital RT) but included as rt_Vterm.h (lowercase). This will fail on case-sensitive filesystems like Linux.


22-27: Capture the read length and null-terminate before logging.

RT_Vterm_Read returns the byte count read but does not append '\0'. Printing buf directly without termination risks reading past the buffer when exactly 32 bytes are received.

Apply this diff:

     if (RT_Vterm_HasData())
     {
         // Read data from downstream buffer
-        RT_Vterm_Read(buf, sizeof(buf));
-        rt_kprintf("收到下行: %s\n", buf);
+        uint32_t rx_len = RT_Vterm_Read(buf, sizeof(buf) - 1);
+        buf[rx_len] = '\0';
+        rt_kprintf("收到下行: %s\n", buf);
     }

38-53: Remove unsupported %f format specifier.

RT_Vterm_printf does not implement floating-point formatting (no %f case in RT_Verm_print.c:333-507). The float variable and format specifier will produce undefined output.

Apply this diff to remove the float example:

 void vterm_example_printf(void)
 {
     int         num  = 123;
     unsigned    hex  = 0xABCD;
     const char *str  = "hello";
-    float       fval = 3.1415f;
 
     RT_Vterm_printf("==== Vterm printf 示例 ====\n");
     RT_Vterm_printf("整数: %d\n", num);
     RT_Vterm_printf("十六进制: 0x%X\n", hex);
     RT_Vterm_printf("字符串: %s\n", str);
     RT_Vterm_printf("混合: num=%d, hex=0x%X, str=%s\n", num, hex, str);
     RT_Vterm_printf("请在调试器端查看输出\n");
 
     rt_kprintf("Vterm printf example done.\n");
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c1409b5 and 5d04601.

📒 Files selected for processing (3)
  • RT_Vterm.h (1 hunks)
  • RT_Vterm_examples/RT_Vterm_example_basic.c (1 hunks)
  • drv_Vterm.c (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: RT-Trace
Repo: RT-Thread-packages/Virtual-Terminal PR: 1
File: RT_Verm_print.c:479-481
Timestamp: 2025-11-07T06:27:44.999Z
Learning: The Virtual-Terminal project (RT-Thread-packages/Virtual-Terminal) is currently targeting 32-bit platforms only and does not consider 64-bit platform support.
Learnt from: RT-Trace
Repo: RT-Thread-packages/Virtual-Terminal PR: 1
File: RT_Vterm.h:4-4
Timestamp: 2025-11-07T06:29:47.968Z
Learning: In the Virtual-Terminal project (RT-Thread-packages/Virtual-Terminal), the header file RT_tunnel.h is not part of this repository. It comes from an external "tunnel" package dependency that must be pulled/installed separately.
📚 Learning: 2025-11-07T06:29:47.968Z
Learnt from: RT-Trace
Repo: RT-Thread-packages/Virtual-Terminal PR: 1
File: RT_Vterm.h:4-4
Timestamp: 2025-11-07T06:29:47.968Z
Learning: In the Virtual-Terminal project (RT-Thread-packages/Virtual-Terminal), the header file RT_tunnel.h is not part of this repository. It comes from an external "tunnel" package dependency that must be pulled/installed separately.

Applied to files:

  • drv_Vterm.c
  • RT_Vterm.h
  • RT_Vterm_examples/RT_Vterm_example_basic.c
📚 Learning: 2025-11-07T06:27:44.999Z
Learnt from: RT-Trace
Repo: RT-Thread-packages/Virtual-Terminal PR: 1
File: RT_Verm_print.c:479-481
Timestamp: 2025-11-07T06:27:44.999Z
Learning: The Virtual-Terminal project (RT-Thread-packages/Virtual-Terminal) is currently targeting 32-bit platforms only and does not consider 64-bit platform support.

Applied to files:

  • RT_Vterm.h
🧬 Code graph analysis (3)
drv_Vterm.c (1)
RT_Vterm.c (3)
  • RT_Vterm_PutChar (186-189)
  • RT_Vterm_Read (109-114)
  • RT_Vterm_HasData (120-125)
RT_Vterm.h (3)
RT_Vterm.c (13)
  • RT_Vterm_Write (58-63)
  • RT_Vterm_WriteDownBuffer (75-80)
  • RT_Vterm_ReadUpBuffer (92-97)
  • RT_Vterm_Read (109-114)
  • RT_Vterm_PutChar (186-189)
  • RT_Vterm_PutCharSkip (196-201)
  • RT_Vterm_HasData (120-125)
  • RT_Vterm_HasDataUp (131-136)
  • RT_Vterm_HasKey (142-145)
  • RT_Vterm_WaitKey (155-166)
  • RT_Vterm_GetKey (172-179)
  • RT_Vterm_GetAvailWriteSpace (207-212)
  • RT_Vterm_Init (33-43)
RT_Verm_print.c (2)
  • RT_Vterm_printf (521-531)
  • RT_Vterm_vprintf (334-508)
drv_Vterm.c (2)
  • vterm_console (126-137)
  • restore_original (148-159)
RT_Vterm_examples/RT_Vterm_example_basic.c (2)
RT_Vterm.c (3)
  • RT_Vterm_Write (58-63)
  • RT_Vterm_HasData (120-125)
  • RT_Vterm_Read (109-114)
RT_Verm_print.c (1)
  • RT_Vterm_printf (521-531)
🪛 Clang (14.0.6)
RT_Vterm.h

[error] 4-4: 'RT_tunnel.h' file not found

(clang-diagnostic-error)

RT_Vterm_examples/RT_Vterm_example_basic.c

[error] 1-1: 'rt_Vterm.h' file not found

(clang-diagnostic-error)

🔇 Additional comments (2)
drv_Vterm.c (2)

187-207: LGTM—Error handling properly implemented.

The function now correctly checks the return value of rt_device_register and propagates errors, addressing the previous review concern.


219-228: LGTM—Error propagation correctly added.

The function now properly checks initialization results and only proceeds with console switching when device registration succeeds, resolving the previous review concern.

RT_Vterm.h Outdated
/**
* @brief Start virtual terminal input assistance function
*/
void Start_Vterm_Input_Assist(void);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify implementation of Start_Vterm_Input_Assist.

The function is declared but its implementation is not visible in the provided files. Confirm that it's implemented elsewhere in the codebase.


🏁 Script executed:

#!/bin/bash
# Search for Start_Vterm_Input_Assist implementation
rg -nP -C3 '^\s*void\s+Start_Vterm_Input_Assist\s*\('

Length of output: 318


I need to conduct a broader search to verify if the implementation exists. Let me search more comprehensively across the codebase.


Actually, let me correct that - I need to search the repository:




🏁 Script executed:

#!/bin/bash
# Search for Start_Vterm_Input_Assist in all source files (not just headers)
rg "Start_Vterm_Input_Assist" -n --type-list | head -20
rg "Start_Vterm_Input_Assist" -n -g '!*.h' -g '*.c' -g '*.cpp' -g '*.cc'

Length of output: 389


🏁 Script executed:

#!/bin/bash
# Search for Start_Vterm_Input_Assist in all files, particularly source files
rg "Start_Vterm_Input_Assist" -n

Length of output: 51


Implement Start_Vterm_Input_Assist function.

The function is declared in RT_Vterm.h at line 154 but has no implementation anywhere in the codebase. This will cause a linker error. You must provide an implementation for this function.

🤖 Prompt for AI Agents
In RT_Vterm.h at line 154 the function Start_Vterm_Input_Assist(void) is
declared but has no definition, causing a linker error; add a matching
implementation in the corresponding source file (e.g., RT_Vterm.c or
RT_Vterm.cpp) with the exact signature and correct linkage (extern "C" if
compiling as C++), include any headers needed, and implement the startup logic
for the input-assist subsystem (or provide a safe no-op body if initialization
is not yet required) so the symbol is defined and the project links
successfully.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (2)
RT_Vterm_examples/RT_Vterm_example_basic.c (2)

1-1: Fix include filename case for Linux compatibility.

The include uses rt_Vterm.h (lowercase) but the actual header is RT_Vterm.h (uppercase). This will fail on case-sensitive filesystems.

Apply this diff:

-#include "rt_Vterm.h"
+#include "RT_Vterm.h"

22-27: Null-terminate buffer before printing.

RT_Vterm_Read returns the number of bytes read but doesn't append a null terminator. Printing with %s without null-termination can read past the buffer.

Apply this diff:

     // Check if downstream buffer has data
     if (RT_Vterm_HasData())
     {
         // Read data from downstream buffer
-        RT_Vterm_Read(buf, sizeof(buf));
+        uint32_t len = RT_Vterm_Read(buf, sizeof(buf) - 1);
+        buf[len] = '\0';
         rt_kprintf("收到下行: %s\n", buf);
     }
🧹 Nitpick comments (3)
RT_Vterm.c (1)

38-38: Add spaces around operators for better readability.

The conditional expression is missing spaces around the logical OR operator and after the negation operator.

Apply this diff:

-    if(!up_tunnel||!down_tunnel)
+    if (!up_tunnel || !down_tunnel)
RT_Vterm_examples/RT_Vterm_example_basic.c (1)

43-43: Remove unused variable.

The fval variable is declared but never used after the float formatting examples were removed.

Apply this diff:

     int         num  = 123;
     unsigned    hex  = 0xABCD;
     const char *str  = "hello";
-    float       fval = 3.1415f;
RT_Vterm.h (1)

115-125: Consider removing unused command infrastructure.

The vterm_cmd_callback_t typedef and vterm_cmd_entry_t struct are declared, and drv_Vterm.c has a static table and counter, but there are no public functions (Vterm_Cmd_Register, Start_Vterm_Input_Assist) implemented to use this infrastructure.

If this is planned for future implementation, consider adding a comment indicating it's reserved for future use. Otherwise, remove the unused declarations to reduce API surface.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5d04601 and c028443.

📒 Files selected for processing (4)
  • RT_Vterm.c (1 hunks)
  • RT_Vterm.h (1 hunks)
  • RT_Vterm_examples/RT_Vterm_example_basic.c (1 hunks)
  • drv_Vterm.c (1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: RT-Trace
Repo: RT-Thread-packages/Virtual-Terminal PR: 1
File: RT_Verm_print.c:479-481
Timestamp: 2025-11-07T06:27:44.999Z
Learning: The Virtual-Terminal project (RT-Thread-packages/Virtual-Terminal) is currently targeting 32-bit platforms only and does not consider 64-bit platform support.
Learnt from: RT-Trace
Repo: RT-Thread-packages/Virtual-Terminal PR: 1
File: RT_Vterm.h:4-4
Timestamp: 2025-11-07T06:29:47.968Z
Learning: In the Virtual-Terminal project (RT-Thread-packages/Virtual-Terminal), the header file RT_tunnel.h is not part of this repository. It comes from an external "tunnel" package dependency that must be pulled/installed separately.
📚 Learning: 2025-11-07T06:29:47.968Z
Learnt from: RT-Trace
Repo: RT-Thread-packages/Virtual-Terminal PR: 1
File: RT_Vterm.h:4-4
Timestamp: 2025-11-07T06:29:47.968Z
Learning: In the Virtual-Terminal project (RT-Thread-packages/Virtual-Terminal), the header file RT_tunnel.h is not part of this repository. It comes from an external "tunnel" package dependency that must be pulled/installed separately.

Applied to files:

  • RT_Vterm_examples/RT_Vterm_example_basic.c
  • drv_Vterm.c
  • RT_Vterm.c
  • RT_Vterm.h
📚 Learning: 2025-11-07T06:27:44.999Z
Learnt from: RT-Trace
Repo: RT-Thread-packages/Virtual-Terminal PR: 1
File: RT_Verm_print.c:479-481
Timestamp: 2025-11-07T06:27:44.999Z
Learning: The Virtual-Terminal project (RT-Thread-packages/Virtual-Terminal) is currently targeting 32-bit platforms only and does not consider 64-bit platform support.

Applied to files:

  • RT_Vterm.c
  • RT_Vterm.h
🧬 Code graph analysis (3)
RT_Vterm_examples/RT_Vterm_example_basic.c (2)
RT_Vterm.c (3)
  • RT_Vterm_Write (65-70)
  • RT_Vterm_HasData (127-132)
  • RT_Vterm_Read (116-121)
RT_Verm_print.c (1)
  • RT_Vterm_printf (521-531)
drv_Vterm.c (1)
RT_Vterm.c (3)
  • RT_Vterm_PutChar (193-196)
  • RT_Vterm_Read (116-121)
  • RT_Vterm_HasData (127-132)
RT_Vterm.h (3)
RT_Vterm.c (13)
  • RT_Vterm_Write (65-70)
  • RT_Vterm_WriteDownBuffer (82-87)
  • RT_Vterm_ReadUpBuffer (99-104)
  • RT_Vterm_Read (116-121)
  • RT_Vterm_PutChar (193-196)
  • RT_Vterm_PutCharSkip (203-208)
  • RT_Vterm_HasData (127-132)
  • RT_Vterm_HasDataUp (138-143)
  • RT_Vterm_HasKey (149-152)
  • RT_Vterm_WaitKey (162-173)
  • RT_Vterm_GetKey (179-186)
  • RT_Vterm_GetAvailWriteSpace (214-219)
  • RT_Vterm_Init (33-50)
RT_Verm_print.c (2)
  • RT_Vterm_printf (521-531)
  • RT_Vterm_vprintf (334-508)
drv_Vterm.c (2)
  • vterm_console (126-137)
  • restore_original (148-159)
🪛 Clang (14.0.6)
RT_Vterm_examples/RT_Vterm_example_basic.c

[error] 1-1: 'rt_Vterm.h' file not found

(clang-diagnostic-error)

RT_Vterm.h

[error] 4-4: 'RT_tunnel.h' file not found

(clang-diagnostic-error)

🔇 Additional comments (1)
RT_Vterm.h (1)

143-150: Constants are unused internally but are already documented.

Verification confirms these four mode constants are not used anywhere within the codebase. However, since they are defined in the public header file RT_Vterm.h and already have clear inline documentation describing their intended purpose, they appear to be part of the public API interface for external consumers. No additional documentation is required.

@Rbb666 Rbb666 merged commit 573e86c into RT-Thread-packages:master Nov 7, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants