Skip to content

Conversation

@lct1001
Copy link

@lct1001 lct1001 commented Oct 29, 2025

…added.

Some comments have been initially added.
components/libc/cplusplus/os/cxx_Semaphore.cpp
components/libc/cplusplus/os/cxx_Thread.cpp
components/libc/cplusplus/utest/tc_atomic.cpp
components/libc/cplusplus/utest/tc_smartptr.cpp
components/libc/cplusplus/utest/tc_thread.cpp

Signed-off-by:Liu Chengtao2739960959@qq.com

拉取/合并请求描述:(PR description)

[
Provide comments for some files in the components/libc/cplusplus/os or components/libc/cplusplus/utest folder to complete the beginner task.
]

当前拉取/合并请求的状态 Intent for your PR

必须选择一项 Choose one (Mandatory):

  • 本拉取/合并请求是一个草稿版本 This PR is for a code-review and is intended to get feedback
  • [* ] 本拉取/合并请求是一个成熟版本 This PR is mature, and ready to be integrated into the repo

代码质量 Code Quality:

我在这个拉取/合并请求中已经考虑了 As part of this pull request, I've considered the following:

  • [ *] 已经仔细查看过代码改动的对比 Already check the difference between PR and old code
  • [* ] 代码风格正确,包括缩进空格,命名及其他风格 Style guide is adhered to, including spacing, naming and other styles
  • [ *] 没有垃圾代码,代码尽量精简,不包含#if 0代码,不包含已经被注释了的代码 All redundant code is removed and cleaned up
  • [ *] 所有变更均有原因及合理的,并且不会影响到其他软件组件代码或BSP All modifications are justified and not affect other components or BSP
  • [ *] 对难懂代码均提供对应的注释 I've commented appropriately where code is tricky
  • [ *] 代码是高质量的 Code in this PR is of high quality
  • [ *] 已经使用formatting 等源码格式化工具确保格式符合RT-Thread代码规范 This PR complies with RT-Thread code specification
  • 如果是新增bsp, 已经添加ci检查到.github/ALL_BSP_COMPILE.json 详细请参考链接BSP自查

…added.

Some comments have been initially added.
components/libc/cplusplus/os/cxx_Semaphore.cpp
components/libc/cplusplus/os/cxx_Thread.cpp
components/libc/cplusplus/utest/tc_atomic.cpp
components/libc/cplusplus/utest/tc_smartptr.cpp
components/libc/cplusplus/utest/tc_thread.cpp

Signed-off-by:Liu Chengtao<2739960959@qq.com>
@lct1001 lct1001 requested a review from mysterywolf as a code owner October 29, 2025 13:37
@CLAassistant
Copy link

