From d0c6c3402c608da22162e670d03b262d70f812be Mon Sep 17 00:00:00 2001 From: hedger Date: Mon, 13 Feb 2023 18:07:53 +0400 Subject: [PATCH] Updater visual fixes (#2391) * updater: removed unused code; fbt: fixed build error processing in certain cases * updater: simplified internal pre-update state * updater: rebalanced stage weights; fixed progress hiccups --- .../updater/scenes/updater_scene_error.c | 6 ++- .../updater/scenes/updater_scene_loadcfg.c | 19 ++++------ applications/system/updater/updater_i.h | 8 +--- .../system/updater/util/update_task.c | 22 +++++------ .../updater/util/update_task_worker_backup.c | 38 +++++++++++++------ lib/toolbox/tar/tar_archive.c | 1 + scripts/fbt_tools/pvsstudio.py | 17 +++++---- 7 files changed, 62 insertions(+), 49 deletions(-) diff --git a/applications/system/updater/scenes/updater_scene_error.c b/applications/system/updater/scenes/updater_scene_error.c index 21bf1637..dbe97c96 100644 --- a/applications/system/updater/scenes/updater_scene_error.c +++ b/applications/system/updater/scenes/updater_scene_error.c @@ -58,8 +58,12 @@ bool updater_scene_error_on_event(void* context, SceneManagerEvent event) { } void updater_scene_error_on_exit(void* context) { + furi_assert(context); Updater* updater = (Updater*)context; widget_reset(updater->widget); - free(updater->pending_update); + + if(updater->loaded_manifest) { + update_manifest_free(updater->loaded_manifest); + } } diff --git a/applications/system/updater/scenes/updater_scene_loadcfg.c b/applications/system/updater/scenes/updater_scene_loadcfg.c index 14f7b203..99866a6d 100644 --- a/applications/system/updater/scenes/updater_scene_loadcfg.c +++ b/applications/system/updater/scenes/updater_scene_loadcfg.c @@ -21,11 +21,9 @@ void updater_scene_loadcfg_apply_callback(GuiButtonType result, InputType type, void updater_scene_loadcfg_on_enter(void* context) { Updater* updater = (Updater*)context; - UpdaterManifestProcessingState* pending_upd = updater->pending_update = - malloc(sizeof(UpdaterManifestProcessingState)); - pending_upd->manifest = update_manifest_alloc(); + UpdateManifest* loaded_manifest = updater->loaded_manifest = update_manifest_alloc(); - if(update_manifest_init(pending_upd->manifest, furi_string_get_cstr(updater->startup_arg))) { + if(update_manifest_init(loaded_manifest, furi_string_get_cstr(updater->startup_arg))) { widget_add_string_element( updater->widget, 64, 12, AlignCenter, AlignCenter, FontPrimary, "Update"); @@ -37,7 +35,7 @@ void updater_scene_loadcfg_on_enter(void* context) { 32, AlignCenter, AlignCenter, - furi_string_get_cstr(pending_upd->manifest->version), + furi_string_get_cstr(loaded_manifest->version), true); widget_add_button_element( @@ -95,13 +93,12 @@ bool updater_scene_loadcfg_on_event(void* context, SceneManagerEvent event) { } void updater_scene_loadcfg_on_exit(void* context) { + furi_assert(context); Updater* updater = (Updater*)context; - if(updater->pending_update) { - update_manifest_free(updater->pending_update->manifest); - furi_string_free(updater->pending_update->message); - } - widget_reset(updater->widget); - free(updater->pending_update); + + if(updater->loaded_manifest) { + update_manifest_free(updater->loaded_manifest); + } } diff --git a/applications/system/updater/updater_i.h b/applications/system/updater/updater_i.h index ae249f38..4e3c704d 100644 --- a/applications/system/updater/updater_i.h +++ b/applications/system/updater/updater_i.h @@ -33,12 +33,6 @@ typedef enum { UpdaterCustomEventSdUnmounted, } UpdaterCustomEvent; -typedef struct UpdaterManifestProcessingState { - UpdateManifest* manifest; - FuriString* message; - bool ready_to_be_applied; -} UpdaterManifestProcessingState; - typedef struct { // GUI Gui* gui; @@ -49,7 +43,7 @@ typedef struct { UpdaterMainView* main_view; - UpdaterManifestProcessingState* pending_update; + UpdateManifest* loaded_manifest; UpdatePrepareResult preparation_result; UpdateTask* update_task; diff --git a/applications/system/updater/util/update_task.c b/applications/system/updater/util/update_task.c index 54fe2799..708d10ce 100644 --- a/applications/system/updater/util/update_task.c +++ b/applications/system/updater/util/update_task.c @@ -41,22 +41,22 @@ typedef struct { static const UpdateTaskStageGroupMap update_task_stage_progress[] = { [UpdateTaskStageProgress] = STAGE_DEF(UpdateTaskStageGroupMisc, 0), - [UpdateTaskStageReadManifest] = STAGE_DEF(UpdateTaskStageGroupPreUpdate, 5), - [UpdateTaskStageLfsBackup] = STAGE_DEF(UpdateTaskStageGroupPreUpdate, 15), + [UpdateTaskStageReadManifest] = STAGE_DEF(UpdateTaskStageGroupPreUpdate, 45), + [UpdateTaskStageLfsBackup] = STAGE_DEF(UpdateTaskStageGroupPreUpdate, 5), [UpdateTaskStageRadioImageValidate] = STAGE_DEF(UpdateTaskStageGroupRadio, 15), - [UpdateTaskStageRadioErase] = STAGE_DEF(UpdateTaskStageGroupRadio, 60), - [UpdateTaskStageRadioWrite] = STAGE_DEF(UpdateTaskStageGroupRadio, 80), - [UpdateTaskStageRadioInstall] = STAGE_DEF(UpdateTaskStageGroupRadio, 60), - [UpdateTaskStageRadioBusy] = STAGE_DEF(UpdateTaskStageGroupRadio, 80), + [UpdateTaskStageRadioErase] = STAGE_DEF(UpdateTaskStageGroupRadio, 35), + [UpdateTaskStageRadioWrite] = STAGE_DEF(UpdateTaskStageGroupRadio, 60), + [UpdateTaskStageRadioInstall] = STAGE_DEF(UpdateTaskStageGroupRadio, 30), + [UpdateTaskStageRadioBusy] = STAGE_DEF(UpdateTaskStageGroupRadio, 5), - [UpdateTaskStageOBValidation] = STAGE_DEF(UpdateTaskStageGroupOptionBytes, 10), + [UpdateTaskStageOBValidation] = STAGE_DEF(UpdateTaskStageGroupOptionBytes, 2), - [UpdateTaskStageValidateDFUImage] = STAGE_DEF(UpdateTaskStageGroupFirmware, 50), - [UpdateTaskStageFlashWrite] = STAGE_DEF(UpdateTaskStageGroupFirmware, 200), - [UpdateTaskStageFlashValidate] = STAGE_DEF(UpdateTaskStageGroupFirmware, 30), + [UpdateTaskStageValidateDFUImage] = STAGE_DEF(UpdateTaskStageGroupFirmware, 30), + [UpdateTaskStageFlashWrite] = STAGE_DEF(UpdateTaskStageGroupFirmware, 150), + [UpdateTaskStageFlashValidate] = STAGE_DEF(UpdateTaskStageGroupFirmware, 15), - [UpdateTaskStageLfsRestore] = STAGE_DEF(UpdateTaskStageGroupPostUpdate, 30), + [UpdateTaskStageLfsRestore] = STAGE_DEF(UpdateTaskStageGroupPostUpdate, 5), [UpdateTaskStageResourcesUpdate] = STAGE_DEF(UpdateTaskStageGroupResources, 255), [UpdateTaskStageSplashscreenInstall] = STAGE_DEF(UpdateTaskStageGroupSplashscreen, 5), diff --git a/applications/system/updater/util/update_task_worker_backup.c b/applications/system/updater/util/update_task_worker_backup.c index 78040106..ed53c353 100644 --- a/applications/system/updater/util/update_task_worker_backup.c +++ b/applications/system/updater/util/update_task_worker_backup.c @@ -41,6 +41,14 @@ static bool update_task_pre_update(UpdateTask* update_task) { return success; } +typedef enum { + UpdateTaskResourcesWeightsFileCleanup = 20, + UpdateTaskResourcesWeightsDirCleanup = 20, + UpdateTaskResourcesWeightsFileUnpack = 60, +} UpdateTaskResourcesWeights; + +#define UPDATE_TASK_RESOURCES_FILE_TO_TOTAL_PERCENT 90 + typedef struct { UpdateTask* update_task; int32_t total_files, processed_files; @@ -54,33 +62,36 @@ static bool update_task_resource_unpack_cb(const char* name, bool is_directory, update_task_set_progress( unpack_progress->update_task, UpdateTaskStageProgress, - /* For this stage, last 70% of progress = extraction */ - 30 + (unpack_progress->processed_files * 70) / (unpack_progress->total_files + 1)); + /* For this stage, last progress segment = extraction */ + (UpdateTaskResourcesWeightsFileCleanup + UpdateTaskResourcesWeightsDirCleanup) + + (unpack_progress->processed_files * UpdateTaskResourcesWeightsFileUnpack) / + (unpack_progress->total_files + 1)); return true; } -static void - update_task_cleanup_resources(UpdateTask* update_task, uint32_t n_approx_file_entries) { +static void update_task_cleanup_resources(UpdateTask* update_task, const uint32_t n_tar_entries) { ResourceManifestReader* manifest_reader = resource_manifest_reader_alloc(update_task->storage); do { - FURI_LOG_I(TAG, "Cleaning up old manifest"); + FURI_LOG_D(TAG, "Cleaning up old manifest"); if(!resource_manifest_reader_open(manifest_reader, EXT_PATH("Manifest"))) { FURI_LOG_W(TAG, "No existing manifest"); break; } - /* We got # of entries in TAR file. Approx 1/4th is dir entries, we skip them */ - n_approx_file_entries = n_approx_file_entries * 3 / 4 + 1; - uint32_t n_processed_files = 0; + const uint32_t n_approx_file_entries = + n_tar_entries * UPDATE_TASK_RESOURCES_FILE_TO_TOTAL_PERCENT / 100 + 1; + uint32_t n_dir_entries = 1; ResourceManifestEntry* entry_ptr = NULL; + uint32_t n_processed_entries = 0; while((entry_ptr = resource_manifest_reader_next(manifest_reader))) { if(entry_ptr->type == ResourceManifestEntryTypeFile) { update_task_set_progress( update_task, UpdateTaskStageProgress, - /* For this stage, first 20% of progress = cleanup files */ - (n_processed_files++ * 20) / (n_approx_file_entries + 1)); + /* For this stage, first pass = old manifest's file cleanup */ + (n_processed_entries++ * UpdateTaskResourcesWeightsFileCleanup) / + n_approx_file_entries); FuriString* file_path = furi_string_alloc(); path_concat( @@ -88,16 +99,21 @@ static void FURI_LOG_D(TAG, "Removing %s", furi_string_get_cstr(file_path)); storage_simply_remove(update_task->storage, furi_string_get_cstr(file_path)); furi_string_free(file_path); + } else if(entry_ptr->type == ResourceManifestEntryTypeDirectory) { + n_dir_entries++; } } + n_processed_entries = 0; while((entry_ptr = resource_manifest_reader_previous(manifest_reader))) { if(entry_ptr->type == ResourceManifestEntryTypeDirectory) { update_task_set_progress( update_task, UpdateTaskStageProgress, /* For this stage, second 10% of progress = cleanup directories */ - (n_processed_files++ * 10) / (n_approx_file_entries + 1)); + UpdateTaskResourcesWeightsFileCleanup + + (n_processed_entries++ * UpdateTaskResourcesWeightsDirCleanup) / + n_dir_entries); FuriString* folder_path = furi_string_alloc(); File* folder_file = storage_file_alloc(update_task->storage); diff --git a/lib/toolbox/tar/tar_archive.c b/lib/toolbox/tar/tar_archive.c index fd0d175e..01969b47 100644 --- a/lib/toolbox/tar/tar_archive.c +++ b/lib/toolbox/tar/tar_archive.c @@ -106,6 +106,7 @@ void tar_archive_set_file_callback(TarArchive* archive, tar_unpack_file_cb callb static int tar_archive_entry_counter(mtar_t* tar, const mtar_header_t* header, void* param) { UNUSED(tar); UNUSED(header); + furi_assert(param); int32_t* counter = param; (*counter)++; return 0; diff --git a/scripts/fbt_tools/pvsstudio.py b/scripts/fbt_tools/pvsstudio.py index 02014836..593559a3 100644 --- a/scripts/fbt_tools/pvsstudio.py +++ b/scripts/fbt_tools/pvsstudio.py @@ -1,6 +1,6 @@ from SCons.Builder import Builder from SCons.Action import Action -from SCons.Script import Delete, Mkdir, GetBuildFailures +from SCons.Script import Delete, Mkdir, GetBuildFailures, Flatten import multiprocessing import webbrowser import atexit @@ -30,13 +30,14 @@ def atexist_handler(): return for bf in GetBuildFailures(): - if bf.node.exists and bf.node.name.endswith(".html"): - # macOS - if sys.platform == "darwin": - subprocess.run(["open", bf.node.abspath]) - else: - webbrowser.open(bf.node.abspath) - break + for node in Flatten(bf.node): + if node.exists and node.name.endswith(".html"): + # macOS + if sys.platform == "darwin": + subprocess.run(["open", node.abspath]) + else: + webbrowser.open(node.abspath) + break def generate(env):