diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a4deadc0..3e3969f86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,8 @@ - SmartRider parser (by @jaylikesbunda) * Apps: **Check out more Apps updates and fixes by following** [this link](https://github.com/xMasterX/all-the-plugins/commits/dev) ## Other changes +* FuriHalSerial: Fix RXFNE interrupt hang, aka freezing with UART output when Expansion Modules are enabled (by @WillyJL) +* Expansion: add is_connected api (by @HaxSam & @WillyJL) * RFID 125khz: Fix strange bug with LCD backlight going off after doing "Write" * GUI: Added `submenu_remove_item()` to API, was needed for NFC Type 4 related changes (by @WillyJL) * SubGHz: Fix possible frequency analyzer deadlock when holding Ok (by @WillyJL) diff --git a/applications/services/expansion/expansion.c b/applications/services/expansion/expansion.c index 219bf0641..2b0c5b27a 100644 --- a/applications/services/expansion/expansion.c +++ b/applications/services/expansion/expansion.c @@ -18,6 +18,7 @@ typedef enum { ExpansionStateDisabled, ExpansionStateEnabled, ExpansionStateRunning, + ExpansionStateConnectionEstablished, } ExpansionState; typedef enum { @@ -27,10 +28,13 @@ typedef enum { ExpansionMessageTypeReloadSettings, ExpansionMessageTypeModuleConnected, ExpansionMessageTypeModuleDisconnected, + ExpansionMessageTypeConnectionEstablished, + ExpansionMessageTypeIsConnected, } ExpansionMessageType; typedef union { FuriHalSerialId serial_id; + bool* is_connected; } ExpansionMessageData; typedef struct { @@ -67,13 +71,21 @@ static void expansion_detect_callback(void* context) { UNUSED(status); } -static void expansion_worker_callback(void* context) { +static void expansion_worker_callback(void* context, ExpansionWorkerCallbackReason reason) { furi_assert(context); Expansion* instance = context; - ExpansionMessage message = { - .type = ExpansionMessageTypeModuleDisconnected, - .api_lock = NULL, // Not locking the API here to avoid a deadlock + ExpansionMessage message; + switch(reason) { + case ExpansionWorkerCallbackReasonExit: + message.type = ExpansionMessageTypeModuleDisconnected; + message.api_lock = NULL; // Not locking the API here to avoid a deadlock + break; + + case ExpansionWorkerCallbackReasonConnected: + message.type = ExpansionMessageTypeConnectionEstablished; + message.api_lock = api_lock_alloc_locked(); + break; }; const FuriStatus status = furi_message_queue_put(instance->queue, &message, FuriWaitForever); @@ -106,7 +118,9 @@ static void UNUSED(data); if(instance->state == ExpansionStateDisabled) { return; - } else if(instance->state == ExpansionStateRunning) { + } else if( + instance->state == ExpansionStateRunning || + instance->state == ExpansionStateConnectionEstablished) { expansion_worker_stop(instance->worker); expansion_worker_free(instance->worker); } else { @@ -124,7 +138,9 @@ static void expansion_control_handler_set_listen_serial( if(instance->state != ExpansionStateDisabled && instance->serial_id == data->serial_id) { return; - } else if(instance->state == ExpansionStateRunning) { + } else if( + instance->state == ExpansionStateRunning || + instance->state == ExpansionStateConnectionEstablished) { expansion_worker_stop(instance->worker); expansion_worker_free(instance->worker); @@ -182,7 +198,8 @@ static void expansion_control_handler_module_disconnected( Expansion* instance, const ExpansionMessageData* data) { UNUSED(data); - if(instance->state != ExpansionStateRunning) { + if(instance->state != ExpansionStateRunning && + instance->state != ExpansionStateConnectionEstablished) { return; } @@ -192,6 +209,23 @@ static void expansion_control_handler_module_disconnected( instance->serial_id, expansion_detect_callback, instance); } +static void expansion_control_handler_connection_established( + Expansion* instance, + const ExpansionMessageData* data) { + UNUSED(data); + if(instance->state != ExpansionStateRunning && + instance->state != ExpansionStateConnectionEstablished) { + return; + } + + instance->state = ExpansionStateConnectionEstablished; +} + +static void + expansion_control_handler_is_connected(Expansion* instance, const ExpansionMessageData* data) { + *data->is_connected = instance->state == ExpansionStateConnectionEstablished; +} + typedef void (*ExpansionControlHandler)(Expansion*, const ExpansionMessageData*); static const ExpansionControlHandler expansion_control_handlers[] = { @@ -201,6 +235,8 @@ static const ExpansionControlHandler expansion_control_handlers[] = { [ExpansionMessageTypeReloadSettings] = expansion_control_handler_reload_settings, [ExpansionMessageTypeModuleConnected] = expansion_control_handler_module_connected, [ExpansionMessageTypeModuleDisconnected] = expansion_control_handler_module_disconnected, + [ExpansionMessageTypeConnectionEstablished] = expansion_control_handler_connection_established, + [ExpansionMessageTypeIsConnected] = expansion_control_handler_is_connected, }; static int32_t expansion_control(void* context) { @@ -295,6 +331,22 @@ void expansion_disable(Expansion* instance) { api_lock_wait_unlock_and_free(message.api_lock); } +bool expansion_is_connected(Expansion* instance) { + furi_check(instance); + bool is_connected; + + ExpansionMessage message = { + .type = ExpansionMessageTypeIsConnected, + .data.is_connected = &is_connected, + .api_lock = api_lock_alloc_locked(), + }; + + furi_message_queue_put(instance->queue, &message, FuriWaitForever); + api_lock_wait_unlock_and_free(message.api_lock); + + return is_connected; +} + void expansion_set_listen_serial(Expansion* instance, FuriHalSerialId serial_id) { furi_check(instance); furi_check(serial_id < FuriHalSerialIdMax); diff --git a/applications/services/expansion/expansion.h b/applications/services/expansion/expansion.h index e169b3c15..1b0879b1e 100644 --- a/applications/services/expansion/expansion.h +++ b/applications/services/expansion/expansion.h @@ -50,6 +50,15 @@ void expansion_enable(Expansion* instance); */ void expansion_disable(Expansion* instance); +/** + * @brief Check if an expansion module is connected. + * + * @param[in,out] instance pointer to the Expansion instance. + * + * @returns true if the module is connected and initialized, false otherwise. + */ +bool expansion_is_connected(Expansion* instance); + /** * @brief Enable support for expansion modules on designated serial port. * diff --git a/applications/services/expansion/expansion_worker.c b/applications/services/expansion/expansion_worker.c index ac2a5935b..e6d17c152 100644 --- a/applications/services/expansion/expansion_worker.c +++ b/applications/services/expansion/expansion_worker.c @@ -35,7 +35,8 @@ typedef enum { ExpansionWorkerFlagError = 1 << 2, } ExpansionWorkerFlag; -#define EXPANSION_ALL_FLAGS (ExpansionWorkerFlagData | ExpansionWorkerFlagStop) +#define EXPANSION_ALL_FLAGS \ + (ExpansionWorkerFlagData | ExpansionWorkerFlagStop | ExpansionWorkerFlagError) struct ExpansionWorker { FuriThread* thread; @@ -225,6 +226,7 @@ static bool expansion_worker_handle_state_handshake( if(furi_hal_serial_is_baud_rate_supported(instance->serial_handle, baud_rate)) { instance->state = ExpansionWorkerStateConnected; + instance->callback(instance->cb_context, ExpansionWorkerCallbackReasonConnected); // Send response at previous baud rate if(!expansion_worker_send_status_response(instance, ExpansionFrameErrorNone)) break; furi_hal_serial_set_br(instance->serial_handle, baud_rate); @@ -360,6 +362,8 @@ static int32_t expansion_worker(void* context) { expansion_worker_state_machine(instance); } + furi_hal_serial_async_rx_stop(instance->serial_handle); + if(instance->state == ExpansionWorkerStateRpcActive) { expansion_worker_rpc_session_close(instance); } @@ -371,7 +375,7 @@ static int32_t expansion_worker(void* context) { // Do not invoke worker callback on user-requested exit if((instance->exit_reason != ExpansionWorkerExitReasonUser) && (instance->callback != NULL)) { - instance->callback(instance->cb_context); + instance->callback(instance->cb_context, ExpansionWorkerCallbackReasonExit); } return 0; diff --git a/applications/services/expansion/expansion_worker.h b/applications/services/expansion/expansion_worker.h index 761f79c1d..faab2887f 100644 --- a/applications/services/expansion/expansion_worker.h +++ b/applications/services/expansion/expansion_worker.h @@ -17,14 +17,20 @@ */ typedef struct ExpansionWorker ExpansionWorker; +typedef enum { + ExpansionWorkerCallbackReasonExit, + ExpansionWorkerCallbackReasonConnected, +} ExpansionWorkerCallbackReason; + /** * @brief Worker callback type. * * @see expansion_worker_set_callback() * * @param[in,out] context pointer to a user-defined object. + * @param[in] reason reason for the callback. */ -typedef void (*ExpansionWorkerCallback)(void* context); +typedef void (*ExpansionWorkerCallback)(void* context, ExpansionWorkerCallbackReason reason); /** * @brief Create an expansion worker instance. diff --git a/targets/f7/furi_hal/furi_hal_serial.c b/targets/f7/furi_hal/furi_hal_serial.c index 8ad9794a8..e5c30a278 100644 --- a/targets/f7/furi_hal/furi_hal_serial.c +++ b/targets/f7/furi_hal/furi_hal_serial.c @@ -817,6 +817,21 @@ static void furi_hal_serial_async_rx_configure( FuriHalSerialHandle* handle, FuriHalSerialAsyncRxCallback callback, void* context) { + // Disable RXFNE interrupts before unsetting the user callback that reads data + // Otherwise interrupt runs without reading data and without clearing RXFNE flag + // This would cause a system hang as the same interrupt runs in loop forever + if(!callback) { + if(handle->id == FuriHalSerialIdUsart) { + LL_USART_DisableIT_RXNE_RXFNE(USART1); + furi_hal_interrupt_set_isr(FuriHalInterruptIdUart1, NULL, NULL); + furi_hal_serial_usart_deinit_dma_rx(); + } else if(handle->id == FuriHalSerialIdLpuart) { + LL_LPUART_DisableIT_RXNE_RXFNE(LPUART1); + furi_hal_interrupt_set_isr(FuriHalInterruptIdLpUart1, NULL, NULL); + furi_hal_serial_lpuart_deinit_dma_rx(); + } + } + // Handle must be configured before enabling RX interrupt // as it might be triggered right away on a misconfigured handle furi_hal_serial[handle->id].rx_byte_callback = callback; @@ -824,27 +839,17 @@ static void furi_hal_serial_async_rx_configure( furi_hal_serial[handle->id].rx_dma_callback = NULL; furi_hal_serial[handle->id].context = context; - if(handle->id == FuriHalSerialIdUsart) { - if(callback) { + if(callback) { + if(handle->id == FuriHalSerialIdUsart) { furi_hal_serial_usart_deinit_dma_rx(); furi_hal_interrupt_set_isr( FuriHalInterruptIdUart1, furi_hal_serial_usart_irq_callback, NULL); LL_USART_EnableIT_RXNE_RXFNE(USART1); - } else { - furi_hal_interrupt_set_isr(FuriHalInterruptIdUart1, NULL, NULL); - furi_hal_serial_usart_deinit_dma_rx(); - LL_USART_DisableIT_RXNE_RXFNE(USART1); - } - } else if(handle->id == FuriHalSerialIdLpuart) { - if(callback) { + } else if(handle->id == FuriHalSerialIdLpuart) { furi_hal_serial_lpuart_deinit_dma_rx(); furi_hal_interrupt_set_isr( FuriHalInterruptIdLpUart1, furi_hal_serial_lpuart_irq_callback, NULL); LL_LPUART_EnableIT_RXNE_RXFNE(LPUART1); - } else { - furi_hal_interrupt_set_isr(FuriHalInterruptIdLpUart1, NULL, NULL); - furi_hal_serial_lpuart_deinit_dma_rx(); - LL_LPUART_DisableIT_RXNE_RXFNE(LPUART1); } } } @@ -944,33 +949,39 @@ static void furi_hal_serial_dma_configure( FuriHalSerialHandle* handle, FuriHalSerialDmaRxCallback callback, void* context) { - furi_check(handle); - - if(handle->id == FuriHalSerialIdUsart) { - if(callback) { - furi_hal_serial_usart_init_dma_rx(); - furi_hal_interrupt_set_isr( - FuriHalInterruptIdUart1, furi_hal_serial_usart_irq_callback, NULL); - } else { + // Disable RXFNE interrupts before unsetting the user callback that reads data + // Otherwise interrupt runs without reading data and without clearing RXFNE flag + // This would cause a system hang as the same interrupt runs in loop forever + if(!callback) { + if(handle->id == FuriHalSerialIdUsart) { LL_USART_DisableIT_RXNE_RXFNE(USART1); furi_hal_interrupt_set_isr(FuriHalInterruptIdUart1, NULL, NULL); furi_hal_serial_usart_deinit_dma_rx(); - } - } else if(handle->id == FuriHalSerialIdLpuart) { - if(callback) { - furi_hal_serial_lpuart_init_dma_rx(); - furi_hal_interrupt_set_isr( - FuriHalInterruptIdLpUart1, furi_hal_serial_lpuart_irq_callback, NULL); - } else { + } else if(handle->id == FuriHalSerialIdLpuart) { LL_LPUART_DisableIT_RXNE_RXFNE(LPUART1); furi_hal_interrupt_set_isr(FuriHalInterruptIdLpUart1, NULL, NULL); furi_hal_serial_lpuart_deinit_dma_rx(); } } + + // Handle must be configured before enabling RX interrupt + // as it might be triggered right away on a misconfigured handle furi_hal_serial[handle->id].rx_byte_callback = NULL; furi_hal_serial[handle->id].handle = handle; furi_hal_serial[handle->id].rx_dma_callback = callback; furi_hal_serial[handle->id].context = context; + + if(callback) { + if(handle->id == FuriHalSerialIdUsart) { + furi_hal_serial_usart_init_dma_rx(); + furi_hal_interrupt_set_isr( + FuriHalInterruptIdUart1, furi_hal_serial_usart_irq_callback, NULL); + } else if(handle->id == FuriHalSerialIdLpuart) { + furi_hal_serial_lpuart_init_dma_rx(); + furi_hal_interrupt_set_isr( + FuriHalInterruptIdLpUart1, furi_hal_serial_lpuart_irq_callback, NULL); + } + } } void furi_hal_serial_dma_rx_start(