Skip to content

Conversation

@RCSN
Copy link
Contributor

@RCSN RCSN commented Dec 17, 2025

  • 添加 RT_CAN_CMD_SET_SENDMODE 命令控制非阻塞发送
  • 通过 RT_CAN_CMD_SET_SENDMODE 命令添加全局发送模式控制。
  • 只有显式使能非阻塞模式后才会触发发送完成回调。

…n-blocking TX control

- Add global send mode control via RT_CAN_CMD_SET_SENDMODE command.
TX completion callback is only triggered when non-blocking mode is explicitly enabled through this command.

Signed-off-by: Runcheng Lu <runcheng.lu@hpmicro.com>
Copilot AI review requested due to automatic review settings December 17, 2025 04:17
@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:feature/add_send_completed_callback_for_can
  • 设置PR number为 \ Set the PR number to:11070
  1. 等待工作流完成 | Wait for the workflow to complete
    格式化后的代码将自动推送至你的分支。
    The formatted code will be automatically pushed to your branch.

完成后,提交将自动更新至 feature/add_send_completed_callback_for_can 分支,关联的 Pull Request 也会同步更新。
Once completed, commits will be pushed to the feature/add_send_completed_callback_for_can 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!

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 pull request adds global send mode control for the CAN driver by introducing a new command RT_CAN_CMD_SET_SENDMODE and a corresponding field send_mode_nonblocking to enable non-blocking transmission behavior globally. When enabled, this allows the TX completion callback to be triggered for all send operations, not just those explicitly marked as non-blocking.

Key Changes:

  • Added RT_CAN_CMD_SET_SENDMODE (0x1E) command definition for controlling global send mode
  • Added send_mode_nonblocking field to rt_can_device structure to track the global send mode state
  • Modified rt_can_write() to check the global send mode flag alongside existing non-blocking conditions
  • Updated ISR handler rt_hw_can_isr() to invoke tx_complete callback when global non-blocking mode is enabled

Reviewed changes

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

File Description
components/drivers/include/drivers/dev_can.h Added RT_CAN_CMD_SET_SENDMODE command definition and send_mode_nonblocking field to rt_can_device structure
components/drivers/can/dev_can.c Modified rt_can_write() to check global send mode; updated ISR to trigger tx_complete callback for non-blocking mode

Critical Issues Found:

  • 🔴 The RT_CAN_CMD_SET_SENDMODE command is defined but not implemented in rt_can_control() function, making it unusable
  • 🔴 The send_mode_nonblocking field is never initialized, leading to undefined behavior
  • 🔴 The tx_complete callback is invoked before pending messages are processed from the ring buffer, causing incorrect timing
  • 🟡 Future dates in changelog entries (2025-09-20) need correction
  • 🟡 Missing documentation for the new command and field

#define RT_CAN_CMD_SET_BAUD_FD 0x1B
#define RT_CAN_CMD_SET_BITTIMING 0x1C
#define RT_CAN_CMD_START 0x1D
#define RT_CAN_CMD_SET_SENDMODE 0x1E
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

[Bug/错误]: RT_CAN_CMD_SET_SENDMODE command is defined but not implemented / RT_CAN_CMD_SET_SENDMODE 命令已定义但未实现

English: The new command RT_CAN_CMD_SET_SENDMODE (0x1E) is defined in the header file, but there is no corresponding case handler in the rt_can_control() function in dev_can.c. This means users cannot actually use this command to set the send mode. You need to add a case in rt_can_control() to handle this command and set can->send_mode_nonblocking based on the args parameter.

中文:新命令 RT_CAN_CMD_SET_SENDMODE (0x1E) 在头文件中定义,但在 dev_can.c 的 rt_can_control() 函数中没有相应的 case 处理程序。这意味着用户实际上无法使用此命令来设置发送模式。您需要在 rt_can_control() 中添加一个 case 来处理此命令,并根据 args 参数设置 can->send_mode_nonblocking。

Example implementation / 实现示例:
Add a case in rt_can_control() switch statement to handle RT_CAN_CMD_SET_SENDMODE and set can->send_mode_nonblocking = (rt_uint8_t)(rt_ubase_t)args.