CLAassistant commented Oct 29, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ lct1001
❌ github-actions[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions
Copy link

👋 感谢您对 RT-Thread 的贡献!Thank you for your contribution to RT-Thread!

为确保代码符合 RT-Thread 的编码规范,请在你的仓库中执行以下步骤运行代码格式化工作流(如果格式化CI运行失败)。
To ensure your code complies with RT-Thread's coding style, please run the code formatting workflow by following the steps below (If the formatting of CI fails to run).


🛠 操作步骤 | Steps

  1. 前往 Actions 页面 | Go to the Actions page
    点击进入工作流 → | Click to open workflow →

  2. 点击 Run workflow | Click Run workflow

  • 设置需排除的文件/目录(目录请以"/"结尾)
    Set files/directories to exclude (directories should end with "/")
  • 将目标分支设置为 \ Set the target branch to:master
  • 设置PR number为 \ Set the PR number to:10870
  1. 等待工作流完成 | Wait for the workflow to complete
    格式化后的代码将自动推送至你的分支。
    The formatted code will be automatically pushed to your branch.

完成后,提交将自动更新至 master 分支,关联的 Pull Request 也会同步更新。
Once completed, commits will be pushed to the master branch automatically, and the related Pull Request will be updated.

如有问题欢迎联系我们,再次感谢您的贡献!💐
If you have any questions, feel free to reach out. Thanks again for your contribution!

@github-actions
Copy link

github-actions bot commented Oct 29, 2025

📌 Code Review Assignment

🏷️ Tag: components

Reviewers: Maihuanyi

Changed Files (Click to expand)
  • components/libc/cplusplus/os/cxx_Semaphore.cpp
  • components/libc/cplusplus/os/cxx_Thread.cpp
  • components/libc/cplusplus/utest/tc_atomic.cpp
  • components/libc/cplusplus/utest/tc_smartptr.cpp
  • components/libc/cplusplus/utest/tc_thread.cpp

🏷️ Tag: components_libc

Reviewers: GorrayLi mysterywolf

Changed Files (Click to expand)
  • components/libc/cplusplus/os/cxx_Semaphore.cpp
  • components/libc/cplusplus/os/cxx_Thread.cpp
  • components/libc/cplusplus/utest/tc_atomic.cpp
  • components/libc/cplusplus/utest/tc_smartptr.cpp
  • components/libc/cplusplus/utest/tc_thread.cpp

📊 Current Review Status (Last Updated: 2025-11-13 18:30 CST)

  • GorrayLi Pending Review
  • Maihuanyi Pending Review
  • mysterywolf Pending Review

📝 Review Instructions

  1. 维护者可以通过单击此处来刷新审查状态: 🔄 刷新状态
    Maintainers can refresh the review status by clicking here: 🔄 Refresh Status

  2. 确认审核通过后评论 LGTM/lgtm
    Comment LGTM/lgtm after confirming approval

  3. PR合并前需至少一位维护者确认
    PR must be confirmed by at least one maintainer before merging

ℹ️ 刷新CI状态操作需要具备仓库写入权限。
ℹ️ Refresh CI status operation requires repository Write permission.

@lct1001
Copy link
Author

lct1001 commented Oct 29, 2025

/check-cla

title:Everything is based on remote repositories
author:Liu Chengtao<2739960959@qq.com>
UTEST_UNIT_RUN(test_thread);
}

// Export the test case with initialization and cleanup functions and a timeout of 10 ticks.
Copy link
Member

Choose a reason for hiding this comment

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

代码注释的风格统一下,需要使用/**/

Copilot finished reviewing on behalf of Rbb666 November 13, 2025 02:13
Copy link
Contributor

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 adds English comments and documentation to C++ wrapper classes and unit test files in the RT-Thread libc/cplusplus component. The changes aim to improve code readability and documentation for beginners, though several issues need to be addressed to align with RT-Thread documentation standards.

Key Changes:

  • Added Doxygen-style comments to constructors, methods, and test functions in C++ OS wrapper classes (Thread, Semaphore)
  • Added inline and function-level comments to unit test files (tc_thread, tc_smartptr, tc_atomic)
  • Minor code formatting adjustments (whitespace alignment in tc_atomic.cpp, constructor initializer list formatting in cxx_Thread.cpp)

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 25 comments.

Show a summary per file
File Description
components/libc/cplusplus/utest/tc_thread.cpp Added Doxygen comments to test functions and inline comments explaining test logic
components/libc/cplusplus/utest/tc_smartptr.cpp Added Doxygen comments to initialization and cleanup functions
components/libc/cplusplus/utest/tc_atomic.cpp Added simple comments to init/cleanup functions and adjusted variable alignment formatting
components/libc/cplusplus/os/cxx_Thread.cpp Added Doxygen comments to Thread class methods and constructors, modified constructor initializer list formatting
components/libc/cplusplus/os/cxx_Semaphore.cpp Added Doxygen comments to Semaphore class methods and inline explanatory comments

Comment on lines 133 to 135
* Join the thread with a timeout.
* @param millisec Timeout in milliseconds.
* @return Status code indicating the execution status.
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

Incomplete Doxygen Comment / 不完整的 Doxygen 注释

English: The comment is missing the @brief tag and should document both the parameter and return value.

中文:注释缺少 @brief 标签,并且应该记录参数和返回值。

Suggested format / 建议格式:

/** 
 * @brief Join the thread with a timeout.
 * @param millisec Timeout in milliseconds
 * @return RT_EOK if the thread completed within the timeout, error code otherwise
 */
