Fix file write loop

8 files changed, 91 insertions, 30 deletions

diff --git a/apps/system/zmodem/Kconfig b/apps/system/zmodem/Kconfig
index aa6da18e2..120aa702b 100644
--- a/apps/system/zmodem/Kconfig
+++ b/apps/system/zmodem/Kconfig
@@ -119,10 +119,18 @@ config SYSTEM_ZMODEM_SENDBRAK
 		Can not yet send BREAK
 
 config SYSTEM_ZMODEM_RESPTIME
-	bool "Response time"
+	int "Response time"
 	default 10
 	---help---
-		Response time for sender to respond to requests.
+		Response time for remote peer to respond to requests.
+
+config SYSTEM_ZMODEM_CONNTIME
+	int "Rz startup time"
+	default 30
+	---help---
+		When rz starts, it must wait a for the remote end to start the file
+		transfer.  This may take longer than the normal response time.  This
+		value may be set to tune that longer timeout value.
 
 config SYSTEM_ZMODEM_SERIALNO
 	int "Serial number"
diff --git a/apps/system/zmodem/README.txt b/apps/system/zmodem/README.txt
index 4217e66aa..9048a82d9 100755
--- a/apps/system/zmodem/README.txt
+++ b/apps/system/zmodem/README.txt
@@ -65,6 +65,7 @@ Using NuttX Zmodem with a Linux Host
     using 9600 baud for the data transfers -- high rates may result in data
     overruns):
 
+      $ sudo stty -F /dev/ttyS0 raw      # Puts the TTY in raw transfer mode
       $ sudo stty -F /dev/ttyS0 9600     # Select 9600 BAUD
       $ sudo stty -F /dev/ttyS0 crtscts  # Enables CTS/RTS handshaking
       $ sudo stty -F /dev/ttyS0          # Show the TTY configuration
@@ -105,6 +106,7 @@ Using NuttX Zmodem with a Linux Host
     9600 baud for the data transfers -- high rates may result in data
     overruns):
 
+      $ sudo stty -F /dev/ttyS0 raw      # Puts the TTY in raw transfer mode
       $ sudo stty -F /dev/ttyS0 9600     # Select 9600 BAUD
       $ sudo stty -F /dev/ttyS0 crtscts  # Enables CTS/RTS handshaking
       $ sudo stty -F /dev/ttyS0          # Show the TTY configuration
@@ -144,12 +146,39 @@ Using NuttX Zmodem with a Linux Host
         the previously received data to the file and the serial driver's RX
         buffer is overrun by a few bytes while the write is in progress.
         As a result, when it reads the next buffer of data, a few bytes may
-        be missing (maybe 10). Either (1) we need a more courteous host
-        application, or (2) we need to greatly improve the target side
-        buffering capability!
+        be missing (maybe 10).  The symptom of this missing data is a CRC check
+        failure.
+
+        Either (1) we need a more courteous host application, or (2) we
+        need to greatly improve the target side buffering capability!
+        Either (1) we need a more courteous host application, or (2) we
+        need to greatly improve the target side buffering capability!
 
         My thought now is to implement the NuttX sz and rz commands as
         PC side applications as well.  Matching both sides and obeying
         the handshaking will solve the issues.  Another option might be
         to fix the serial driver hardware flow control somehow.
 
+      sz has several command line options which one would think would
+      alleviate these problems.  But as of yet, I have not found a
+      combination of options that does so:
+
+      -L N, --packetlen N
+          Use ZMODEM sub-packets of length N. A larger N (32 <= N <= 1024)
+          gives slightly higher throughput, a smaller N speeds error
+          recovery. The default is 128 below 300 baud, 256 above 300 baud,
+          or 1024 above 2400 baud.
+
+      -l N, --framelen N
+          Wait for the receiver to acknowledge correct data every N
+          (32 <= N <= 1024) characters. This may be used to avoid network
+          overrun when XOFF flow control is lacking.
+
+      -w N, --windowsize N
+          Limit the transmit window size to N bytes (ZMODEM).
+
+      UPDATE:  I have verified that with debug off and at lower serial
+      BAUD (1200), the transfers of large fails succeed without errors.
+      You may need the Linux sz -O option to keep it from timing out
+      in these low BAUD transfers.
+
diff --git a/apps/system/zmodem/rz_main.c b/apps/system/zmodem/rz_main.c
index 886e17463..a7d475e2f 100644
--- a/apps/system/zmodem/rz_main.c
+++ b/apps/system/zmodem/rz_main.c
@@ -137,7 +137,7 @@ int rz_main(int argc, FAR char **argv)
   ret = zmr_receive(handle);
   if (ret < 0)
     {
-      fprintf(stderr, "File reception failed: %d\n", ret);
+      fprintf(stderr, "ERROR: File reception failed: %d\n", ret);
       goto errout_with_zmodem;
     }
 
