Skip to content

[componnents][finsh] 适配serial_v2提高效率，减少内存占用 #10607

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.

Sign up for GitHub

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

Closed
Ryan-CW-Code wants to merge 1 commit into from
Closed

[componnents][finsh] 适配serial_v2提高效率，减少内存占用 #10607

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion components/finsh/shell.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@

#ifdef RT_USING_FINSH

#include <rtdef.h>
#include <rtdevice.h>
#include "shell.h"
#include "msh.h"

Expand Down Expand Up @@ -177,7 +179,8 @@ int finsh_getchar(void)
{
return -1; /* EOF */
}

/* Temporarily retain the distinction */
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

The comment should be in English for consistency with the codebase. Consider changing to 'Temporarily retain the distinction' or a more descriptive comment explaining why this distinction exists.

Suggested change
/* Temporarily retain the distinction */
/* Retain distinction between serial device versions for compatibility; refactoring pending. */

Copilot uses AI. Check for mistakes.
#ifndef RT_USING_SERIAL_V2
while (rt_device_read(device, -1, &ch, 1) != 1)
{
rt_sem_take(&shell->rx_sem, RT_WAITING_FOREVER);
Expand All @@ -190,6 +193,19 @@ int finsh_getchar(void)
}
}
}
#else
while (rt_device_read(device, -1, &ch, 1) != 1)
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

The serial_v2 implementation uses a tight loop without any yield or delay mechanism. This could lead to high CPU usage when waiting for input. Consider adding a small delay or yield to prevent excessive CPU consumption.

Copilot uses AI. Check for mistakes.
{
if (shell->device != device)
{
device = shell->device;
if (device == RT_NULL)
{
return -1;
}
}
}
#endif
return ch;
#endif /* RT_USING_POSIX_STDIO */
#else
Expand All @@ -199,6 +215,7 @@ int finsh_getchar(void)
}

#if !defined(RT_USING_POSIX_STDIO) && defined(RT_USING_DEVICE)
#ifndef RT_USING_SERIAL_V2
static rt_err_t finsh_rx_ind(rt_device_t dev, rt_size_t size)
{
RT_ASSERT(shell != RT_NULL);
Expand All @@ -208,6 +225,7 @@ static rt_err_t finsh_rx_ind(rt_device_t dev, rt_size_t size)

return RT_EOK;
}
#endif

/**
* @ingroup group_finsh
Expand All @@ -231,8 +249,13 @@ void finsh_set_device(const char *device_name)
/* check whether it's a same device */
if (dev == shell->device) return;
/* open this device and set the new device in finsh shell */
#ifndef RT_USING_SERIAL_V2
if (rt_device_open(dev, RT_DEVICE_OFLAG_RDWR | RT_DEVICE_FLAG_INT_RX | \
RT_DEVICE_FLAG_STREAM) == RT_EOK)
#else
if (rt_device_open(dev, RT_DEVICE_OFLAG_RDWR | RT_DEVICE_FLAG_RX_BLOCKING | \
RT_DEVICE_FLAG_TX_BLOCKING | RT_DEVICE_FLAG_STREAM) == RT_EOK)
#endif
{
if (shell->device != RT_NULL)
{
Expand All @@ -246,7 +269,12 @@ void finsh_set_device(const char *device_name)
shell->line_curpos = shell->line_position = 0;

shell->device = dev;
#ifndef RT_USING_SERIAL_V2
rt_device_set_rx_indicate(dev, finsh_rx_ind);
#else
rt_int32_t rx_timeout = RT_WAITING_FOREVER;
rt_device_control(dev, RT_SERIAL_CTRL_SET_RX_TIMEOUT, (void *)&rx_timeout);
Copy link

Copilot AI Aug 14, 2025

Choose a reason for hiding this comment

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

[nitpick] The variable declaration inside the conditional block reduces code clarity. Consider declaring rx_timeout at the beginning of the function scope or using a more descriptive variable name to indicate its purpose.

Suggested change
rt_device_control(dev, RT_SERIAL_CTRL_SET_RX_TIMEOUT, (void *)&rx_timeout);
rt_device_control(dev, RT_SERIAL_CTRL_SET_RX_TIMEOUT, (void *)&serial_rx_timeout);

Copilot uses AI. Check for mistakes.
#endif
}
}

Expand Down Expand Up @@ -924,7 +952,10 @@ int finsh_system_init(void)
FINSH_THREAD_PRIORITY, 10);
#endif /* RT_USING_HEAP */

#ifndef RT_USING_SERIAL_V2
rt_sem_init(&(shell->rx_sem), "shrx", 0, 0);
#endif

finsh_set_prompt_mode(1);

if (tid != NULL && result == RT_EOK)
Expand Down
2 changes: 2 additions & 0 deletions components/finsh/shell.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ enum input_stat
};
struct finsh_shell
{
#ifndef RT_USING_SERIAL_V2
struct rt_semaphore rx_sem;
#endif

enum input_stat stat;

Expand Down
Loading