Suggested change
* Join the thread with a timeout.
* @param millisec Timeout in milliseconds.
* @return Status code indicating the execution status.
* @brief Join the thread with a timeout.
* @param millisec Timeout in milliseconds.
* @return RT_EOK if the thread completed within the timeout, error code otherwise.

Copilot uses AI. Check for mistakes.
for (int i = 0; i < 100; ++i)
{
++count;
++count; // Increment the count 100 times.
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

[nitpick] Redundant Comment / 冗余注释

English: The comment "Increment the count 100 times" is redundant since the for loop already clearly shows i < 100. This is an example of commenting "what" instead of "why." Remove this comment as it adds no value.

中文:注释"Increment the count 100 times"是冗余的,因为 for 循环已经清楚地显示了 i < 100。这是注释"是什么"而不是"为什么"的例子。删除此注释,因为它没有增加任何价值。

Suggested change
++count; // Increment the count 100 times.
++count;

Copilot uses AI. Check for mistakes.
Comment on lines 31 to 48
// Create and run a thread executing the lambda function.
std::thread t1(func);
t1.join();
t1.join(); // Wait for the thread to finish execution.

// Verify if the count is as expected after the first thread execution.
if (count != 100)
{
uassert_false(1);
uassert_false(1); // Assert failure if count is not 100.
}

// Create and run another thread executing the same lambda function.
std::thread t2(func);
t2.join();
t2.join(); // Wait for the second thread to finish execution.

// Verify if the count is as expected after the second thread execution.
if (count != 200)
{
uassert_false(1);
uassert_false(1); // Assert failure if count is not 200.
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

[nitpick] Redundant Inline Comments / 冗余的行内注释

English: The inline comments on lines 33, 38, 43, and 48 simply restate what the code does (join, assert, join, assert). These are obvious from reading the code. According to RT-Thread documentation standards, avoid comments that merely describe what the code does when it's self-evident. Better to have a single comment explaining the test logic if needed.

中文:第 33、38、43 和 48 行的行内注释仅重述了代码的功能(join、assert、join、assert)。从代码阅读中这些是显而易见的。根据 RT-Thread 文档标准,避免仅描述代码功能的注释,当代码本身是不言自明的时候。如果需要,最好使用单个注释来解释测试逻辑。

Copilot uses AI. Check for mistakes.
Comment on lines 20 to 21
const char *name) :
_entry(RT_NULL), _param(RT_NULL), started(false)
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

Code Formatting Issue / 代码格式问题

English: The brace placement doesn't follow RT-Thread coding style. According to RT-Thread coding standards, braces should be on separate lines, not on the same line as the colon in constructor initializer lists. The original formatting was more aligned with the standard.

中文:大括号位置不符合 RT-Thread 编码风格。根据 RT-Thread 编码标准,大括号应独占一行,而不是与构造函数初始化列表中的冒号在同一行。原始格式更符合标准。

Original format (preferred) / 原格式(推荐):

const char *name)
    : _entry(RT_NULL), _param(RT_NULL), started(false)
{

Current format (non-standard) / 当前格式(不标准):

const char *name) :
    _entry(RT_NULL), _param(RT_NULL), started(false)
{

Copilot uses AI. Check for mistakes.
using namespace rtthread;

/**
* @brief Thread class constructor with parameters for stack size, priority, tick, and name.
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

[nitpick] Incomplete Doxygen Comment / 不完整的 Doxygen 注释

English: The Doxygen comment should document all parameters and follow complete documentation format. Add @param tags for each parameter to make the documentation complete and useful.

中文:Doxygen 注释应记录所有参数并遵循完整的文档格式。为每个参数添加 @param 标签,使文档完整且有用。

Suggested format / 建议格式:

/** 
 * @brief Thread class constructor with parameters for stack size, priority, tick, and name.
 * @param stack_size Stack size in bytes
 * @param priority Thread priority
 * @param tick Time slice in ticks
 * @param name Thread name
 */
Suggested change
* @brief Thread class constructor with parameters for stack size, priority, tick, and name.
* @brief Thread class constructor with parameters for stack size, priority, tick, and name.
* @param stack_size Stack size in bytes
* @param priority Thread priority
* @param tick Time slice in ticks
* @param name Thread name

Copilot uses AI. Check for mistakes.
Comment on lines +64 to +66
/**
* @brief Test case cleanup function.
*/
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

[nitpick] Incomplete Doxygen Comment / 不完整的 Doxygen 注释

English: The Doxygen comment should document the return value. Add @return RT_EOK on success for consistency and completeness.

中文:Doxygen 注释应记录返回值。添加 @return RT_EOK on success 以保持一致性和完整性。

Copilot uses AI. Check for mistakes.
UTEST_UNIT_RUN(test_thread);
}