diff --git a/apps/system/zmodem/zm.h b/apps/system/zmodem/zm.h
index 1f8201469..d025e94ef 100644
--- a/apps/system/zmodem/zm.h
+++ b/apps/system/zmodem/zm.h
@@ -183,7 +183,8 @@
 #define ZME_COMMAND   ZCOMMAND       /* Command, from sending program */
 #define ZME_STDERR    ZSTDERR        /* Output this message to stderr */
 
-#define ZME_OO        252            /* Received OO, termining the receiver */
+#define ZME_CANCEL    251            /* Received the cancelation sequence */
+#define ZME_OO        252            /* Received OO, terminating the receiver */
 #define ZME_DATARCVD  253            /* Data received */
 #define ZME_TIMEOUT   254            /* Timeout */
 #define ZME_ERROR     255            /* Protocol error */
diff --git a/apps/system/zmodem/zm_receive.c b/apps/system/zmodem/zm_receive.c
index e8c0968ac..e04a7ee11 100644
--- a/apps/system/zmodem/zm_receive.c
+++ b/apps/system/zmodem/zm_receive.c
@@ -59,6 +59,8 @@
 #include <assert.h>
 #include <errno.h>
 
+#include <apps/zmodem.h>
+
 #include "zm.h"
 
 /****************************************************************************
@@ -130,7 +132,7 @@ static int zmr_filedata(FAR struct zm_state_s *pzm);
 static int zmr_rcvto(FAR struct zm_state_s *pzm);
 static int zmr_fileto(FAR struct zm_state_s *pzm);
 static int zmr_cmddata(FAR struct zm_state_s *pzm);
-static int zmr_eof(FAR struct zm_state_s *pzm);
+static int zmr_zeof(FAR struct zm_state_s *pzm);
 static int zmr_zfin(FAR struct zm_state_s *pzm);
 static int zmr_finto(FAR struct zm_state_s *pzm);
 static int zmr_oo(FAR struct zm_state_s *pzm);
@@ -215,7 +217,7 @@ static const struct zm_transition_s g_zmr_readready[] =
 {
   {ZME_DATA,     false, ZMR_READING,     zmr_zdata},
   {ZME_NAK,      false, ZMR_READREADY,   zmr_badrpos},
-  {ZME_EOF,      false, ZMR_START,       zmr_eof},
+  {ZME_EOF,      false, ZMR_START,       zmr_zeof},
   {ZME_RQINIT,   true,  ZMR_START,       zmr_zrinit},
   {ZME_FILE,     false, ZMR_READREADY,   zmr_badrpos},
   {ZME_FIN,      true,  ZMR_FINISH,      zmr_zfin},
@@ -232,7 +234,7 @@ static const struct zm_transition_s g_zmr_reading[] =
   {ZME_NAK,      true,  ZMR_READREADY,   zmr_badrpos},
   {ZME_FIN,      true,  ZMR_FINISH,      zmr_zfin},
   {ZME_DATA,     false, ZMR_READING,     zmr_zdata},
-  {ZME_EOF,      true,  ZMR_START,       zmr_eof},
+  {ZME_EOF,      true,  ZMR_START,       zmr_zeof},
   {ZME_DATARCVD, false, ZMR_READING,     zmr_filedata},
   {ZME_TIMEOUT,  false, ZMR_READING,     zmr_fileto},
   {ZME_ERROR,    false, ZMR_READING,     zmr_error}
@@ -717,7 +719,8 @@ static int zmr_filedata(FAR struct zm_state_s *pzm)
 
   if ((pzm->flags & ZM_FLAG_CRKOK) == 0)
     {
-      zmdbg("ERROR: Bad crc, send ZRPOS(%ld)\n", (unsigned long)pzmr->offset);
+      zmdbg("ERROR: Bad crc, send ZRPOS(%ld)\n",
+            (unsigned long)pzmr->offset);
 
       /* No.. increment the count of errors */
 
@@ -759,7 +762,8 @@ static int zmr_filedata(FAR struct zm_state_s *pzm)
 
   /* Write the packet of data to the file */
 
-  ret = zm_writefile(pzmr->outfd, pzm->pktbuf, pzm->pktlen, pzmr->f0 == ZCNL);
+  ret = zm_writefile(pzmr->outfd, pzm->pktbuf, pzm->pktlen,
+                     pzmr->f0 == ZCNL);
   if (ret < 0)
     {
       int errorcode = errno;
@@ -894,14 +898,14 @@ static int zmr_fileto(FAR struct zm_state_s *pzm)
 }
 
 /****************************************************************************
- * Name: zmr_eof
+ * Name: zmr_zeof
  *
  * Description:
  *   Received ZEOF packet. File is now complete
  *
  ****************************************************************************/
 