Copilot uses AI. Check for mistakes.
#else
rt_uint8_t nb_tx_rb_pool[RT_CAN_NB_TX_FIFO_SIZE]; /**< The statically allocated pool for the non-blocking TX ring buffer. */
#endif /* RT_CAN_MALLOC_NB_TX_BUFFER */
rt_uint8_t send_mode_nonblocking; /**< Global send mode: 0=Blocking (default), 1=Non-blocking. */
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

[Bug/错误]: send_mode_nonblocking field is not initialized / send_mode_nonblocking 字段未初始化

English: The new field send_mode_nonblocking is added to struct rt_can_device but is never initialized in rt_hw_can_register(). This means it will have an undefined/garbage value when the device is registered. According to the PR description, the default should be 0 (blocking mode). You should initialize this field to 0 in rt_hw_can_register() around line 920-928 where other fields are initialized.

中文:新字段 send_mode_nonblocking 已添加到 struct rt_can_device,但在 rt_hw_can_register() 中从未初始化。这意味着当设备注册时,它将具有未定义/垃圾值。根据 PR 描述,默认值应为 0(阻塞模式)。您应该在 rt_hw_can_register() 的第 920-928 行附近初始化此字段为 0,那里初始化了其他字段。

Copilot uses AI. Check for mistakes.
#else
rt_uint8_t nb_tx_rb_pool[RT_CAN_NB_TX_FIFO_SIZE]; /**< The statically allocated pool for the non-blocking TX ring buffer. */
#endif /* RT_CAN_MALLOC_NB_TX_BUFFER */
rt_uint8_t send_mode_nonblocking; /**< Global send mode: 0=Blocking (default), 1=Non-blocking. */
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

[Best Practices/最佳实践]: Consider using rt_bool_t for boolean flag / 考虑使用 rt_bool_t 作为布尔标志

English: The field send_mode_nonblocking is used as a boolean flag (0 or 1), but is declared as rt_uint8_t. For better code clarity and consistency with RT-Thread conventions, consider using rt_bool_t type instead. This makes the intent clearer and follows the RTOS's type system for boolean values.

中文:字段 send_mode_nonblocking 用作布尔标志(0 或 1),但声明为 rt_uint8_t。为了更好的代码清晰度和与 RT-Thread 约定的一致性,请考虑使用 rt_bool_t 类型。这使意图更清晰,并遵循 RTOS 的布尔值类型系统。

Copilot uses AI. Check for mistakes.
}
else if (can->ops->sendmsg_nonblocking != RT_NULL)
{
if (can->send_mode_nonblocking == 1) {
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

[Maintainability/可维护性]: Magic number should use symbolic constant / 魔数应使用符号常量

English: The comparison "can->send_mode_nonblocking == 1" uses a magic number. It would be better to define symbolic constants for the send modes (e.g., RT_CAN_SENDMODE_BLOCKING = 0, RT_CAN_SENDMODE_NONBLOCKING = 1) to improve code readability and maintainability. This makes the code self-documenting and easier to understand.

中文:比较"can->send_mode_nonblocking == 1"使用了魔数。最好定义发送模式的符号常量(例如,RT_CAN_SENDMODE_BLOCKING = 0, RT_CAN_SENDMODE_NONBLOCKING = 1)以提高代码可读性和可维护性。这使代码自文档化并更易于理解。

Copilot uses AI. Check for mistakes.
}

if (can->ops->sendmsg_nonblocking != RT_NULL)
}
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

[Spelling/拼写]: Extra space before closing brace comment / 关闭大括号注释前有多余空格

English: There is a trailing space after the closing brace on line 1136 before the comment. According to RT-Thread coding standards, there should be no trailing spaces in the code.

中文:第 1136 行的右大括号后、注释前有尾随空格。根据 RT-Thread 编码标准,代码中不应有尾随空格。

Copilot uses AI. Check for mistakes.
Comment on lines +1136 to +1137
}
else if (can->ops->sendmsg_nonblocking != RT_NULL)
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

[Best Practices/最佳实践]: Inconsistent indentation in if-else block / if-else 块缩进不一致

English: The if-else block has inconsistent formatting. According to RT-Thread coding standards, braces should be on separate lines and consistently formatted. The "else if" on line 1137 should follow the same formatting style as the "if" block above it, with proper alignment and spacing.

