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

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);
+    }
 }

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);
+    }
 }

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;

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),

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);

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;

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):