From 70d13a5a58c4660e27b8c0443162edaf4b5779c7 Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Mon, 24 Jul 2017 11:58:59 +0100 Subject: [PATCH] unifying: Make sure the percentage completion goes from 0% to 100% Pre-filter the pending LuDeviceBootloaderRequest's after they are parsed so that we know exactly how many packets need to be sent to the hardware. As shown in https://github.com/hughsie/fwupd/issues/155, people get worried if the progress bar finishes before 100% as they'll wonder if all the writing completed successfully. --- .../unifying/lu-device-bootloader-nordic.c | 8 ++--- plugins/unifying/lu-device-bootloader-texas.c | 14 +------- plugins/unifying/lu-device-bootloader.c | 35 +++++++++++++++++-- plugins/unifying/lu-device-bootloader.h | 3 +- plugins/unifying/lu-tool.c | 8 ++++- 5 files changed, 44 insertions(+), 24 deletions(-) diff --git a/plugins/unifying/lu-device-bootloader-nordic.c b/plugins/unifying/lu-device-bootloader-nordic.c index 9a3263665..88eb87279 100644 --- a/plugins/unifying/lu-device-bootloader-nordic.c +++ b/plugins/unifying/lu-device-bootloader-nordic.c @@ -236,16 +236,12 @@ lu_device_bootloader_nordic_write_firmware (LuDevice *device, } /* transfer payload */ - reqs = lu_device_bootloader_parse_requests (fw, error); + reqs = lu_device_bootloader_parse_requests (device, fw, error); if (reqs == NULL) return FALSE; + for (guint i = 1; i < reqs->len; i++) { payload = g_ptr_array_index (reqs, i); - - /* skip the bootloader */ - if (payload->addr > lu_device_bootloader_get_addr_hi (device)) - break; - if (!lu_device_bootloader_nordic_write (device, payload->addr, payload->len, diff --git a/plugins/unifying/lu-device-bootloader-texas.c b/plugins/unifying/lu-device-bootloader-texas.c index 07e9b6a3f..904c380cc 100644 --- a/plugins/unifying/lu-device-bootloader-texas.c +++ b/plugins/unifying/lu-device-bootloader-texas.c @@ -134,7 +134,7 @@ lu_device_bootloader_texas_write_firmware (LuDevice *device, g_autoptr(LuDeviceBootloaderRequest) req = lu_device_bootloader_request_new (); /* transfer payload */ - reqs = lu_device_bootloader_parse_requests (fw, error); + reqs = lu_device_bootloader_parse_requests (device, fw, error); if (reqs == NULL) return FALSE; @@ -150,18 +150,6 @@ lu_device_bootloader_texas_write_firmware (LuDevice *device, for (guint i = 0; i < reqs->len; i++) { payload = g_ptr_array_index (reqs, i); - /* skip the bootloader */ - if (payload->addr > lu_device_bootloader_get_addr_hi (device)) { - g_debug ("skipping write @ %04x", payload->addr); - continue; - } - - /* skip the header */ - if (payload->addr < lu_device_bootloader_get_addr_lo (device)) { - g_debug ("skipping write @ %04x", payload->addr); - continue; - } - /* check size */ if (payload->len != 16) { g_set_error (error, diff --git a/plugins/unifying/lu-device-bootloader.c b/plugins/unifying/lu-device-bootloader.c index aa8309bcd..8e22e1943 100644 --- a/plugins/unifying/lu-device-bootloader.c +++ b/plugins/unifying/lu-device-bootloader.c @@ -46,17 +46,18 @@ lu_device_bootloader_request_new (void) } GPtrArray * -lu_device_bootloader_parse_requests (GBytes *fw, GError **error) +lu_device_bootloader_parse_requests (LuDevice *device, GBytes *fw, GError **error) { const gchar *tmp; g_auto(GStrv) lines = NULL; g_autoptr(GPtrArray) reqs = NULL; + guint32 last_addr = 0; reqs = g_ptr_array_new_with_free_func (g_free); tmp = g_bytes_get_data (fw, NULL); lines = g_strsplit_set (tmp, "\n\r", -1); for (guint i = 0; lines[i] != NULL; i++) { - LuDeviceBootloaderRequest *payload; + g_autoptr(LuDeviceBootloaderRequest) payload = NULL; /* skip empty lines */ tmp = lines[i]; @@ -89,7 +90,35 @@ lu_device_bootloader_parse_requests (GBytes *fw, GError **error) } payload->data[j] = lu_buffer_read_uint8 (ptr); } - g_ptr_array_add (reqs, payload); + + /* skip the bootloader */ + if (payload->addr > lu_device_bootloader_get_addr_hi (device)) { + g_debug ("skipping write @ %04x", payload->addr); + continue; + } + + /* skip the header */ + if (payload->addr < lu_device_bootloader_get_addr_lo (device)) { + g_debug ("skipping write @ %04x", payload->addr); + continue; + } + + /* make sure firmware addresses only go up */ + if (payload->addr < last_addr) { + g_debug ("skipping write @ %04x", payload->addr); + continue; + } + last_addr = payload->addr; + + /* pending */ + g_ptr_array_add (reqs, g_steal_pointer (&payload)); + } + if (reqs->len == 0) { + g_set_error_literal (error, + G_IO_ERROR, + G_IO_ERROR_INVALID_DATA, + "firmware data invalid: no payloads found"); + return NULL; } return g_steal_pointer (&reqs); } diff --git a/plugins/unifying/lu-device-bootloader.h b/plugins/unifying/lu-device-bootloader.h index 6d86135a2..d2e448bc0 100644 --- a/plugins/unifying/lu-device-bootloader.h +++ b/plugins/unifying/lu-device-bootloader.h @@ -84,7 +84,8 @@ LuDeviceBootloaderRequest *lu_device_bootloader_request_new (void); G_DEFINE_AUTOPTR_CLEANUP_FUNC(LuDeviceBootloaderRequest, g_free); -GPtrArray *lu_device_bootloader_parse_requests (GBytes *fw, +GPtrArray *lu_device_bootloader_parse_requests (LuDevice *device, + GBytes *fw, GError **error); gboolean lu_device_bootloader_request (LuDevice *device, LuDeviceBootloaderRequest *req, diff --git a/plugins/unifying/lu-tool.c b/plugins/unifying/lu-tool.c index a399dfd38..9ccebc5c9 100644 --- a/plugins/unifying/lu-tool.c +++ b/plugins/unifying/lu-tool.c @@ -252,6 +252,7 @@ static gboolean lu_tool_dump (FuLuToolPrivate *priv, gchar **values, GError **error) { g_autoptr(GPtrArray) reqs = NULL; + g_autoptr(LuDevice) device = NULL; g_autoptr(GBytes) fw = NULL; g_autofree gchar *data = NULL; gsize len = 0; @@ -266,11 +267,16 @@ lu_tool_dump (FuLuToolPrivate *priv, gchar **values, GError **error) return FALSE; } + /* fake a huge device */ + device = g_object_new (LU_TYPE_DEVICE_BOOTLOADER, NULL); + lu_device_bootloader_set_addr_lo (device, 0x0000); + lu_device_bootloader_set_addr_hi (device, 0xffff); + /* load file and display */ if (!g_file_get_contents (values[0], &data, &len, error)) return FALSE; fw = g_bytes_new_static (data, len); - reqs = lu_device_bootloader_parse_requests (fw, error); + reqs = lu_device_bootloader_parse_requests (device, fw, error); if (reqs == NULL) return FALSE; for (guint i = 0; i < reqs->len; i++) {