// Export the test case with initialization and cleanup functions and a timeout of 10 ticks.
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

Comment Style Inconsistency / 注释风格不一致

English: This comment uses // style while other function documentation in the file uses /** */ Doxygen style. For consistency across the file, use the Doxygen format with @brief tag like the other functions.

中文:此注释使用 // 风格,而文件中的其他函数文档使用 /** */ Doxygen 风格。为了整个文件的一致性,请使用带有 @brief 标签的 Doxygen 格式,就像其他函数一样。

Suggested format / 建议格式:

/**
 * @brief Export the test case with initialization and cleanup functions and a timeout of 10 ticks.
 */
Suggested change
// Export the test case with initialization and cleanup functions and a timeout of 10 ticks.
/**
* @brief Export the test case with initialization and cleanup functions and a timeout of 10 ticks.
*/

Copilot uses AI. Check for mistakes.
Comment on lines +241 to +247
// Test case initialization function.
static rt_err_t utest_tc_init(void)
{
return RT_EOK;
}

// Test case cleanup function.
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

Comment Style Inconsistency / 注释风格不一致

English: This comment uses // style while the file contains Doxygen-style comments elsewhere. For consistency, use the Doxygen format with /** */ and @brief tag.

中文:此注释使用 // 风格,而文件中其他地方包含 Doxygen 风格的注释。为了一致性,请使用带有 /** */@brief 标签的 Doxygen 格式。

Suggested format / 建议格式:

/**
 * @brief Test case initialization function.
 * @return RT_EOK on success
 */
Suggested change
// Test case initialization function.
static rt_err_t utest_tc_init(void)
{
return RT_EOK;
}
// Test case cleanup function.
/**
* @brief Test case initialization function.
* @return RT_EOK on success
*/
static rt_err_t utest_tc_init(void)
{
return RT_EOK;
}
/**
* @brief Test case cleanup function.
* @return RT_EOK on success
*/

Copilot uses AI. Check for mistakes.
const char *name)
: _entry(entry), _param(p), started(false)
const char *name) :
_entry(entry), _param(p), started(false)
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

Code Formatting Issue / 代码格式问题

English: The brace placement doesn't follow RT-Thread coding style. According to RT-Thread coding standards, braces should be on separate lines, not on the same line as the colon in constructor initializer lists.

中文:大括号位置不符合 RT-Thread 编码风格。根据 RT-Thread 编码标准,大括号应独占一行,而不是与构造函数初始化列表中的冒号在同一行。

Original format (preferred) / 原格式(推荐):

const char *name)
    : _entry(entry), _param(p), started(false)
{
Suggested change
_entry(entry), _param(p), started(false)
_entry(entry), _param(p), started(false)

Copilot uses AI. Check for mistakes.

/**
* @brief Start the thread execution.
* @return Boolean indicating if the thread was successfully started.
Copy link

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

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

[nitpick] Incomplete Doxygen Comment / 不完整的 Doxygen 注释

English: The comment should include @return documentation to describe the return value properly.

中文:注释应包含 @return 文档以正确描述返回值。

Suggested / 建议:

/** 
 * @brief Start the thread execution.
 * @return true if the thread was successfully started, false otherwise
 */
Suggested change
* @return Boolean indicating if the thread was successfully started.
* @return true if the thread was successfully started, false otherwise

Copilot uses AI. Check for mistakes.
…pecification

Modify the os:cxx_Semaphore.cpp and cxx_Thread.cpp to comply with the rtthread specification.
Change the comment symbol in utest:tc_thread.cpp from '//' to '/**/' to comply with rtthread specifications.For utest:tc_atomic.cpp and tc_smartptr.cpp,select the original respository files without modification.
signed-off-by: Liu Chengtao<2739960959@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants