Loader: good looking error messages (#2873)

* Loader: special error for unknown external app
* Loader: update special error
* Loader: beautify GUI errors, remove redundant logs
* Loader: fix gui error vertical position
* Desktop settings: add external menu apps
* Desktop: smaller settings struct and fix incorrect behavior with ext apps

Co-authored-by: Aleksandr Kutuzov <alleteam@gmail.com>

applications/services/desktop/desktop_settings.h

@@ -8,7 +8,7 @@
 #include <toolbox/saved_struct.h>
 #include <storage/storage.h>
 
-#define DESKTOP_SETTINGS_VER (7)
+#define DESKTOP_SETTINGS_VER (8)
 
 #define DESKTOP_SETTINGS_PATH INT_PATH(DESKTOP_SETTINGS_FILE_NAME)
 #define DESKTOP_SETTINGS_MAGIC (0x17)
@@ -42,7 +42,6 @@ typedef struct {
 } PinCode;
 
 typedef struct {
-    bool is_external;
     char name_or_path[MAX_APP_LENGTH];
 } FavoriteApp;
 

applications/services/loader/loader.c

@@ -3,6 +3,7 @@
 #include <applications.h>
 #include <storage/storage.h>
 #include <furi_hal.h>
+#include <assets_icons.h>
 
 #include <dialogs/dialogs.h>
 #include <toolbox/path.h>
@@ -11,7 +12,20 @@
 
 #define TAG "Loader"
 #define LOADER_MAGIC_THREAD_VALUE 0xDEADBEEF
-// api
+
+// helpers
+
+static const char* loader_find_external_application_by_name(const char* app_name) {
+    for(size_t i = 0; i < FLIPPER_EXTERNAL_APPS_COUNT; i++) {
+        if(strcmp(FLIPPER_EXTERNAL_APPS[i].name, app_name) == 0) {
+            return FLIPPER_EXTERNAL_APPS[i].path;
+        }
+    }
+
+    return NULL;
+}
+
+// API
 
 LoaderStatus
     loader_start(Loader* loader, const char* name, const char* args, FuriString* error_message) {
@@ -33,17 +47,33 @@ LoaderStatus loader_start_with_gui_error(Loader* loader, const char* name, const
     FuriString* error_message = furi_string_alloc();
     LoaderStatus status = loader_start(loader, name, args, error_message);
 
-    // TODO: we have many places where we can emit a double start, ex: desktop, menu
+    if(status == LoaderStatusErrorUnknownApp &&
-    // so i prefer to not show LoaderStatusErrorAppStarted error message for now
+       loader_find_external_application_by_name(name) != NULL) {
-    if(status == LoaderStatusErrorUnknownApp || status == LoaderStatusErrorInternal) {
+        // Special case for external apps
+        DialogsApp* dialogs = furi_record_open(RECORD_DIALOGS);
+        DialogMessage* message = dialog_message_alloc();
+        dialog_message_set_header(message, "Update needed", 64, 3, AlignCenter, AlignTop);
+        dialog_message_set_buttons(message, NULL, NULL, NULL);
+        dialog_message_set_icon(message, &I_DolphinCommon_56x48, 72, 17);
+        dialog_message_set_text(
+            message, "Update firmware\nto run this app", 3, 26, AlignLeft, AlignTop);
+        dialog_message_show(dialogs, message);
+        dialog_message_free(message);
+        furi_record_close(RECORD_DIALOGS);
+    } else if(status == LoaderStatusErrorUnknownApp || status == LoaderStatusErrorInternal) {
+        // TODO: we have many places where we can emit a double start, ex: desktop, menu
+        // so i prefer to not show LoaderStatusErrorAppStarted error message for now
         DialogsApp* dialogs = furi_record_open(RECORD_DIALOGS);
         DialogMessage* message = dialog_message_alloc();
         dialog_message_set_header(message, "Error", 64, 0, AlignCenter, AlignTop);
         dialog_message_set_buttons(message, NULL, NULL, NULL);
 
-        furi_string_replace(error_message, ":", "\n");
+        furi_string_replace(error_message, "/ext/apps/", "");
+        furi_string_replace(error_message, ", ", "\n");
+        furi_string_replace(error_message, ": ", "\n");
+
         dialog_message_set_text(
-            message, furi_string_get_cstr(error_message), 64, 32, AlignCenter, AlignCenter);
+            message, furi_string_get_cstr(error_message), 64, 35, AlignCenter, AlignCenter);
 
         dialog_message_show(dialogs, message);
         dialog_message_free(message);
@@ -170,16 +200,6 @@ static const FlipperInternalApplication* loader_find_application_by_name(const c
     return application;
 }
 
-static const char* loader_find_external_application_by_name(const char* app_name) {
-    for(size_t i = 0; i < FLIPPER_EXTERNAL_APPS_COUNT; i++) {
-        if(strcmp(FLIPPER_EXTERNAL_APPS[i].name, app_name) == 0) {
-            return FLIPPER_EXTERNAL_APPS[i].path;
-        }
-    }
-
-    return NULL;
-}
-
 static void loader_start_app_thread(Loader* loader, FlipperInternalApplicationFlag flags) {
     // setup heap trace
     FuriHalRtcHeapTrackMode mode = furi_hal_rtc_get_heap_track_mode();
@@ -278,22 +298,20 @@ static LoaderStatus loader_start_external_app(
         if(preload_res != FlipperApplicationPreloadStatusSuccess) {
             const char* err_msg = flipper_application_preload_status_to_string(preload_res);
             status = loader_make_status_error(
-                LoaderStatusErrorInternal, error_message, "Preload failed %s: %s", path, err_msg);
+                LoaderStatusErrorInternal, error_message, "Preload failed, %s: %s", path, err_msg);
             break;
         }
 
-        FURI_LOG_I(TAG, "Mapping");
         FlipperApplicationLoadStatus load_status =
             flipper_application_map_to_memory(loader->app.fap);
         if(load_status != FlipperApplicationLoadStatusSuccess) {
             const char* err_msg = flipper_application_load_status_to_string(load_status);
             status = loader_make_status_error(
-                LoaderStatusErrorInternal, error_message, "Load failed %s: %s", path, err_msg);
+                LoaderStatusErrorInternal, error_message, "Load failed, %s: %s", path, err_msg);
             break;
         }
 
         FURI_LOG_I(TAG, "Loaded in %zums", (size_t)(furi_get_tick() - start));
-        FURI_LOG_I(TAG, "Starting app");
 
         loader->app.thread = flipper_application_alloc_thread(loader->app.fap, args);
         FuriString* app_name = furi_string_alloc();
@@ -397,7 +415,7 @@ static LoaderStatus loader_do_start_by_name(
             }
         }
 
-        // check external apps
+        // check Faps
         {
             Storage* storage = furi_record_open(RECORD_STORAGE);
             if(storage_file_exists(storage, name)) {

applications/services/loader/loader_applications.c

@@ -20,7 +20,7 @@ static int32_t loader_applications_thread(void* p);
 LoaderApplications* loader_applications_alloc(void (*closed_cb)(void*), void* context) {
     LoaderApplications* loader_applications = malloc(sizeof(LoaderApplications));
     loader_applications->thread =
-        furi_thread_alloc_ex(TAG, 512, loader_applications_thread, (void*)loader_applications);
+        furi_thread_alloc_ex(TAG, 768, loader_applications_thread, (void*)loader_applications);
     loader_applications->closed_cb = closed_cb;
     loader_applications->context = context;
     furi_thread_start(loader_applications->thread);

applications/services/loader/loader_menu.c

@@ -60,7 +60,7 @@ static void loader_menu_apps_callback(void* context, uint32_t index) {
 
 static void loader_menu_external_apps_callback(void* context, uint32_t index) {
     UNUSED(context);
-    const char* path = FLIPPER_EXTERNAL_APPS[index].path;
+    const char* path = FLIPPER_EXTERNAL_APPS[index].name;
     loader_menu_start(path);
 }
 

applications/settings/desktop_settings/scenes/desktop_settings_scene_favorite.c

@@ -5,11 +5,26 @@
 #include <storage/storage.h>
 #include <dialogs/dialogs.h>
 
+#define APPS_COUNT (FLIPPER_APPS_COUNT + FLIPPER_EXTERNAL_APPS_COUNT)
+
 #define EXTERNAL_BROWSER_NAME ("Applications")
-#define EXTERNAL_BROWSER_INDEX (FLIPPER_APPS_COUNT + 1)
+#define EXTERNAL_BROWSER_INDEX (APPS_COUNT + 1)
 
 #define EXTERNAL_APPLICATION_NAME ("[External Application]")
-#define EXTERNAL_APPLICATION_INDEX (FLIPPER_APPS_COUNT + 2)
+#define EXTERNAL_APPLICATION_INDEX (APPS_COUNT + 2)
+
+#define PRESELECTED_SPECIAL 0xffffffff
+
+static const char* favorite_fap_get_app_name(size_t i) {
+    const char* name;
+    if(i < FLIPPER_APPS_COUNT) {
+        name = FLIPPER_APPS[i].name;
+    } else {
+        name = FLIPPER_EXTERNAL_APPS[i - FLIPPER_APPS_COUNT].name;
+    }
+
+    return name;
+}
 
 static bool favorite_fap_selector_item_callback(
     FuriString* file_path,
@@ -42,21 +57,17 @@ void desktop_settings_scene_favorite_on_enter(void* context) {
 
     uint32_t primary_favorite =
         scene_manager_get_scene_state(app->scene_manager, DesktopSettingsAppSceneFavorite);
-    uint32_t pre_select_item = 0;
+    uint32_t pre_select_item = PRESELECTED_SPECIAL;
     FavoriteApp* curr_favorite_app = primary_favorite ? &app->settings.favorite_primary :
                                                         &app->settings.favorite_secondary;
 
-    for(size_t i = 0; i < FLIPPER_APPS_COUNT; i++) {
+    for(size_t i = 0; i < APPS_COUNT; i++) {
-        submenu_add_item(
+        const char* name = favorite_fap_get_app_name(i);
-            submenu,
+
-            FLIPPER_APPS[i].name,
+        submenu_add_item(submenu, name, i, desktop_settings_scene_favorite_submenu_callback, app);
-            i,
-            desktop_settings_scene_favorite_submenu_callback,
-            app);
 
         // Select favorite item in submenu
-        if(!curr_favorite_app->is_external &&
+        if(!strcmp(name, curr_favorite_app->name_or_path)) {
-           !strcmp(FLIPPER_APPS[i].name, curr_favorite_app->name_or_path)) {
             pre_select_item = i;
         }
     }
@@ -77,7 +88,7 @@ void desktop_settings_scene_favorite_on_enter(void* context) {
         desktop_settings_scene_favorite_submenu_callback,
         app);
 
-    if(curr_favorite_app->is_external) {
+    if(pre_select_item == PRESELECTED_SPECIAL) {
         if(curr_favorite_app->name_or_path[0] == '\0') {
             pre_select_item = EXTERNAL_BROWSER_INDEX;
         } else {
@@ -104,7 +115,6 @@ bool desktop_settings_scene_favorite_on_event(void* context, SceneManagerEvent e
 
     if(event.type == SceneManagerEventTypeCustom) {
         if(event.event == EXTERNAL_BROWSER_INDEX) {
-            curr_favorite_app->is_external = true;
             curr_favorite_app->name_or_path[0] = '\0';
             consumed = true;
         } else if(event.event == EXTERNAL_APPLICATION_INDEX) {
@@ -125,7 +135,6 @@ bool desktop_settings_scene_favorite_on_event(void* context, SceneManagerEvent e
 
             if(dialog_file_browser_show(app->dialogs, temp_path, temp_path, &browser_options)) {
                 submenu_reset(app->submenu); // Prevent menu from being shown when we exiting scene
-                curr_favorite_app->is_external = true;
                 strncpy(
                     curr_favorite_app->name_or_path,
                     furi_string_get_cstr(temp_path),
@@ -133,9 +142,8 @@ bool desktop_settings_scene_favorite_on_event(void* context, SceneManagerEvent e
                 consumed = true;
             }
         } else {
-            curr_favorite_app->is_external = false;
+            const char* name = favorite_fap_get_app_name(event.event);
-            strncpy(
+            if(name) strncpy(curr_favorite_app->name_or_path, name, MAX_APP_LENGTH);
-                curr_favorite_app->name_or_path, FLIPPER_APPS[event.event].name, MAX_APP_LENGTH);
             consumed = true;
         }
         if(consumed) {