From 8dc1edac18139aed83ee2f389c33e338403c085f Mon Sep 17 00:00:00 2001 From: Sergey Gavrilov Date: Thu, 13 Jul 2023 15:02:59 +0300 Subject: [PATCH] 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 --- .../services/desktop/desktop_settings.h | 3 +- applications/services/loader/loader.c | 60 ++++++++++++------- .../services/loader/loader_applications.c | 2 +- applications/services/loader/loader_menu.c | 2 +- .../scenes/desktop_settings_scene_favorite.c | 44 ++++++++------ 5 files changed, 68 insertions(+), 43 deletions(-) diff --git a/applications/services/desktop/desktop_settings.h b/applications/services/desktop/desktop_settings.h index 7ab39094d..9b88868a8 100644 --- a/applications/services/desktop/desktop_settings.h +++ b/applications/services/desktop/desktop_settings.h @@ -8,7 +8,7 @@ #include #include -#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; diff --git a/applications/services/loader/loader.c b/applications/services/loader/loader.c index e7fa38596..41c0f95d4 100644 --- a/applications/services/loader/loader.c +++ b/applications/services/loader/loader.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -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 - // so i prefer to not show LoaderStatusErrorAppStarted error message for now - if(status == LoaderStatusErrorUnknownApp || status == LoaderStatusErrorInternal) { + if(status == LoaderStatusErrorUnknownApp && + loader_find_external_application_by_name(name) != NULL) { + // 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)) { diff --git a/applications/services/loader/loader_applications.c b/applications/services/loader/loader_applications.c index 7bf189e55..8ae91d764 100644 --- a/applications/services/loader/loader_applications.c +++ b/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); diff --git a/applications/services/loader/loader_menu.c b/applications/services/loader/loader_menu.c index 2c48b0923..149fea72c 100644 --- a/applications/services/loader/loader_menu.c +++ b/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); } diff --git a/applications/settings/desktop_settings/scenes/desktop_settings_scene_favorite.c b/applications/settings/desktop_settings/scenes/desktop_settings_scene_favorite.c index c35a10568..26e7bc587 100644 --- a/applications/settings/desktop_settings/scenes/desktop_settings_scene_favorite.c +++ b/applications/settings/desktop_settings/scenes/desktop_settings_scene_favorite.c @@ -5,11 +5,26 @@ #include #include +#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++) { - submenu_add_item( - submenu, - FLIPPER_APPS[i].name, - i, - desktop_settings_scene_favorite_submenu_callback, - app); + for(size_t i = 0; i < APPS_COUNT; i++) { + const char* name = favorite_fap_get_app_name(i); + + submenu_add_item(submenu, name, i, desktop_settings_scene_favorite_submenu_callback, app); // Select favorite item in submenu - if(!curr_favorite_app->is_external && - !strcmp(FLIPPER_APPS[i].name, curr_favorite_app->name_or_path)) { + if(!strcmp(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; - strncpy( - curr_favorite_app->name_or_path, FLIPPER_APPS[event.event].name, MAX_APP_LENGTH); + const char* name = favorite_fap_get_app_name(event.event); + if(name) strncpy(curr_favorite_app->name_or_path, name, MAX_APP_LENGTH); consumed = true; } if(consumed) {