Skip to content

[utilities][ymodem] Resolve the compatibility issues with receiving files via the YMODEM protocol - #12051

Open
lvtaolong wants to merge 1 commit into
RT-Thread:masterfrom
lvtaolong:lvtaolong-patch-1
Open

[utilities][ymodem] Resolve the compatibility issues with receiving files via the YMODEM protocol#12051
lvtaolong wants to merge 1 commit into
RT-Thread:masterfrom
lvtaolong:lvtaolong-patch-1

Conversation

@lvtaolong

@lvtaolong lvtaolong commented Nov 12, 2026

Copy link
Copy Markdown

add 40ms delay between in sending ACK and sending C character

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

[

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

通过ymodem协议发送文件给单板的功能，当接收方在收到第二个EOT后，要发送ACK和C字符，但是发送两个字符之前没有延迟，如果两者间隔时间极短，而发送方的串口驱动的超时时间或中断处理不当，可能导致吞掉了其中某一个字节（比如只读到了 0x06，漏掉了 0x43），最终导致发送方认为用ymodem发送文件失败了。

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

在_rym_putchar(ctx, RYM_CODE_ACK)--发送ACK字符和_rym_putchar(ctx, RYM_CODE_C)--发送C字符之间插入40ms的延时

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

  • BSP: /bsp/stm32/stm32f429-atk-apollo
  • .config: 默认配置 + RT_USING_RYM+ YMODEM_USING_CRC_TABLE
  • action: 使用 GitHub Actions；本地ENV环境V2.0.0+Scons编译通过，lvtaolong-patch-1 55e74d8
    修改前后的固件版本在stm32f427 MCU的单板上测试通过，确认该修改能解决问题。
    ]

当前拉取/合并请求的状态 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
  • 已经使用clang-format 源码格式化工具确保格式符合RT-Thread代码规范 This PR has been formatted with clang-format and complies with RT-Thread code specification
  • 如果是新增bsp, 已经添加ci检查到.github/ALL_BSP_COMPILE.json 详细请参考链接BSP自查

@lvtaolong
Update ymodem.c
55e74d8 
add 40ms delay between in sending ACK and sending C character
@CLAassistant

CLAassistant commented Nov 12, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions

github-actions Bot commented Nov 12, 2026

Copy link
Copy Markdown

👋 感谢您对 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

  • Use workflow from 保持默认分支（通常为 master）
    Keep the default branch (usually master) in Use workflow from
  • 在 branch 输入框填写 PR 分支 lvtaolong-patch-1
    Enter PR branch lvtaolong-patch-1 in the branch field
  • 设置需排除的文件/目录（目录请以"/"结尾）
    Set files/directories to exclude (directories should end with "/")
  1. 等待工作流完成 | Wait for the workflow to complete
    格式化后的代码将作为独立提交推送至你的分支。
    The formatting changes will be pushed to your branch as a separate commit.

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

@lvtaolong lvtaolong changed the title 解决通通过ymodem协议接收文件的兼容性问题 Resolve the compatibility issues with receiving files via the YMODEM protocol Nov 12, 2026
@lvtaolong lvtaolong changed the title Resolve the compatibility issues with receiving files via the YMODEM protocol [utilities][ymodem] Resolve the compatibility issues with receiving files via the YMODEM protocol Nov 12, 2026
@github-actions

github-actions Bot commented Nov 12, 2026

Copy link
Copy Markdown

📌 Code Review Assignment

🏷️ Tag: components

Reviewers: @Maihuanyi

Changed Files (Click to expand)
  • components/utilities/ymodem/ymodem.c

🏷️ Tag: components_utilities_ymodem

Reviewers: @wdfk-prog

Changed Files (Click to expand)
  • components/utilities/ymodem/ymodem.c

📊 Current Review Status (Last Updated: 2026-11-12 18:26 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 reviewed Nov 13, 2026
View reviewed changes

@Rbb666 Rbb666 left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

已核对改动仅在第二个 EOT 后的 ACK 与 C 之间增加线程延时。作者已提供 STM32 板端验证记录；已批准此前待运行的仓库 CI，CLA 仍未签署。当前单一提交无需压缩，请把 Update ymodem.c 改为有模块前缀且说明行为的标题，例如 [utilities][ymodem] Delay the final C for sender compatibility。补充 CI 诊断：静态分析运行 https://github.com/RT-Thread/rt-thread/actions/runs/43193723997 的错误位于未修改的 K230 PWM、var_export 和本分支不存在的路径，ymodem.c 自身没有报告错误。精确 head 的 format_ignore.py 使用 get_changed_files(HEAD, origin/master)，git_utils.py 直接对两个端点做 git diff，因主线推进把主线新增/修改也加入了本分支待扫描列表。建议先同步主线后重新检查，或由 CI 维护者修正 PR 文件选择范围；本次不把这些错误归因于新增的 YMODEM 延时。

Comment thread components/utilities/ymodem/ymodem.c
* or interrupt handling, one of the two bytes may get swallowed (e.g. only 0x06 is read and 0x43 is missed).
* So add 40ms delay here.
*/
rt_thread_mdelay(40);

@Rbb666 Rbb666 Nov 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[建议] 请补充复现所用发送工具及版本、波特率，以及 40 ms 的选择依据，并确认连续文件和标准发送端在加入间隔后均能完成最后一个空包握手。这里对所有调用者固定增加一次延时，保留明确的兼容性测试条件有助于后续维护，避免仅用“字节粘连”解释发送端读取问题。

@wdfk-prog

wdfk-prog commented Nov 13, 2026

Copy link
Copy Markdown
Contributor
  • 问题不是这样子解决的.这个是针对你的环境硬件使用情况的解决方案,对于其他环境下的大部分是不需要这样子处理的
  • 发送方的串口驱动的超时时间或中断处理不当;需要去解决接收方的问题,而不是修改发送方

@Rbb666 Rbb666 added the -1 No vote label Nov 13, 2026
@Rbb666

Rbb666 commented Nov 13, 2026

Copy link
Copy Markdown
Member
  • 问题不是这样子解决的.这个是针对你的环境硬件使用情况的解决方案,对于其他环境下的大部分是不需要这样子处理的
  • 发送方的串口驱动的超时时间或中断处理不当;需要去解决接收方的问题,而不是修改发送方

赞同

@github-actions

github-actions Bot commented Nov 13, 2026

Copy link
Copy Markdown

MemBrowse Memory Report · Explore what changed →

No memory changes detected for:

Catch memory regressions in CI with MemBrowse. Free for open source.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Rbb666 Rbb666 Rbb666 left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

-1 No vote Component

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lvtaolong @CLAassistant @wdfk-prog @Rbb666