[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 <alleteam@gmail.com>

applications/desktop/desktop.c

@@ -4,8 +4,6 @@
 #include <furi.h>
 #include <furi_hal.h>
 
-#include <loader/loader.h>
-
 #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;
 

applications/desktop/desktop_i.h

@@ -18,6 +18,8 @@
 #include <gui/modules/popup.h>
 #include <gui/scene_manager.h>
 
+#include <loader/loader.h>
+
 #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;
 };
 

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 {

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

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;

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

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;

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() {

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;