fix(ymodem): improve send read loop and unify session cleanup

- Accumulate short reads, mark finishing on EOF
- Track begin/end callback states and cleanup on error paths
- Add ACK handling with retry/error counting in send flow

Author: wdfk-prog

components/utilities/ymodem/ry_sy.c

@@ -7,6 +7,7 @@
  * Date           Author       Notes
  * 2019-12-09     Steven Liu   the first version
  * 2021-04-14     Meco Man     Check the file path's legitimacy of 'sy' command
+ * 2026-02-01     wdfk-prog    update ymodem transfer behaviors
  */
 
 #include <rtthread.h>
@@ -157,20 +158,35 @@ static enum rym_code _rym_send_data(
 {
     struct custom_ctx *cctx = (struct custom_ctx *)ctx;
     rt_size_t read_size;
-    int retry_read;
+    int rlen;
 
     read_size = 0;
-    for (retry_read = 0; retry_read < 10; retry_read++)
+    /* Loop until we fill one YMODEM data block, hit EOF, or get a read error. */
+    while (read_size < len)
     {
-        read_size += read(cctx->fd, buf + read_size, len - read_size);
-        if (read_size == len)
+        rlen = read(cctx->fd, buf + read_size, len - read_size);
+        if (rlen > 0)
+        {
+            read_size += rlen;
+            if (read_size == len)
+                break;
+        }
+        else if (rlen == 0)
+        {
+            /* EOF: mark finishing so sender switches to EOT after padding. */
+            ctx->stage = RYM_STAGE_FINISHING;
             break;
+        }
+        else
+        {
+            /* Read error: abort transfer and report file error to the protocol. */
+            return RYM_ERR_FILE;
+        }
     }
 
     if (read_size < len)
     {
         rt_memset(buf + read_size, 0x1A, len - read_size);
-        ctx->stage = RYM_STAGE_FINISHING;
     }
 
     if (read_size > 128)

components/utilities/ymodem/ymodem.c

@@ -8,6 +8,7 @@
  * Date           Author       Notes
  * 2013-04-14     Grissiom     initial implementation
  * 2019-12-09     Steven Liu   add YMODEM send protocol
+ * 2026-02-01     wdfk-prog    update ymodem callbacks and error handling
  */
 
 #include <rthw.h>
@@ -180,6 +181,27 @@ static rt_err_t _rym_send_packet(
     return RT_EOK;
 }
 
+/**
+ * @brief Finalize a transfer, record error state, invoke end callback, and free buffer.
+ *
+ * @param ctx Transfer context.
+ * @param err Transfer result (RT_EOK on success, negative on failure).
+ */
+static void _rym_cleanup(struct rym_ctx *ctx, rt_err_t err)
+{
+    ctx->last_err = err;
+    if (ctx->on_end && ctx->begin_called && !ctx->end_called)
+    {
+        ctx->on_end(ctx, ctx->buf + 3, 128);
+        ctx->end_called = 1;
+    }
+    if (ctx->buf)
+    {
+        rt_free(ctx->buf);
+        ctx->buf = RT_NULL;
+    }
+}
+
 static rt_ssize_t _rym_putchar(struct rym_ctx *ctx, rt_uint8_t code)
 {
     rt_device_write(ctx->dev, 0, &code, sizeof(code));
@@ -254,7 +276,13 @@ static rt_err_t _rym_do_handshake(
 
     /* congratulations, check passed. */
     if (ctx->on_begin && ctx->on_begin(ctx, ctx->buf + 3, data_sz - 5) != RYM_CODE_ACK)
+    {
         return -RYM_ERR_CAN;
+    }
+    else
+    {
+        ctx->begin_called = 1;
+    }
 
     return RT_EOK;
 }
@@ -289,12 +317,17 @@ static rt_err_t _rym_do_send_handshake(
 
     /* congratulations, check passed. */
     if (ctx->on_begin && ctx->on_begin(ctx, ctx->buf + 3, data_sz - 5) != RYM_CODE_SOH)
+    {
         return -RYM_ERR_CODE;
+    }
+    else
+    {
+        ctx->begin_called = 1;
+    }
 
     code = RYM_CODE_SOH;
     _rym_send_packet(ctx, code, index);
 
-    rt_device_set_rx_indicate(ctx->dev, _rym_rx_ind);
     getc_ack = _rym_getchar(ctx);
 
     if (getc_ack != RYM_CODE_ACK)
@@ -440,25 +473,43 @@ static rt_err_t _rym_do_send_trans(struct rym_ctx *ctx)
     rt_size_t data_sz;
     rt_uint32_t index = 1;
     rt_uint8_t getc_ack;
+    rt_size_t errors = 0;
+    rt_uint8_t have_packet = 0;
 
     data_sz = _RYM_STX_PKG_SZ;
     while (1)
     {
-        if (!ctx->on_data)
+        if (!have_packet)
         {
-            return -RYM_ERR_CODE;
+            if (!ctx->on_data)
+            {
+                return -RYM_ERR_CODE;
+            }
+            /* Prepare the next payload only once; reuse it on retransmits. */
+            code = ctx->on_data(ctx, ctx->buf + 3, data_sz - 5);
+            have_packet = 1;
         }
-        code = ctx->on_data(ctx, ctx->buf + 3, data_sz - 5);
 
-        _rym_send_packet(ctx, code, index);
-        index++;
-        rt_device_set_rx_indicate(ctx->dev, _rym_rx_ind);
+        if (_rym_send_packet(ctx, code, index) != RT_EOK)
+        {
+            return -RYM_ERR_CODE;
+        }
 
         getc_ack = _rym_getchar(ctx);
         if (getc_ack != RYM_CODE_ACK)
         {
-            return -RYM_ERR_ACK;
+            if (getc_ack == RYM_CODE_CAN)
+                return -RYM_ERR_CAN;
+
+            if (++errors > RYM_MAX_ERRORS)
+                return -RYM_ERR_ACK;
+
+            /* Not ACK: retry the same packet until we hit the error limit. */
+            continue;
         }
+        errors = 0;
+        have_packet = 0;
+        index++;
 
         if (ctx->stage == RYM_STAGE_FINISHING)
             break;
@@ -478,7 +529,10 @@ static rt_err_t _rym_do_fin(struct rym_ctx *ctx)
     /* we already got one EOT in the caller. invoke the callback if there is
      * one. */
     if (ctx->on_end)
+    {
         ctx->on_end(ctx, ctx->buf + 3, 128);
+        ctx->end_called = 1;
+    }
 
     _rym_putchar(ctx, RYM_CODE_NAK);
     code = _rym_read_code(ctx, RYM_WAIT_PKG_TICK);
@@ -538,7 +592,6 @@ static rt_err_t _rym_do_send_fin(struct rym_ctx *ctx)
     rt_uint8_t getc_ack;
 
     data_sz = _RYM_SOH_PKG_SZ;
-    rt_device_set_rx_indicate(ctx->dev, _rym_rx_ind);
 
     _rym_putchar(ctx, RYM_CODE_EOT);
     getc_ack = _rym_getchar(ctx);
@@ -564,7 +617,13 @@ static rt_err_t _rym_do_send_fin(struct rym_ctx *ctx)
     }
 
     if (ctx->on_end && ctx->on_end(ctx, ctx->buf + 3, data_sz - 5) != RYM_CODE_SOH)
+    {
         return -RYM_ERR_CODE;
+    }
+    else
+    {
+        ctx->end_called = 1;
+    }
 
     code = RYM_CODE_SOH;
 
@@ -582,32 +641,32 @@ static rt_err_t _rym_do_recv(
     rt_err_t err;
 
     ctx->stage = RYM_STAGE_NONE;
+    ctx->begin_called = 0;
+    ctx->end_called = 0;
+    ctx->last_err = RT_EOK;
 
     ctx->buf = rt_malloc(_RYM_STX_PKG_SZ);
     if (ctx->buf == RT_NULL)
         return -RT_ENOMEM;
 
     err = _rym_do_handshake(ctx, handshake_timeout);
     if (err != RT_EOK)
-    {
-        rt_free(ctx->buf);
-        return err;
-    }
+        goto __cleanup;
     while (1)
     {
         err = _rym_do_trans(ctx);
+        if (err != RT_EOK)
+            goto __cleanup;
 
         err = _rym_do_fin(ctx);
         if (err != RT_EOK)
-        {
-            rt_free(ctx->buf);
-            return err;
-        }
+            goto __cleanup;
 
         if (ctx->stage == RYM_STAGE_FINISHED)
             break;
     }
-    rt_free(ctx->buf);
+__cleanup:
+    _rym_cleanup(ctx, err);
     return err;
 }
 
@@ -618,33 +677,27 @@ static rt_err_t _rym_do_send(
     rt_err_t err;
 
     ctx->stage = RYM_STAGE_NONE;
+    ctx->begin_called = 0;
+    ctx->end_called = 0;
+    ctx->last_err = RT_EOK;
 
     ctx->buf = rt_malloc(_RYM_STX_PKG_SZ);
     if (ctx->buf == RT_NULL)
         return -RT_ENOMEM;
 
     err = _rym_do_send_handshake(ctx, handshake_timeout);
     if (err != RT_EOK)
-    {
-        rt_free(ctx->buf);
-        return err;
-    }
+        goto __cleanup;
 
     err = _rym_do_send_trans(ctx);
     if (err != RT_EOK)
-    {
-        rt_free(ctx->buf);
-        return err;
-    }
+        goto __cleanup;
 
     err = _rym_do_send_fin(ctx);
     if (err != RT_EOK)
-    {
-        rt_free(ctx->buf);
-        return err;
-    }
-
-    rt_free(ctx->buf);
+        goto __cleanup;
+__cleanup:
+    _rym_cleanup(ctx, err);
     return err;
 }
 
@@ -666,9 +719,12 @@ rt_err_t rym_recv_on_device(
     _rym_the_ctx = ctx;
 
     ctx->on_begin = on_begin;
-    ctx->on_data  = on_data;
-    ctx->on_end   = on_end;
-    ctx->dev      = dev;
+    ctx->on_data = on_data;
+    ctx->on_end = on_end;
+    ctx->begin_called = 0;
+    ctx->end_called = 0;
+    ctx->last_err = RT_EOK;
+    ctx->dev = dev;
     rt_sem_init(&ctx->sem, "rymsem", 0, RT_IPC_FLAG_FIFO);
 
     odev_rx_ind = dev->rx_indicate;
@@ -723,9 +779,12 @@ rt_err_t rym_send_on_device(
     _rym_the_ctx = ctx;
 
     ctx->on_begin = on_begin;
-    ctx->on_data  = on_data;
-    ctx->on_end   = on_end;
-    ctx->dev      = dev;
+    ctx->on_data = on_data;
+    ctx->on_end = on_end;
+    ctx->begin_called = 0;
+    ctx->end_called = 0;
+    ctx->last_err = RT_EOK;
+    ctx->dev = dev;
     rt_sem_init(&ctx->sem, "rymsem", 0, RT_IPC_FLAG_FIFO);
 
     odev_rx_ind = dev->rx_indicate;

components/utilities/ymodem/ymodem.h

@@ -9,6 +9,7 @@
  * 2013-04-14     Grissiom     initial implementation
  * 2019-12-09     Steven Liu   add YMODEM send protocol
  * 2022-08-04     Meco Man     move error codes to rym_code to silence warnings
+ * 2026-02-01     wdfk-prog    update ymodem callbacks and error handling
  */
 
 #ifndef __YMODEM_H__
@@ -81,13 +82,22 @@ enum rym_stage
 };
 
 struct rym_ctx;
-/* When receiving files, the buf will be the data received from ymodem protocol
+/**
+ * @brief YMODEM callback signature.
+ *
+ * @param ctx The context of the current session.
+ *
+ * @note When receiving files, the buf will be the data received from ymodem protocol
  * and the len is the data size.
  *
  * When sending files, the len is the buf size in RYM. The callback need to
  * fill the buf with data to send. Returning RYM_CODE_EOT will terminate the
  * transfer and the buf will be discarded. Any other return values will cause
  * the transfer continue.
+ *
+ * @note Keep this typedef unchanged for compatibility with external packages.
+ *       To allow error-aware handling without breaking ABI, add state fields
+ *       (e.g. ctx->last_err) in rym_ctx for callbacks to inspect.
  */
 typedef enum rym_code(*rym_callback)(struct rym_ctx *ctx, rt_uint8_t *buf, rt_size_t len);
 
@@ -98,14 +108,37 @@ typedef enum rym_code(*rym_callback)(struct rym_ctx *ctx, rt_uint8_t *buf, rt_si
  */
 struct rym_ctx
 {
+    /**
+     * @brief Data callback for each received/sent block.
+     */
     rym_callback on_data;
+    /**
+     * @brief Begin callback for the initial header block.
+     */
     rym_callback on_begin;
+    /**
+     * @brief End callback for session finalization.
+     *        Callers should check ctx->last_err to distinguish success vs failure.
+     */
     rym_callback on_end;
     /* When error happened, user need to check this to get when the error has
      * happened. */
     enum rym_stage stage;
     /* user could get the error content through this */
     rt_uint8_t *buf;
+    /**
+     * @brief Callback lifecycle state: set when on_begin succeeds.
+     */
+    rt_uint8_t begin_called;
+    /**
+     * @brief Callback lifecycle state: set when on_end is invoked.
+     */
+    rt_uint8_t end_called;
+    /**
+     * @brief Last transfer error seen by the core state machine.
+     *        on_end can inspect this to distinguish success vs failure.
+     */
+    rt_err_t last_err;
 
     struct rt_semaphore sem;