On Fri, 27 Nov 2015 15:35:20 +0100
Bernhard Nortmann <[email protected]> wrote:

> Add "--progress" option and a progress bar display for FEL
> transfers. This picks up on a suggestion from Alexander Kaplan
> and the discussion at
> https://groups.google.com/forum/#!topic/linux-sunxi/lz0oQBwjex0
> 
> Signed-off-by: Bernhard Nortmann <[email protected]>
> ---
>  fel.c      | 16 ++++++++++++++--
>  progress.c | 49 ++++++++++++++++++++++++++++++++++++++++++-------
>  progress.h |  4 ++++
>  3 files changed, 60 insertions(+), 9 deletions(-)
> 
> diff --git a/fel.c b/fel.c
> index 89014d7..595b682 100644
> --- a/fel.c
> +++ b/fel.c
> @@ -81,7 +81,12 @@ static const int AW_USB_MAX_BULK_SEND = 4 * 1024 * 1024; 
> // 4 MiB per bulk reque
>  void usb_bulk_send(libusb_device_handle *usb, int ep, const void *data,
>                  size_t length, bool progress)
>  {
> -     size_t max_chunk = AW_USB_MAX_BULK_SEND; /* maximum chunk size */
> +     /*
> +      * With no progress notifications, we'll use the maximum chunk size.
> +      * Otherwise, it's useful to lower the size (have more chunks) to get
> +      * more frequent status updates. 128 KiB per request seem suitable.
> +      */
> +     size_t max_chunk = progress ? 128 * 1024 : AW_USB_MAX_BULK_SEND;
>  
>       size_t chunk;
>       int rc, sent;
> @@ -1215,6 +1220,7 @@ static int aw_fel_get_endpoint(libusb_device_handle 
> *usb)
>  
>  int main(int argc, char **argv)
>  {
> +     bool pflag_active = false; /* -p switch, causing "write" to output 
> progress */
>       int rc;
>       libusb_device_handle *handle = NULL;
>       int iface_detached = -1;
> @@ -1224,6 +1230,7 @@ int main(int argc, char **argv)
>       if (argc <= 1) {
>               printf("Usage: %s [options] command arguments... [command...]\n"
>                       "       -v, --verbose                   Verbose 
> logging\n"
> +                     "       -p, --progress                  \"write\" 
> transfers show a progress bar\n"
>                       "\n"
>                       "       spl file                        Load and 
> execute U-Boot SPL\n"
>                       "               If file additionally contains a main 
> U-Boot binary\n"
> @@ -1283,7 +1290,11 @@ int main(int argc, char **argv)
>  
>       while (argc > 1 ) {
>               int skip = 1;
> -             if (strncmp(argv[1], "hex", 3) == 0 && argc > 3) {
> +
> +             if (strcmp(argv[1], "--progress") == 0 ||
> +                 strcmp(argv[1], "-p") == 0) {
> +                     pflag_active = true;

With this way of handling command line switches, "sunxi-fel -p -v ..."
does not work right. But I guess, this can be fixed later.

> +             } else if (strncmp(argv[1], "hex", 3) == 0 && argc > 3) {
>                       aw_fel_hexdump(handle, strtoul(argv[2], NULL, 0), 
> strtoul(argv[3], NULL, 0));
>                       skip = 3;
>               } else if (strncmp(argv[1], "dump", 4) == 0 && argc > 3) {
> @@ -1300,6 +1311,7 @@ int main(int argc, char **argv)
>                       size_t size;
>                       void *buf = load_file(argv[3], &size);
>                       uint32_t offset = strtoul(argv[2], NULL, 0);
> +                     progress_start(pflag_active ? progress_bar : NULL, 
> size);
>                       double elapsed = aw_write_buffer(handle, buf, offset, 
> size, true);
>                       if (elapsed > 0)
>                               pr_info("%.1f kB written in %.1f sec (speed: 
> %.1f kB/s)\n",
> diff --git a/progress.c b/progress.c
> index 9c2a7d4..5b24198 100644
> --- a/progress.c
> +++ b/progress.c
> @@ -20,8 +20,6 @@
>  #include <sys/time.h>
>  #include <unistd.h>
>  
> -#include "common.h"
> -
>  /* Less reliable than clock_gettime, but does not require linking with -lrt 
> */
>  inline double gettime(void)
>  {
> @@ -30,11 +28,48 @@ inline double gettime(void)
>       return tv.tv_sec + (double)tv.tv_usec / 1000000.;
>  }
>  
> +/* Private progress state variable */
> +
> +typedef struct {
> +     progress_cb_t callback;
> +     size_t total;
> +     size_t done;
> +} progress_private_t;
> +
> +static progress_private_t progress = {
> +     .callback = NULL,
> +};
> +
> +/* 'External' API */
> +
> +void progress_start(progress_cb_t callback, size_t expected_total)
> +{
> +     progress.callback = callback;
> +     progress.total = expected_total;
> +     progress.done = 0;
> +}
> +
>  /* Update progress status, passing information to the callback function. */
> -void progress_update(size_t UNUSED(bytes_done))
> +void progress_update(size_t bytes_done)
> +{
> +     progress.done += bytes_done;
> +     if (progress.callback)
> +             progress.callback(progress.total, progress.done);
> +}
> +
> +/* Callback function implementing a simple progress bar written to stdout */
> +void progress_bar(size_t total, size_t done)
>  {

A minor nitpick. Do we need to have "size_t total" as an argument to
the progress bar callback function?

For comparison, when the transfer speed and ETA calculation is added
in the next patch, nothing like this is done for the transfer start
timestamp.

> -     /*
> -      * This is a non-functional placeholder!
> -      * It will be replaced in a later patch.
> -      */
> +     static const int WIDTH = 60; /* # of characters to use for progress bar 
> */
> +
> +     float ratio = total > 0 ? (float)done / total : 0;
> +     int i, pos = WIDTH * ratio;
> +
> +     printf("\r%3.0f%% [", ratio * 100); /* current percentage */
> +     for (i = 0; i < pos; i++) putchar('=');
> +     for (i = pos; i < WIDTH; i++) putchar(' ');
> +     printf("] ");
> +
> +     if (done >= total) putchar('\n'); /* output newline when complete */
> +     fflush(stdout);
>  }
> diff --git a/progress.h b/progress.h
> index 360988e..6c4dc7f 100644
> --- a/progress.h
> +++ b/progress.h
> @@ -29,6 +29,10 @@ typedef void (*progress_cb_t)(size_t total, size_t done);
>  
>  double gettime(void);
>  
> +void progress_start(progress_cb_t callback, size_t expected_total);
>  void progress_update(size_t bytes_done);
>  
> +/* progress callback implementations for various display styles */
> +void progress_bar(size_t total, size_t done);
> +
>  #endif /* _SUNXI_TOOLS_PROGRESS_H */

Reviewed-by: Siarhei Siamashka <[email protected]>

-- 
Best regards,
Siarhei Siamashka

-- 
You received this message because you are subscribed to the Google Groups 
"linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to