Skip to content

Conversation

wdfk-prog
Copy link
Contributor

@wdfk-prog wdfk-prog commented Sep 20, 2025

  • 新增支持非阻塞模式下的CAN消息发送,包括软件环形缓冲区管理和动态内存分配选项。
  • 完善相关注释和错误处理。
  • 更新示例代码以展示阻塞与非阻塞发送的使用方式,并修正部分结构体字段命名与宏定义

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

[

为什么提交这份PR (why to submit this PR)

你的解决方案是什么 (what is your solution)

请提供验证的bsp和config (provide the config and bsp)

  • BSP:
  • .config:
  • action:

]

当前拉取/合并请求的状态 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自查

- 新增支持非阻塞模式下的CAN消息发送,包括软件环形缓冲区管理和动态内存分配选项。
- 完善相关注释和错误处理。
- 更新示例代码以展示阻塞与非阻塞发送的使用方式,并修正部分结构体字段命名与宏定义
Copy link

📌 Code Review Assignment

🏷️ Tag: bsp_stm32

Reviewers: @Liang1795 @hamburger-os @wdfk-prog

Changed Files (Click to expand)
  • bsp/stm32/libraries/HAL_Drivers/drivers/drv_can.c

🏷️ Tag: components

Reviewers: @Maihuanyi

Changed Files (Click to expand)
  • components/drivers/can/Kconfig
  • components/drivers/can/dev_can.c
  • components/drivers/include/drivers/dev_can.h

🏷️ Tag: components_driver_can

Reviewers: @wdfk-prog

Changed Files (Click to expand)
  • components/drivers/can/Kconfig
  • components/drivers/can/dev_can.c

📊 Current Review Status (Last Updated: 2025-09-20 15:03 CST)


📝 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.

@Rbb666 Rbb666 requested a review from Copilot September 21, 2025 01:30
Copy link
Contributor

@Copilot 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 implements a non-blocking send mechanism for the RT-Thread CAN driver framework, enhancing the existing blocking-only transmission capabilities. The implementation adds support for ISR-safe message transmission through software ring buffers and dynamic memory allocation options.

  • Added non-blocking CAN message transmission with software ring buffer management
  • Enhanced API documentation and example code with bilingual (English/Chinese) comments
  • Introduced new configuration options for buffer sizing and memory allocation strategies

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 10 comments.

File Description
components/drivers/include/drivers/dev_can.h Added non-blocking flag to rt_can_msg struct, new ring buffer fields to device struct, and comprehensive API documentation
components/drivers/can/dev_can.c Implemented core non-blocking transmission logic, enhanced error handling, and added ISR-safe buffer management
components/drivers/can/Kconfig Added configuration options for non-blocking buffer size and dynamic allocation
bsp/stm32/libraries/HAL_Drivers/drivers/drv_can.c Implemented STM32-specific non-blocking send function and updated ops structure

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2006-2025, RT-Thread Development Team
* Copyright (c) 2006-2025 RT-Thread Development Team
Copy link
Preview

Copilot AI Sep 21, 2025

Choose a reason for hiding this comment

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

English: Missing comma after year range in copyright header.
中文: 版权头中年份范围后缺少逗号。

Should be: * Copyright (c) 2006-2025, RT-Thread Development Team

Suggested change
* Copyright (c) 2006-2025 RT-Thread Development Team
* Copyright (c) 2006-2025, RT-Thread Development Team

Copilot uses AI. Check for mistakes.

rt_uint32_t rsv : 1; /* Reserved bit */
rt_uint32_t len : 8; /* Data length code (DLC) */
rt_uint32_t priv : 8; /* Private data, used for specific driver implementations */
rt_int32_t hdr_index : 8; /* Should be defined as:rx.FilterMatchIndex,which should be changed to rt_int32_t hdr_index : 8*/
Copy link
Preview

Copilot AI Sep 21, 2025

Choose a reason for hiding this comment

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

English: The comment has formatting issues and unclear wording. The comment should be rewritten for clarity.
中文: 注释格式有问题且表达不清楚。应该重新编写注释以提高清晰度。

Suggested improvement:

rt_int32_t hdr_index : 8;       /* Hardware filter index that matched this message (-1 for default queue) */
Suggested change
rt_int32_t hdr_index : 8; /* Should be defined as:rx.FilterMatchIndex,which should be changed to rt_int32_t hdr_index : 8*/
rt_int32_t hdr_index : 8; /* Hardware filter index that matched this message (-1 for default queue) */

Copilot uses AI. Check for mistakes.

Comment on lines +476 to +479
rt_uint32_t reserved : 3;
#else
rt_uint32_t rxfifo : 2; /* Redefined to return :CAN RX FIFO0/CAN RX FIFO1*/
rt_uint32_t reserved : 5;
Copy link
Preview

Copilot AI Sep 21, 2025

Choose a reason for hiding this comment

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

English: The reserved bit fields have different sizes (3 vs 5) between CAN-FD and non-CAN-FD configurations. This inconsistency could lead to confusion and potential alignment issues.
中文: 在CAN-FD和非CAN-FD配置之间,保留位字段的大小不同(3位对5位)。这种不一致可能导致混淆和潜在的对齐问题。

Consider using consistent naming or documenting why different sizes are needed.

Suggested change
rt_uint32_t reserved : 3;
#else
rt_uint32_t rxfifo : 2; /* Redefined to return :CAN RX FIFO0/CAN RX FIFO1*/
rt_uint32_t reserved : 5;
rt_uint32_t reserved : 5; /* Consistent reserved bits for alignment */
#else
rt_uint32_t rxfifo : 2; /* Redefined to return :CAN RX FIFO0/CAN RX FIFO1*/
rt_uint32_t reserved : 5; /* Consistent reserved bits for alignment */

