Skip to content

bsp:k230:add support for temperature sensor driver #10609

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 20, 2025

Conversation

DannyRay019
Copy link
Contributor

Added a temperature sensor driver and a test file test_ts.c.
The test uses temperature sensor to measure the chip temperature,
to check if the driver works correctly.

Signed-off-by: XU HU [email protected]

@github-actions github-actions bot added the BSP label Aug 17, 2025
Copy link

github-actions bot commented Aug 17, 2025

📌 Code Review Assignment

🏷️ Tag: bsp_k230

Reviewers: unicornx

Changed Files (Click to expand)
  • bsp/k230/.config
  • bsp/k230/board/Kconfig
  • bsp/k230/drivers/interdrv/ts/SConscript
  • bsp/k230/drivers/interdrv/ts/drv_ts.c
  • bsp/k230/drivers/interdrv/ts/drv_ts.h
  • bsp/k230/drivers/utest/SConscript
  • bsp/k230/drivers/utest/test_ts.c
  • bsp/k230/rtconfig.h

📊 Current Review Status (Last Updated: 2025-08-19 16:45 CST)

  • unicornx 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.

@unicornx unicornx self-requested a review August 18, 2025 00:09
Copy link
Contributor

@unicornx unicornx left a comment

Choose a reason for hiding this comment

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

第一次 review。

请不要把 vendor 的代码直接拿过来,写得不合适的地方要把把关,能改进的要改进。

@@ -4,6 +4,11 @@ menu "Drivers Configuration"
bool "Enable ADC"
select RT_USING_ADC
default n

config BSP_USING_TS
bool "Enable TS"
Copy link
Contributor

Choose a reason for hiding this comment

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

建议 menu 上不要用 TS 这个缩写,不是啥专有名词,不好理解,直接用全称吧。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

这里sdk上的全程是tsensor,用这个吗,还是temperature_sensor

ts_dev = (rt_device_t)rt_device_find(TS_DEV_NAME);
if (ts_dev == RT_NULL)
{
uassert_false(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

可以对 ts_dev 直接 uassert


if(rt_device_open(ts_dev, RT_DEVICE_OFLAG_RDWR) != RT_EOK)
{
LOG_E("open %s device failed!\n", TS_DEV_NAME);
Copy link
Contributor

Choose a reason for hiding this comment

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

用 uassert

rt_uint32_t cnt;
double temp = 0;
ts_open();
for(cnt=0; cnt<5; cnt++)
Copy link
Contributor

Choose a reason for hiding this comment

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

代码不要挤在一起,关键字,变量名之间用空格隔开
例子:

for (cnt = 0; cnt < 5; cnt++)

reval = rt_device_read(ts_dev, 0, &temp, sizeof(double));
if(reval <= 0)
{
uassert_false(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

直接对 reval 进行 uassert

}

static void tsensor_stop(void) {
if (ts_base_addr == RT_NULL) return; // Ensure base address is set
Copy link
Contributor

Choose a reason for hiding this comment

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

如果要检查错误,那就应该第一时间返回错误,但这个函数返回了 void!

}

static int tsensor_read_data(uint16_t *data, uint32_t timeout_ms) {
if (ts_base_addr == RT_NULL || data == RT_NULL) return -1; // Ensure base address is set
Copy link
Contributor

Choose a reason for hiding this comment

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

不要返回立即数,用恰当的错误返回,不要用立即数。
全文检查。不再赘述


static rt_err_t ts_deivce_open(rt_device_t dev, rt_uint16_t oflag)
{
tsensor_init();
Copy link
Contributor

Choose a reason for hiding this comment

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

初始化如果失败了怎么办,仍然返回ok?

{
if(sizeof(double) != size) {
LOG_E("%s invalid buffer size %u\n", __func__, size);
return 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

返回值 0 一般代表成功,即 RT_EOK。 可是这里是要返回失败!
全文检查不再赘述

Copy link
Contributor

Choose a reason for hiding this comment

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

测试用例没有覆盖 ts_device_control, 建议增加

@DannyRay019
Copy link
Contributor Author

收到,我再继续改一下

Copy link
Contributor

@unicornx unicornx left a comment

Choose a reason for hiding this comment

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

第二次 review,只有一个小问题,其他 LGTM。


config BSP_USING_TS
bool "Enable TS"
select RT_USING_TS
Copy link
Contributor

Choose a reason for hiding this comment

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

“Enable Temperature Sensor”
写全称吧,给人看的,不要简写。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

收到

@unicornx unicornx requested a review from Rbb666 August 19, 2025 08:27
@unicornx
Copy link
Contributor

@Rbb666 please review and merge, thanks.

@unicornx
Copy link
Contributor

@DannyRay019 CI Check 有错误,请用 format 工具检查一下。

Added a temperature sensor driver and a test file test_ts.c.
The test uses temperature sensor to measure the chip temperature,
to check if the driver works correctly.

Signed-off-by: XU HU [email protected]
@unicornx
Copy link
Contributor

@Rbb666 检查完毕,请 review & merge,谢谢

@Rbb666 Rbb666 merged commit 60b3ccb into RT-Thread:master Aug 20, 2025
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants