JianyuWang0623 commented on code in PR #3101:
URL: https://github.com/apache/nuttx-apps/pull/3101#discussion_r2158237352


##########
system/fastboot/fastboot.c:
##########
@@ -991,60 +999,75 @@ static void fastboot_oem(FAR struct fastboot_ctx_s 
*context,
     }
 }
 
-static void fastboot_command_loop(FAR struct fastboot_ctx_s *context)
+static void fastboot_command_loop(FAR struct fastboot_ctx_s *first,
+                                  const size_t nctx)
 {
-  struct epoll_event ev[1];
+  FAR struct fastboot_ctx_s *context;
+  struct epoll_event ev[nctx];
   int epfd;
+  int n;
 
-  if (context->left > 0)
+  epfd = epoll_create(1);
+  for (context = first, n = nctx; n-- > 0; context++)
     {
-      epfd = epoll_create(1);
-      ev[0].events = EPOLLIN;
-      ev[0].data.ptr = context;
-
-      if (epoll_ctl(epfd, EPOLL_CTL_ADD, context->tran_fd[0], &ev[0]) < 0)
+      ev[n].events = EPOLLIN;
+      ev[n].data.ptr = context;
+      if (epoll_ctl(epfd, EPOLL_CTL_ADD, context->tran_fd[0], &ev[n]) < 0)
         {
           fb_err("add poll %d", context->tran_fd[0]);
           return;
         }
+    }
 
-      if (epoll_wait(epfd, ev, nitems(ev), context->left) <= 0)
+  if (first->left > 0)
+    {
+      if (epoll_wait(epfd, ev, nitems(ev), first->left) <= 0)
         {
           return;
         }
     }
 
   /* Reinitialize for storing TCP transport remaining data size. */
 
-  context->left = 0;
+  for (context = first, n = nctx; n-- > 0; context++)
+    {
+      context->left = 0;
+    }
 
   while (1)
     {
       char buffer[FASTBOOT_MSG_LEN + 1];
       size_t ncmds = nitems(g_fast_cmd);
       size_t index;
+      int ready;
 
-      ssize_t r = context->ops->read(context, buffer, FASTBOOT_MSG_LEN);
-      if (r < 0)
+      ready = epoll_wait(epfd, ev, nitems(ev), -1);
+      for (n = 0; n < ready; )

Review Comment:
   Everything is ok for usbdev, but for some case of TCP transport: error 
occured if the "fastboot command" includes multiple "fastboot protocol 
command", for example: The command "fastboot flash" includes "getvar:has-slot", 
"getvar:max-downloadsize", and so on.
   
   The reason is that currently the patch only add socket fd `tran_fd[0]` to 
epoll, the connect fd `tran_fd[1]` was not, so after a single fastboot command 
the loop will run to `epoll_wait()` and the commands later will not be read.
   
   There are two ways:
   1. add the connect fd `tran_fd[1]`(usbdev_out for USB) to epoll.
   2. read until EOF(for TCP disconnect) or error EAGAIN for USB.
   
   Currently select the 2nd way, because, for example, this can avoid flashing 
partition from diferent transport at the same time.
   
   `for (n--; n >= 0; ) ` and the var `n` was decreated in line 1043:
   
https://github.com/apache/nuttx-apps/blob/23f776cb649181cab9e0d9cdca3904e02f8cc1f8/system/fastboot/fastboot.c#L1036-L1045



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to