Copilot uses AI. Check for mistakes.

rt_uint32_t fd_frame : 1; /* CAN-FD frame indicator */
rt_uint32_t brs : 1; /* Bit-rate switching indicator for CAN-FD */
rt_uint32_t rxfifo : 2; /* Redefined to return :CAN RX FIFO0/CAN RX FIFO1*/
rt_uint32_t reserved : 3;
Copy link
Preview

Copilot AI Sep 21, 2025

Choose a reason for hiding this comment

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

English: The reserved bit fields have different sizes (3 vs 5) between CAN-FD and non-CAN-FD configurations. This inconsistency could lead to confusion and potential alignment issues.
中文: 在CAN-FD和非CAN-FD配置之间,保留位字段的大小不同(3位对5位)。这种不一致可能导致混淆和潜在的对齐问题。

Consider using consistent naming or documenting why different sizes are needed.

Suggested change
rt_uint32_t reserved : 3;
rt_uint32_t reserved : 5;

Copilot uses AI. Check for mistakes.

@@ -1,12 +1,13 @@
/*
* Copyright (c) 2006-2025, RT-Thread Development Team
* Copyright (c) 2006-2025 RT-Thread Development Team
Copy link
Preview

Copilot AI Sep 21, 2025

Choose a reason for hiding this comment

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

English: Missing comma after year range in copyright header.
中文: 版权头中年份范围后缺少逗号。

Should be: * Copyright (c) 2006-2025, RT-Thread Development Team

Suggested change
* Copyright (c) 2006-2025 RT-Thread Development Team
* Copyright (c) 2006-2025, RT-Thread Development Team

Copilot uses AI. Check for mistakes.

@@ -461,7 +590,7 @@ static rt_ssize_t rt_can_read(struct rt_device *dev,
struct rt_can_device *can;

RT_ASSERT(dev != RT_NULL);
if (size == 0) return 0;
if (size == 0) return -RT_EEMPTY;
Copy link
Preview

Copilot AI Sep 21, 2025

Choose a reason for hiding this comment

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

English: The error code -RT_EEMPTY is inappropriate for zero-size buffer inputs. This error code typically indicates an empty container, not an invalid parameter.
中文: 错误码-RT_EEMPTY不适合用于零大小缓冲区输入。这个错误码通常表示容器为空,而不是无效参数。

Should use -RT_EINVAL for invalid size parameter instead.

Suggested change
if (size == 0) return -RT_EEMPTY;
if (size == 0) return -RT_EINVAL;

Copilot uses AI. Check for mistakes.

if (size == 0) return 0;
RT_ASSERT(buffer != RT_NULL);

if (size == 0) return -RT_EEMPTY;
Copy link
Preview

Copilot AI Sep 21, 2025

Choose a reason for hiding this comment

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

English: The error code -RT_EEMPTY is inappropriate for zero-size buffer inputs. This error code typically indicates an empty container, not an invalid parameter.
中文: 错误码-RT_EEMPTY不适合用于零大小缓冲区输入。这个错误码通常表示容器为空,而不是无效参数。

Should use -RT_EINVAL for invalid size parameter instead.

Suggested change
if (size == 0) return -RT_EEMPTY;
if (size == 0) return -RT_EINVAL;

Copilot uses AI. Check for mistakes.

@@ -470,22 +599,64 @@ static rt_ssize_t rt_can_read(struct rt_device *dev,
return _can_int_rx(can, buffer, size);
}

return 0;
return -RT_ENOSYS;
Copy link
Preview

Copilot AI Sep 21, 2025

Choose a reason for hiding this comment

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

English: Using -RT_ENOSYS (function not implemented) as a fallback return value may be misleading when the device is configured but the specific mode is not supported.
中文: 使用-RT_ENOSYS(功能未实现)作为后备返回值,当设备已配置但不支持特定模式时可能会产生误导。

Consider using -RT_ENOTSUP (operation not supported) for better semantic accuracy.

Suggested change
return -RT_ENOSYS;
return -RT_ENOTSUP;

Copilot uses AI. Check for mistakes.

@@ -496,7 +667,7 @@ static rt_ssize_t rt_can_write(struct rt_device *dev,
return _can_int_tx(can, buffer, size);
}
}
return 0;
return -RT_ENOSYS;
Copy link
Preview

Copilot AI Sep 21, 2025

Choose a reason for hiding this comment

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

English: Using -RT_ENOSYS (function not implemented) as a fallback return value may be misleading when the device is configured but the specific mode is not supported.
中文: 使用-RT_ENOSYS(功能未实现)作为后备返回值,当设备已配置但不支持特定模式时可能会产生误导。

Consider using -RT_ENOTSUP (operation not supported) for better semantic accuracy.

Suggested change
return -RT_ENOSYS;
return -RT_ENOTSUP;

Copilot uses AI. Check for mistakes.

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2006-2024 RT-Thread Development Team
* Copyright (c) 2006-2025 RT-Thread Development Team
Copy link
Preview

Copilot AI Sep 21, 2025

Choose a reason for hiding this comment

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

English: Missing comma after year range in copyright header.
中文: 版权头中年份范围后缺少逗号。

Should be: * Copyright (c) 2006-2025, RT-Thread Development Team

Suggested change
* Copyright (c) 2006-2025 RT-Thread Development Team
* Copyright (c) 2006-2025, RT-Thread Development Team

Copilot uses AI. Check for mistakes.

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