中文:if-else 块的格式不一致。根据 RT-Thread 编码标准,大括号应该独占一行并保持一致的格式。第 1137 行的"else if"应该遵循与上面"if"块相同的格式样式,具有适当的对齐和间距。

Copilot uses AI. Check for mistakes.
Comment on lines +1140 to +1143
device = &(can->parent);
if (device->tx_complete != RT_NULL)
{
device->tx_complete(device, RT_NULL);
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

[Performance/性能]: Redundant device pointer assignment / 冗余的设备指针赋值

English: The variable 'device' is assigned inside the if block at line 1140, but it's only used on line 1141. This assignment can be avoided by directly accessing the device via "&(can->parent)" in the tx_complete check. This reduces unnecessary variable declarations and improves code efficiency, which is important in ISR context.

中文:变量 'device' 在第 1140 行的 if 块内被赋值,但它仅在第 1141 行使用。可以通过在 tx_complete 检查中直接访问"&(can->parent)"来避免此赋值。这减少了不必要的变量声明并提高了代码效率,这在 ISR 上下文中很重要。

Copilot uses AI. Check for mistakes.
{
tx_fifo->buffer[no].result = RT_CAN_SND_RESULT_ERR;
}

Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

[Spelling/拼写]: Extra blank line after condition check / 条件检查后有多余的空行

English: There is an unnecessary blank line at line 1133 after the closing brace. According to RT-Thread coding standards, avoid extra blank lines within function blocks unless separating logical sections.

中文:第 1133 行的右大括号后有一个不必要的空行。根据 RT-Thread 编码标准,除非分隔逻辑部分,否则应避免在函数块内使用额外的空行。

Copilot uses AI. Check for mistakes.
#else
rt_uint8_t nb_tx_rb_pool[RT_CAN_NB_TX_FIFO_SIZE]; /**< The statically allocated pool for the non-blocking TX ring buffer. */
#endif /* RT_CAN_MALLOC_NB_TX_BUFFER */
rt_uint8_t send_mode_nonblocking; /**< Global send mode: 0=Blocking (default), 1=Non-blocking. */
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

[Documentation/文档]: Missing documentation for send_mode_nonblocking field / 缺少 send_mode_nonblocking 字段的文档

English: The comment for send_mode_nonblocking field should be in English according to RT-Thread coding standards. All code comments should use English to maintain consistency. The current comment should be updated to proper English documentation format.

中文:根据 RT-Thread 编码标准,send_mode_nonblocking 字段的注释应使用英文。所有代码注释都应使用英文以保持一致性。当前注释应更新为正确的英文文档格式。

Copilot uses AI. Check for mistakes.
Comment on lines +1139 to +1145
if (can->send_mode_nonblocking == 1) {
device = &(can->parent);
if (device->tx_complete != RT_NULL)
{
device->tx_complete(device, RT_NULL);
}
}
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

[Bug/错误]: tx_complete callback invoked before messages are processed from ring buffer / 在从环形缓冲区处理消息之前调用 tx_complete 回调

English: The tx_complete callback is invoked (lines 1139-1145) BEFORE the while loop that processes pending messages from the ring buffer (lines 1146-1172). This is logically incorrect because:

  1. The tx_complete callback should notify that transmission is complete and space is available
  2. But messages are still pending in the ring buffer and haven't been sent yet
  3. This could lead to incorrect application behavior where the callback is called prematurely

The tx_complete callback should be moved AFTER the while loop completes, or should be called only when the ring buffer becomes empty. Consider the proper timing of this notification in relation to the actual message transmission.

中文:tx_complete 回调在处理环形缓冲区中待处理消息的 while 循环(第 1146-1172 行)之前被调用(第 1139-1145 行)。这在逻辑上是不正确的,因为:

  1. tx_complete 回调应通知传输完成且空间可用
  2. 但消息仍在环形缓冲区中等待且尚未发送
  3. 这可能导致应用程序行为不正确,因为回调被过早调用

tx_complete 回调应在 while 循环完成后移动,或者仅在环形缓冲区变空时调用。请考虑此通知与实际消息传输的正确时序关系。

Copilot uses AI. Check for mistakes.
@RCSN RCSN closed this Dec 17, 2025
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.

1 participant