-static int zmr_eof(FAR struct zm_state_s *pzm)
+static int zmr_zeof(FAR struct zm_state_s *pzm)
 {
   FAR struct zmr_state_s *pzmr = (FAR struct zmr_state_s *)pzm;
 
@@ -1652,7 +1656,13 @@ int zmr_receive(ZMRHANDLE handle)
 {
   FAR struct zmr_state_s *pzmr = (FAR struct zmr_state_s*)handle;
 
-  /* The state machine data pump will do the entire job */
+  /* The first thing that should happen is to receive ZRQINIT from the
+   * remote sender.  This could take while so use a long timeout.
+   */
+
+  pzmr->cmn.timeout = CONFIG_SYSTEM_ZMODEM_CONNTIME;
+
+  /* Then the state machine data pump will do the rest of the job */
 
   return zm_datapump(&pzmr->cmn);
 }
diff --git a/apps/system/zmodem/zm_send.c b/apps/system/zmodem/zm_send.c
index 8ba31abc1..f93a90304 100644
--- a/apps/system/zmodem/zm_send.c
+++ b/apps/system/zmodem/zm_send.c
@@ -63,6 +63,7 @@
 #include <crc32.h>
 
 #include <nuttx/ascii.h>
+#include <apps/zmodem.h>
 
 #include "zm.h"
 
diff --git a/apps/system/zmodem/zm_state.c b/apps/system/zmodem/zm_state.c
index fb12569ea..3aaa461b7 100644
--- a/apps/system/zmodem/zm_state.c
+++ b/apps/system/zmodem/zm_state.c
@@ -784,10 +784,10 @@ static int zm_parse(FAR struct zm_state_s *pzm, size_t rcvlen)
         {
           if (++pzm->ncan >= 5)
             {
-              zmdbg("Remote end has cancelled");
+              zmdbg("Remote end has canceled\n");
               pzm->rcvlen = 0;
               pzm->rcvndx = 0;
-              return -EAGAIN;
+              return zm_event(pzm, ZME_CANCEL);
             }
         }
       else
diff --git a/apps/system/zmodem/zm_utils.c b/apps/system/zmodem/zm_utils.c
index 7717f6cbe..17f21844b 100644
--- a/apps/system/zmodem/zm_utils.c
+++ b/apps/system/zmodem/zm_utils.c
@@ -246,23 +246,23 @@ ssize_t zm_write(int fd, FAR const uint8_t *buffer, size_t buflen)
 {
   ssize_t nwritten;
   size_t wrsize;
-  size_t total = 0;
+  size_t remaining;
 
   /* Read reading as necessary until the requested buffer is filled or until
    * an end of file indication or irrecoverable error is encountered.
    */
 
-  while (total < buflen)
+  for (remaining = buflen; remaining > 0; )
     {
 #if CONFIG_SYSTEM_ZMODEM_WRITESIZE > 0
-       if (buflen > CONFIG_SYSTEM_ZMODEM_WRITESIZE)
+       if (remaining > CONFIG_SYSTEM_ZMODEM_WRITESIZE)
          {
            wrsize = CONFIG_SYSTEM_ZMODEM_WRITESIZE;
          }
        else
 #endif
          {
-           wrsize = buflen;
+           wrsize = remaining;
          }
 
       /* Get the next gulp of data from the file */
@@ -287,13 +287,12 @@ ssize_t zm_write(int fd, FAR const uint8_t *buffer, size_t buflen)
         {
           /* Updates counts and pointers for the next read */
 
-          total  += nwritten;
-          buffer += nwritten;
-          buflen -= nwritten;
+          buffer    += nwritten;
+          remaining -= nwritten;
         }
     }
 
-  return (int)total;
+  return (int)(buflen - remaining);
 }
 
 /****************************************************************************
@@ -383,11 +382,24 @@ int zm_writefile(int fd, FAR const uint8_t *buffer, size_t buflen, bool zcnl)
                   nbytes  = 0;
                 }
 
-              if (ret == OK && !newline)
+              if (ret == OK)
                 {
-                  ret     = zm_write(fd, (FAR uint8_t *)"\n", 1);
-                  newline = true;
-                }
+                  /* Skip one char of \r\n? */
+
+                  if (newline)
+                    {
+                      /* Yes.. But don't skip if there is another */
+
+                      newline = false;
+                    }
+                  else
+                    {
+                      /* Write one newline and skip the follow \r or \n */
+
+                      ret     = zm_write(fd, (FAR uint8_t *)"\n", 1);
+                      newline = true;
+                    }
+               }
             }
           else
             {
@@ -401,7 +413,7 @@ int zm_writefile(int fd, FAR const uint8_t *buffer, size_t buflen, bool zcnl)
             }
         }
 
-      /* Write any trainling data that does not end with a newline */
+      /* Write any trailing data that does not end with a newline */
 
       if (ret == OK && nbytes > 0)
         {