Fix various printing issues with the progressbar

Specifically, fix the progressbar to:

 * Print at 100% after an 'unknown' percentage task has completed
 * Refresh the progressbar if being called without a main loop running
 * Allow the progressbar to start with a h-offset without moving 'left'
 * Don't cause high CPU load when calling fu_progressbar_update() ever few us

Also, add some unit tests to discover all the issues.
This commit is contained in:
Richard Hughes 2017-09-16 21:22:18 +01:00
parent 4f98fe89ba
commit b5b4beb472
3 changed files with 89 additions and 23 deletions

View File

@ -38,6 +38,7 @@ struct _FuProgressbar
guint percentage; guint percentage;
guint to_erase; /* chars */ guint to_erase; /* chars */
guint timer_id; guint timer_id;
gint64 last_animated; /* monotonic */
}; };
G_DEFINE_TYPE (FuProgressbar, fu_progressbar, G_TYPE_OBJECT) G_DEFINE_TYPE (FuProgressbar, fu_progressbar, G_TYPE_OBJECT)
@ -87,10 +88,11 @@ fu_progressbar_status_to_string (FwupdStatus status)
} }
static void static void
fu_progressbar_refresh (FuProgressbar *self) fu_progressbar_refresh (FuProgressbar *self, FwupdStatus status, guint percentage)
{ {
const gchar *title; const gchar *title;
guint i; guint i;
gboolean is_idle_newline = FALSE;
g_autoptr(GString) str = g_string_new (NULL); g_autoptr(GString) str = g_string_new (NULL);
/* erase previous line */ /* erase previous line */
@ -98,21 +100,20 @@ fu_progressbar_refresh (FuProgressbar *self)
g_print ("\b"); g_print ("\b");
/* add status */ /* add status */
if (self->status == FWUPD_STATUS_IDLE) { if (status == FWUPD_STATUS_IDLE) {
if (self->to_erase > 0) percentage = 100;
g_print ("\n"); status = self->status;
self->to_erase = 0; is_idle_newline = TRUE;
return;
} }
title = fu_progressbar_status_to_string (self->status); title = fu_progressbar_status_to_string (status);
g_string_append (str, title); g_string_append (str, title);
for (i = str->len; i < self->length_status; i++) for (i = str->len; i < self->length_status; i++)
g_string_append_c (str, ' '); g_string_append_c (str, ' ');
/* add progressbar */ /* add progressbar */
g_string_append (str, "["); g_string_append (str, "[");
if (self->percentage > 0) { if (percentage > 0) {
for (i = 0; i < self->length_percentage * self->percentage / 100; i++) for (i = 0; i < (self->length_percentage - 1) * percentage / 100; i++)
g_string_append_c (str, '*'); g_string_append_c (str, '*');
for (i = i + 1; i < self->length_percentage; i++) for (i = i + 1; i < self->length_percentage; i++)
g_string_append_c (str, ' '); g_string_append_c (str, ' ');
@ -121,14 +122,37 @@ fu_progressbar_refresh (FuProgressbar *self)
for (i = 0; i < self->spinner_idx; i++) for (i = 0; i < self->spinner_idx; i++)
g_string_append_c (str, ' '); g_string_append_c (str, ' ');
g_string_append_c (str, chars[i / 4 % G_N_ELEMENTS(chars)]); g_string_append_c (str, chars[i / 4 % G_N_ELEMENTS(chars)]);
for (i = i + 1; i < self->length_percentage; i++) for (i = i + 1; i < self->length_percentage - 1; i++)
g_string_append_c (str, ' '); g_string_append_c (str, ' ');
} }
g_string_append_c (str, ']'); g_string_append_c (str, ']');
/* dump to screen */ /* dump to screen */
g_print ("%s", str->str); g_print ("%s", str->str);
self->to_erase = str->len; self->to_erase = str->len - 2;
/* done */
if (is_idle_newline) {
g_print ("\n");
self->to_erase = 0;
return;
}
}
static void
fu_progressbar_spin_inc (FuProgressbar *self)
{
/* reset */
self->last_animated = g_get_monotonic_time ();
/* up to down */
if (self->spinner_count_up) {
if (++self->spinner_idx > self->length_percentage - 3)
self->spinner_count_up = FALSE;
} else {
if (--self->spinner_idx == 0)
self->spinner_count_up = TRUE;
}
} }
static gboolean static gboolean
@ -137,16 +161,10 @@ fu_progressbar_spin_cb (gpointer user_data)
FuProgressbar *self = FU_PROGRESSBAR (user_data); FuProgressbar *self = FU_PROGRESSBAR (user_data);
/* move the spinner index up to down */ /* move the spinner index up to down */
if (self->spinner_count_up) { fu_progressbar_spin_inc (self);
if (++self->spinner_idx > self->length_percentage - 2)
self->spinner_count_up = FALSE;
} else {
if (--self->spinner_idx == 0)
self->spinner_count_up = TRUE;
}
/* update the terminal */ /* update the terminal */
fu_progressbar_refresh (self); fu_progressbar_refresh (self, self->status, self->percentage);
return G_SOURCE_CONTINUE; return G_SOURCE_CONTINUE;
} }
@ -177,9 +195,21 @@ fu_progressbar_update (FuProgressbar *self, FwupdStatus status, guint percentage
{ {
g_return_if_fail (FU_IS_PROGRESSBAR (self)); g_return_if_fail (FU_IS_PROGRESSBAR (self));
/* cache */ /* if the main loop isn't spinning and we've not had a chance to
self->status = status; * execute the callback just do the refresh now manually */
self->percentage = percentage; if (percentage == 0 &&
status != FWUPD_STATUS_IDLE &&
self->status != FWUPD_STATUS_UNKNOWN) {
if ((g_get_monotonic_time () - self->last_animated) / 1000 > 40) {
fu_progressbar_spin_inc (self);
fu_progressbar_refresh (self, status, percentage);
}
}
/* ignore duplicates */
if (self->status == status &&
self->percentage == percentage)
return;
/* enable or disable the spinner timeout */ /* enable or disable the spinner timeout */
if (percentage > 0) { if (percentage > 0) {
@ -189,7 +219,11 @@ fu_progressbar_update (FuProgressbar *self, FwupdStatus status, guint percentage
} }
/* update the terminal */ /* update the terminal */
fu_progressbar_refresh (self); fu_progressbar_refresh (self, status, percentage);
/* cache */
self->status = status;
self->percentage = percentage;
} }
void void

View File

@ -34,6 +34,7 @@
#include "fu-keyring.h" #include "fu-keyring.h"
#include "fu-pending.h" #include "fu-pending.h"
#include "fu-plugin-private.h" #include "fu-plugin-private.h"
#include "fu-progressbar.h"
#include "fu-hwids.h" #include "fu-hwids.h"
#include "fu-smbios.h" #include "fu-smbios.h"
#include "fu-test.h" #include "fu-test.h"
@ -735,6 +736,34 @@ fu_common_spawn_func (void)
g_assert_cmpint (lines, ==, 6); g_assert_cmpint (lines, ==, 6);
} }
static void
fu_progressbar_func (void)
{
g_autoptr(FuProgressbar) progressbar = fu_progressbar_new ();
fu_progressbar_set_length_status (progressbar, 20);
fu_progressbar_set_length_percentage (progressbar, 50);
g_print ("\n");
for (guint i = 0; i < 100; i++) {
fu_progressbar_update (progressbar, FWUPD_STATUS_DECOMPRESSING, i);
g_usleep (10000);
}
fu_progressbar_update (progressbar, FWUPD_STATUS_IDLE, 0);
for (guint i = 0; i < 100; i++) {
guint pc = (i > 25 && i < 75) ? 0 : i;
fu_progressbar_update (progressbar, FWUPD_STATUS_LOADING, pc);
g_usleep (10000);
}
fu_progressbar_update (progressbar, FWUPD_STATUS_IDLE, 0);
for (guint i = 0; i < 5000; i++) {
fu_progressbar_update (progressbar, FWUPD_STATUS_LOADING, 0);
g_usleep (1000);
}
fu_progressbar_update (progressbar, FWUPD_STATUS_IDLE, 0);
}
int int
main (int argc, char **argv) main (int argc, char **argv)
{ {
@ -747,6 +776,8 @@ main (int argc, char **argv)
g_assert_cmpint (g_mkdir_with_parents ("/tmp/fwupd-self-test/var/lib/fwupd", 0755), ==, 0); g_assert_cmpint (g_mkdir_with_parents ("/tmp/fwupd-self-test/var/lib/fwupd", 0755), ==, 0);
/* tests go here */ /* tests go here */
if (g_test_slow ())
g_test_add_func ("/fwupd/progressbar", fu_progressbar_func);
g_test_add_func ("/fwupd/device-locker{success}", fu_device_locker_func); g_test_add_func ("/fwupd/device-locker{success}", fu_device_locker_func);
g_test_add_func ("/fwupd/device-locker{fail}", fu_device_locker_fail_func); g_test_add_func ("/fwupd/device-locker{fail}", fu_device_locker_fail_func);
g_test_add_func ("/fwupd/device{metadata}", fu_device_metadata_func); g_test_add_func ("/fwupd/device{metadata}", fu_device_metadata_func);

View File

@ -166,6 +166,7 @@ if get_option('enable-tests')
'fu-keyring.c', 'fu-keyring.c',
'fu-keyring-result.c', 'fu-keyring-result.c',
'fu-plugin.c', 'fu-plugin.c',
'fu-progressbar.c',
'fu-smbios.c', 'fu-smbios.c',
'fu-test.c', 'fu-test.c',
], ],