Merge pull request avrdudes#1724 from stefanrueger/progress-reporting

Document progress reporting in source code

src/avr.c

@@ -1625,24 +1625,88 @@ int avr_unlock(const PROGRAMMER *pgm, const AVRPART *p) {
 /*
  * Report the progress of a read or write operation from/to the device
  *
- * The first call of report_progress() should look like this (for a write):
+ * Potentially time-consuming libavrdude functions such as avr_read() and
+ * avr_write() use this interface to inform the user of their progress.
  *
- * report_progress(0, 1, "Writing");
+ * The first call of report_progress() normally looks like this, eg, for a
+ * write-to-device operation:
  *
- * Then hdr should be passed NULL on subsequent calls *
- * report_progress(k, n, NULL); // k/n signifies proportion of work done
+ *   report_progress(0, 1, "Writing");
  *
- * with 0 <= k < n, while the operation is progressing. Once the operation is
- * complete, a final call should be made as such to ensure proper termination
- * of the progress report; choose one of the following three forms:
+ * Then hdr should be passed NULL on subsequent calls
  *
- * report_progress(n, n, NULL); // finished OK, terminate with double \n
- * report_progress(1, 0, NULL); // finished OK, do not print terminating \n
- * report_progress(1, -1, NULL); // finished not OK, print double \n
+ *   report_progress(k, n, NULL);  // k/n signifies proportion of work done
+ *
+ * with 0 <= k < n, while the operation is progressing. Once the operation
+ * is complete, a final call must be made to ensure proper termination of
+ * the progress report; choose one of the following three forms:
+ *
+ *   report_progress(n, n, NULL);  // Finished OK: terminate display
+ *   report_progress(1, 0, NULL);  // Finished OK: do not terminate display
+ *   report_progress(1, -1, NULL); // Finished on error: task not completed
  *
  * It is OK to call report_progress(1, -1, NULL) in a subroutine when
- * encountering a fatal error to terminate the reporting here and there even
- * though no report may have been started.
+ * encountering a fatal error to terminate the reporting here and there
+ * even though no report may have been started. It is also OK to skip the
+ * first call report_progress(0, 1, "<title>") in which case the following
+ * report_progress() calls should not generate any report.
+ *
+ * In fact, avr_read() and avr_write(), or their core parts avr_read_mem()
+ * and avr_write_mem() for that matter, internally only issue ongoing
+ *
+ *   report_progress(k, n, NULL);
+ *
+ * reporting and leave it to the caller whether or not reports should be
+ * generated at all: the caller's responsibility is to initiate the
+ * reporting or not by calling report_progress(0, 1, "Reading/Writing") or
+ * not. An example of good practice is the following sequence:
+ *
+ *   if(mem->size > 32 || verbose > 1)       // Reporting required?
+ *     report_progress(0, 1, "Reading");
+ *   rc = avr_read(pgm, part, mem->desc, 0); // Errors terminate reporting
+ *   report_progress(1, 1, NULL);            // Ensure reporting finishes
+ *
+ *
+ * report_progress() relies on an application specific function
+ *
+ *   void app_updprg(int percent, double etime, const char *hdr, int finish);
+ *
+ * being pointed at by the global function pointer update_progress. This
+ * function controls how the application informs the user how much
+ * progress the particular operation has made. This could be, eg, showing
+ * a video of an increasing number of dancing hamsters, playing the audio
+ * of a drum roll, showing a countdown clock or a progress bar. It is the
+ * application's responsibility to provide that function and to assign it
+ *
+ *   update_progress = app_updprg;           // Install progress updating
+ *
+ * before the application's first call of progress_report(). The update
+ * function has to keep track whether reporting was initiated or has been
+ * prematurely cut short, eg, by an error. It received an int percentage
+ * in [0, 100] of how much progress has been made, a double etime of how
+ * much time in seconds has passed since the activity started, a string
+ * hdr that describes the activity, eg, "Reading" (device memory) and an
+ * integer finish that tells the routine how the task has finished.
+ *
+ * Reporting should only start upon the first non-NULL hdr string was
+ * passed. Reporting should end immediately after percent reaches 100.
+ * Calls to update_progress() once reporting has ended should not show
+ * progress until the next time a non-NULL hdr was passed. The last
+ * argument finish can have three values:
+ *   -1  A severe error occurred and reporting ends; as the current
+ *       percent value will be 100 the most recently passed percent of a
+ *       previous call, if any, indicates how far the task has come before
+ *       the error occurred.
+ *    0  If percent is 100 reporting ends and the caller does not wish the
+ *       display to be terminated; for an ASCII progress bar this means
+ *       that no terminating \n is printed
+ *    1  If percent is 100 reporting ends and the caller wishes the
+ *       display to be terminated; for an ASCII progress bar this means
+ *       that two terminating \n are printed
+ *
+ * As an example see how term.c's void update_progress_tty() function
+ * shows an ASCII progress bar.
+ *
  */
 
 void report_progress(int completed, int total, const char *hdr) {

src/term.c

@@ -2681,26 +2681,44 @@ static int cmd_include(const PROGRAMMER *pgm, const AVRPART *p, int argc, const
 }
 
 
+/*
+ * ASCII progress bar
+ *
+ * A 50 character bar is gradually filled with hash marks (#) to indicate
+ * progress, and completed with hyphen (-) once an error occurred:
+ *
+ * Reading | ##############################                     | 59% 0.41 s
+ *
+ * Reading | ###############################------------------- | 61% 0.42 s
+ *
+ * First non-NULL heading hdr starts reporting, percent=100 stops reporting;
+ * etime is the wall-clock time in seconds that the task has taken so for;
+ * finish can take on three values:
+ *   -1 task ended in error, show the last valid percentage and fill
+ *      progress bar with hyphens instead of hashes
+ *    0 do not terminate progress bar with \n when finishing at 100 percent
+ *    1 terminate progress bar with \n when finishing at 100 percent
+ */
 static void update_progress_tty(int percent, double etime, const char *hdr, int finish) {
   static char *header;
   static int last, done = 1;
   int i;
 
-  setvbuf(stderr, (char *) NULL, _IONBF, 0);
+  setvbuf(stderr, (char *) NULL, _IONBF, 0);  // Set stderr to be ubuffered
 
   if(hdr) {
-    lmsg_info("");
-    last = done = 0;
+    lmsg_info("");              // Print new line unless already done before
+    last = done = 0;            // OK, we have a header, start reporting
     if(header)
       free(header);
-    header = cfg_strdup("update_progress_tty()",  hdr);
+    header = cfg_strdup(__func__, hdr);
   }
 
   percent = percent > 100? 100: percent < 0? 0: percent;
 
   if(!done) {
     if(!header)
-      header = cfg_strdup("update_progress_tty()", "report");
+      header = cfg_strdup(__func__, "report");
 
     int showperc = finish >= 0? percent: last;
 
@@ -2710,16 +2728,17 @@ static void update_progress_tty(int percent, double etime, const char *hdr, int
       hashes[i/2] = '#';
     hashes[50] = 0;
 
+    // Overwrite line using \r
     msg_info("\r%s | %s | %d%% %0.2f s ", header, hashes, showperc, etime);
     if(percent == 100) {
       if(finish)
         lmsg_info("");
-      done = 1;
+      done = 1;                 // Stop future reporting
     }
   }
   last = percent;
 
-  setvbuf(stderr, (char *) NULL, _IOLBF, 0);
+  setvbuf(stderr, (char *) NULL, _IOLBF, 0); // Set stderr to be line buffered
 }
 
 static void update_progress_no_tty(int percent, double etime, const char *hdr, int finish) {