From 425a74a001e5ce36db12e8e0c967abcd54a2b814 Mon Sep 17 00:00:00 2001 From: SG Date: Mon, 28 Mar 2022 23:42:31 +1000 Subject: [PATCH] [FL-2383, FL-2384] iButton, Desktop bug fixes (#1062) * iButton HAL: correct init and emulation sequence * FuriHal: cleanup init/deinit routines for emulation timer * FuriHal: cleanup TIM initialization routines in RFID and SubGhz * iButton: delay in emulate loop * Desktop: fix crash caused by invalid animation manager state, described in FL-2384 Co-authored-by: Aleksandr Kutuzov --- applications/desktop/desktop.c | 39 ++++++++++++---- applications/desktop/desktop_i.h | 4 +- .../desktop/scenes/desktop_scene_lock_menu.c | 4 +- .../desktop/scenes/desktop_scene_main.c | 46 +------------------ applications/desktop/views/desktop_events.h | 5 +- applications/ibutton/ibutton_cli.cpp | 1 + .../targets/f7/furi_hal/furi_hal_ibutton.c | 16 ++++--- firmware/targets/f7/furi_hal/furi_hal_rfid.c | 3 +- .../targets/f7/furi_hal/furi_hal_subghz.c | 11 ++--- 9 files changed, 55 insertions(+), 74 deletions(-) diff --git a/applications/desktop/desktop.c b/applications/desktop/desktop.c index 1e3edf77..0f969ce2 100644 --- a/applications/desktop/desktop.c +++ b/applications/desktop/desktop.c @@ -4,8 +4,6 @@ #include #include -#include - #include "animations/animation_manager.h" #include "desktop/scenes/desktop_scene.h" #include "desktop/scenes/desktop_scene_i.h" @@ -15,6 +13,18 @@ #include "desktop_i.h" #include "desktop_helpers.h" +static void desktop_loader_callback(const void* message, void* context) { + furi_assert(context); + Desktop* desktop = context; + const LoaderEvent* event = message; + + if(event->type == LoaderEventTypeApplicationStarted) { + view_dispatcher_send_custom_event(desktop->view_dispatcher, DesktopGlobalBeforeAppStarted); + } else if(event->type == LoaderEventTypeApplicationStopped) { + view_dispatcher_send_custom_event(desktop->view_dispatcher, DesktopGlobalAfterAppFinished); + } +} + static void desktop_lock_icon_callback(Canvas* canvas, void* context) { furi_assert(canvas); canvas_draw_icon(canvas, 0, 0, &I_Lock_8x8); @@ -23,6 +33,16 @@ static void desktop_lock_icon_callback(Canvas* canvas, void* context) { static bool desktop_custom_event_callback(void* context, uint32_t event) { furi_assert(context); Desktop* desktop = (Desktop*)context; + + switch(event) { + case DesktopGlobalBeforeAppStarted: + animation_manager_unload_and_stall_animation(desktop->animation_manager); + return true; + case DesktopGlobalAfterAppFinished: + animation_manager_load_and_continue_animation(desktop->animation_manager); + return true; + } + return scene_manager_handle_custom_event(desktop->scene_manager, event); } @@ -41,7 +61,6 @@ static void desktop_tick_event_callback(void* context) { Desktop* desktop_alloc() { Desktop* desktop = malloc(sizeof(Desktop)); - desktop->unload_animation_semaphore = osSemaphoreNew(1, 0, NULL); desktop->animation_manager = animation_manager_alloc(); desktop->gui = furi_record_open("gui"); desktop->scene_thread = furi_thread_alloc(); @@ -122,12 +141,13 @@ Desktop* desktop_alloc() { gui_add_view_port(desktop->gui, desktop->lock_viewport, GuiLayerStatusBarLeft); // Special case: autostart application is already running - Loader* loader = furi_record_open("loader"); - if(loader_is_locked(loader) && + desktop->loader = furi_record_open("loader"); + if(loader_is_locked(desktop->loader) && animation_manager_is_animation_loaded(desktop->animation_manager)) { animation_manager_unload_and_stall_animation(desktop->animation_manager); } - furi_record_close("loader"); + desktop->app_start_stop_subscription = furi_pubsub_subscribe( + loader_get_pubsub(desktop->loader), desktop_loader_callback, desktop); return desktop; } @@ -135,6 +155,11 @@ Desktop* desktop_alloc() { void desktop_free(Desktop* desktop) { furi_assert(desktop); + furi_pubsub_unsubscribe( + loader_get_pubsub(desktop->loader), desktop->app_start_stop_subscription); + desktop->loader = NULL; + furi_record_close("loader"); + view_dispatcher_remove_view(desktop->view_dispatcher, DesktopViewIdMain); view_dispatcher_remove_view(desktop->view_dispatcher, DesktopViewIdLockMenu); view_dispatcher_remove_view(desktop->view_dispatcher, DesktopViewIdLocked); @@ -159,8 +184,6 @@ void desktop_free(Desktop* desktop) { popup_free(desktop->hw_mismatch_popup); desktop_view_pin_timeout_free(desktop->pin_timeout_view); - osSemaphoreDelete(desktop->unload_animation_semaphore); - furi_record_close("gui"); desktop->gui = NULL; diff --git a/applications/desktop/desktop_i.h b/applications/desktop/desktop_i.h index fb15dbd7..8eb7be21 100644 --- a/applications/desktop/desktop_i.h +++ b/applications/desktop/desktop_i.h @@ -18,6 +18,8 @@ #include #include +#include + #define STATUS_BAR_Y_SHIFT 13 typedef enum { @@ -57,7 +59,7 @@ struct Desktop { ViewPort* lock_viewport; AnimationManager* animation_manager; - osSemaphoreId_t unload_animation_semaphore; + Loader* loader; FuriPubSubSubscription* app_start_stop_subscription; }; diff --git a/applications/desktop/scenes/desktop_scene_lock_menu.c b/applications/desktop/scenes/desktop_scene_lock_menu.c index 776e059e..058081eb 100644 --- a/applications/desktop/scenes/desktop_scene_lock_menu.c +++ b/applications/desktop/scenes/desktop_scene_lock_menu.c @@ -60,10 +60,8 @@ bool desktop_scene_lock_menu_on_event(void* context, SceneManagerEvent event) { desktop->scene_manager, DesktopSceneLocked, SCENE_LOCKED_FIRST_ENTER); scene_manager_next_scene(desktop->scene_manager, DesktopSceneLocked); } else { - Loader* loader = furi_record_open("loader"); LoaderStatus status = - loader_start(loader, "Desktop", DESKTOP_SETTINGS_RUN_PIN_SETUP_ARG); - furi_record_close("loader"); + loader_start(desktop->loader, "Desktop", DESKTOP_SETTINGS_RUN_PIN_SETUP_ARG); if(status == LoaderStatusOk) { scene_manager_set_scene_state(desktop->scene_manager, DesktopSceneLockMenu, 1); } else { diff --git a/applications/desktop/scenes/desktop_scene_main.c b/applications/desktop/scenes/desktop_scene_main.c index 7ba09e0f..719ba2e4 100644 --- a/applications/desktop/scenes/desktop_scene_main.c +++ b/applications/desktop/scenes/desktop_scene_main.c @@ -12,21 +12,6 @@ #define TAG "DesktopSrv" -static void desktop_scene_main_app_started_callback(const void* message, void* context) { - furi_assert(context); - Desktop* desktop = context; - const LoaderEvent* event = message; - - if(event->type == LoaderEventTypeApplicationStarted) { - view_dispatcher_send_custom_event( - desktop->view_dispatcher, DesktopMainEventBeforeAppStarted); - osSemaphoreAcquire(desktop->unload_animation_semaphore, osWaitForever); - } else if(event->type == LoaderEventTypeApplicationStopped) { - view_dispatcher_send_custom_event( - desktop->view_dispatcher, DesktopMainEventAfterAppFinished); - } -} - static void desktop_scene_main_new_idle_animation_callback(void* context) { furi_assert(context); Desktop* desktop = context; @@ -85,12 +70,6 @@ void desktop_scene_main_on_enter(void* context) { animation_manager_set_interact_callback( desktop->animation_manager, desktop_scene_main_interact_animation_callback); - furi_assert(osSemaphoreGetCount(desktop->unload_animation_semaphore) == 0); - Loader* loader = furi_record_open("loader"); - desktop->app_start_stop_subscription = furi_pubsub_subscribe( - loader_get_pubsub(loader), desktop_scene_main_app_started_callback, desktop); - furi_record_close("loader"); - desktop_main_set_callback(main_view, desktop_scene_main_callback, desktop); view_dispatcher_switch_to_view(desktop->view_dispatcher, DesktopViewIdMain); @@ -127,13 +106,11 @@ bool desktop_scene_main_on_event(void* context, SceneManagerEvent event) { case DesktopMainEventOpenFavorite: LOAD_DESKTOP_SETTINGS(&desktop->settings); if(desktop->settings.favorite < FLIPPER_APPS_COUNT) { - Loader* loader = furi_record_open("loader"); - LoaderStatus status = - loader_start(loader, FLIPPER_APPS[desktop->settings.favorite].name, NULL); + LoaderStatus status = loader_start( + desktop->loader, FLIPPER_APPS[desktop->settings.favorite].name, NULL); if(status != LoaderStatusOk) { FURI_LOG_E(TAG, "loader_start failed: %d", status); } - furi_record_close("loader"); } else { FURI_LOG_E(TAG, "Can't find favorite application"); } @@ -152,15 +129,6 @@ bool desktop_scene_main_on_event(void* context, SceneManagerEvent event) { animation_manager_interact_process(desktop->animation_manager); consumed = true; break; - case DesktopMainEventBeforeAppStarted: - animation_manager_unload_and_stall_animation(desktop->animation_manager); - osSemaphoreRelease(desktop->unload_animation_semaphore); - consumed = true; - break; - case DesktopMainEventAfterAppFinished: - animation_manager_load_and_continue_animation(desktop->animation_manager); - consumed = true; - break; case DesktopLockedEventUpdate: desktop_view_locked_update(desktop->locked_view); consumed = true; @@ -177,16 +145,6 @@ bool desktop_scene_main_on_event(void* context, SceneManagerEvent event) { void desktop_scene_main_on_exit(void* context) { Desktop* desktop = (Desktop*)context; - /** - * We're allowed to leave this scene only when any other app & loader - * is finished, that's why we can be sure there is no task waiting - * for start/stop semaphore - */ - Loader* loader = furi_record_open("loader"); - furi_pubsub_unsubscribe(loader_get_pubsub(loader), desktop->app_start_stop_subscription); - furi_record_close("loader"); - furi_assert(osSemaphoreGetCount(desktop->unload_animation_semaphore) == 0); - animation_manager_set_new_idle_callback(desktop->animation_manager, NULL); animation_manager_set_check_callback(desktop->animation_manager, NULL); animation_manager_set_interact_callback(desktop->animation_manager, NULL); diff --git a/applications/desktop/views/desktop_events.h b/applications/desktop/views/desktop_events.h index 49d6f81e..6e5eb2f6 100644 --- a/applications/desktop/views/desktop_events.h +++ b/applications/desktop/views/desktop_events.h @@ -7,8 +7,6 @@ typedef enum { DesktopMainEventOpenMenu, DesktopMainEventOpenDebug, DesktopMainEventRightShort, - DesktopMainEventBeforeAppStarted, - DesktopMainEventAfterAppFinished, DesktopLockedEventUnlocked, DesktopLockedEventUpdate, @@ -37,4 +35,7 @@ typedef enum { DesktopAnimationEventNewIdleAnimation, DesktopAnimationEventInteractAnimation, + // Global events + DesktopGlobalBeforeAppStarted, + DesktopGlobalAfterAppFinished, } DesktopEvent; diff --git a/applications/ibutton/ibutton_cli.cpp b/applications/ibutton/ibutton_cli.cpp index d46f8a23..3e41befa 100644 --- a/applications/ibutton/ibutton_cli.cpp +++ b/applications/ibutton/ibutton_cli.cpp @@ -209,6 +209,7 @@ void ibutton_cli_emulate(Cli* cli, string_t args) { while(!exit) { exit = cli_cmd_interrupt_received(cli); + delay(100); }; worker->stop_emulate(); diff --git a/firmware/targets/f7/furi_hal/furi_hal_ibutton.c b/firmware/targets/f7/furi_hal/furi_hal_ibutton.c index e4a18dd3..90ba1edc 100644 --- a/firmware/targets/f7/furi_hal/furi_hal_ibutton.c +++ b/firmware/targets/f7/furi_hal/furi_hal_ibutton.c @@ -49,6 +49,11 @@ void furi_hal_ibutton_emulate_start( LL_TIM_DeInit(FURI_HAL_IBUTTON_TIMER); FURI_CRITICAL_EXIT(); + furi_hal_interrupt_set_timer_isr(FURI_HAL_IBUTTON_TIMER, furi_hal_ibutton_emulate_isr); + NVIC_SetPriority( + FURI_HAL_IBUTTON_TIMER_IRQ, NVIC_EncodePriority(NVIC_GetPriorityGrouping(), 5, 0)); + NVIC_EnableIRQ(FURI_HAL_IBUTTON_TIMER_IRQ); + LL_TIM_SetPrescaler(FURI_HAL_IBUTTON_TIMER, 0); LL_TIM_SetCounterMode(FURI_HAL_IBUTTON_TIMER, LL_TIM_COUNTERMODE_UP); LL_TIM_SetAutoReload(FURI_HAL_IBUTTON_TIMER, period); @@ -61,12 +66,6 @@ void furi_hal_ibutton_emulate_start( LL_TIM_EnableIT_UPDATE(FURI_HAL_IBUTTON_TIMER); - furi_hal_interrupt_set_timer_isr(FURI_HAL_IBUTTON_TIMER, furi_hal_ibutton_emulate_isr); - - NVIC_SetPriority( - FURI_HAL_IBUTTON_TIMER_IRQ, NVIC_EncodePriority(NVIC_GetPriorityGrouping(), 5, 0)); - NVIC_EnableIRQ(FURI_HAL_IBUTTON_TIMER_IRQ); - LL_TIM_EnableCounter(FURI_HAL_IBUTTON_TIMER); } @@ -80,6 +79,11 @@ void furi_hal_ibutton_emulate_stop() { if(furi_hal_ibutton->state == FuriHalIbuttonStateRunning) { furi_hal_ibutton->state = FuriHalIbuttonStateIdle; LL_TIM_DisableCounter(FURI_HAL_IBUTTON_TIMER); + + FURI_CRITICAL_ENTER(); + LL_TIM_DeInit(FURI_HAL_IBUTTON_TIMER); + FURI_CRITICAL_EXIT(); + furi_hal_interrupt_set_timer_isr(FURI_HAL_IBUTTON_TIMER, NULL); furi_hal_ibutton->callback = NULL; diff --git a/firmware/targets/f7/furi_hal/furi_hal_rfid.c b/firmware/targets/f7/furi_hal/furi_hal_rfid.c index a86a26ec..6efd83a1 100644 --- a/firmware/targets/f7/furi_hal/furi_hal_rfid.c +++ b/firmware/targets/f7/furi_hal/furi_hal_rfid.c @@ -193,7 +193,6 @@ void furi_hal_rfid_tim_emulate_start(FuriHalRfidEmulateCallback callback, void* } furi_hal_interrupt_set_timer_isr(FURI_HAL_RFID_EMULATE_TIMER, furi_hal_rfid_emulate_isr); - NVIC_SetPriority( FURI_HAL_RFID_EMULATE_TIMER_IRQ, NVIC_EncodePriority(NVIC_GetPriorityGrouping(), 5, 0)); NVIC_EnableIRQ(FURI_HAL_RFID_EMULATE_TIMER_IRQ); @@ -204,9 +203,9 @@ void furi_hal_rfid_tim_emulate_start(FuriHalRfidEmulateCallback callback, void* } void furi_hal_rfid_tim_emulate_stop() { - furi_hal_interrupt_set_timer_isr(FURI_HAL_RFID_EMULATE_TIMER, NULL); LL_TIM_DisableCounter(FURI_HAL_RFID_EMULATE_TIMER); LL_TIM_DisableAllOutputs(FURI_HAL_RFID_EMULATE_TIMER); + furi_hal_interrupt_set_timer_isr(FURI_HAL_RFID_EMULATE_TIMER, NULL); } void furi_hal_rfid_tim_reset() { diff --git a/firmware/targets/f7/furi_hal/furi_hal_subghz.c b/firmware/targets/f7/furi_hal/furi_hal_subghz.c index b7825558..1c498ed3 100644 --- a/firmware/targets/f7/furi_hal/furi_hal_subghz.c +++ b/firmware/targets/f7/furi_hal/furi_hal_subghz.c @@ -732,10 +732,6 @@ void furi_hal_subghz_start_async_rx(FuriHalSubGhzCaptureCallback callback, void* LL_TIM_CC_EnableChannel(TIM2, LL_TIM_CHANNEL_CH1); LL_TIM_CC_EnableChannel(TIM2, LL_TIM_CHANNEL_CH2); - // Enable NVIC - NVIC_SetPriority(TIM2_IRQn, NVIC_EncodePriority(NVIC_GetPriorityGrouping(), 5, 0)); - NVIC_EnableIRQ(TIM2_IRQn); - // Start timer LL_TIM_SetCounter(TIM2, 0); LL_TIM_EnableCounter(TIM2); @@ -919,6 +915,9 @@ bool furi_hal_subghz_start_async_tx(FuriHalSubGhzAsyncTxCallback callback, void* LL_TIM_DisableMasterSlaveMode(TIM2); furi_hal_interrupt_set_timer_isr(TIM2, furi_hal_subghz_async_tx_timer_isr); + NVIC_SetPriority(TIM2_IRQn, NVIC_EncodePriority(NVIC_GetPriorityGrouping(), 5, 0)); + NVIC_EnableIRQ(TIM2_IRQn); + LL_TIM_EnableIT_UPDATE(TIM2); LL_TIM_EnableDMAReq_UPDATE(TIM2); LL_TIM_CC_EnableChannel(TIM2, LL_TIM_CHANNEL_CH2); @@ -930,10 +929,6 @@ bool furi_hal_subghz_start_async_tx(FuriHalSubGhzAsyncTxCallback callback, void* #endif furi_hal_subghz_tx(); - // Enable NVIC - NVIC_SetPriority(TIM2_IRQn, NVIC_EncodePriority(NVIC_GetPriorityGrouping(), 5, 0)); - NVIC_EnableIRQ(TIM2_IRQn); - LL_TIM_SetCounter(TIM2, 0); LL_TIM_EnableCounter(TIM2); return true;