diff --git a/applications/debug/expansion_test/expansion_test.c b/applications/debug/expansion_test/expansion_test.c index 0b4b0b27c..a0b8b42e8 100644 --- a/applications/debug/expansion_test/expansion_test.c +++ b/applications/debug/expansion_test/expansion_test.c @@ -100,17 +100,19 @@ static void expansion_test_app_start(ExpansionTestApp* instance) { // Configure the serial port furi_hal_serial_init(instance->handle, EXPANSION_PROTOCOL_DEFAULT_BAUD_RATE); // Start waiting for the initial pulse - expansion_enable(instance->expansion, HOST_SERIAL_ID); + expansion_set_listen_serial(instance->expansion, HOST_SERIAL_ID); furi_hal_serial_async_rx_start( instance->handle, expansion_test_app_serial_rx_callback, instance, false); } static void expansion_test_app_stop(ExpansionTestApp* instance) { + // Disable expansion module support + expansion_disable(instance->expansion); // Give back the module handle furi_hal_serial_control_release(instance->handle); - // Turn expansion module support off - expansion_disable(instance->expansion); + // Restore expansion user settings + expansion_enable(instance->expansion); furi_record_close(RECORD_EXPANSION); } diff --git a/applications/debug/unit_tests/expansion/expansion_test.c b/applications/debug/unit_tests/expansion/expansion_test.c index 0513da537..50fe1b9f4 100644 --- a/applications/debug/unit_tests/expansion/expansion_test.c +++ b/applications/debug/unit_tests/expansion/expansion_test.c @@ -1,8 +1,14 @@ #include "../minunit.h" #include +#include + #include +#define EXPANSION_TEST_GARBAGE_MAGIC (0xB19AF) +#define EXPANSION_TEST_GARBAGE_BUF_SIZE (0x100U) +#define EXPANSION_TEST_GARBAGE_ITERATIONS (100U) + MU_TEST(test_expansion_encoded_size) { ExpansionFrame frame = {}; @@ -28,43 +34,62 @@ MU_TEST(test_expansion_encoded_size) { MU_TEST(test_expansion_remaining_size) { ExpansionFrame frame = {}; - mu_assert_int_eq(1, expansion_frame_get_remaining_size(&frame, 0)); + size_t remaining_size; + mu_check(expansion_frame_get_remaining_size(&frame, 0, &remaining_size)); + mu_assert_int_eq(1, remaining_size); frame.header.type = ExpansionFrameTypeHeartbeat; - mu_assert_int_eq(1, expansion_frame_get_remaining_size(&frame, 0)); - mu_assert_int_eq(0, expansion_frame_get_remaining_size(&frame, 1)); - mu_assert_int_eq(0, expansion_frame_get_remaining_size(&frame, 100)); + mu_check(expansion_frame_get_remaining_size(&frame, 0, &remaining_size)); + mu_assert_int_eq(1, remaining_size); + mu_check(expansion_frame_get_remaining_size(&frame, 1, &remaining_size)); + mu_assert_int_eq(0, remaining_size); + mu_check(expansion_frame_get_remaining_size(&frame, 100, &remaining_size)); + mu_assert_int_eq(0, remaining_size); frame.header.type = ExpansionFrameTypeStatus; - mu_assert_int_eq(1, expansion_frame_get_remaining_size(&frame, 0)); - mu_assert_int_eq(1, expansion_frame_get_remaining_size(&frame, 1)); - mu_assert_int_eq(0, expansion_frame_get_remaining_size(&frame, 2)); - mu_assert_int_eq(0, expansion_frame_get_remaining_size(&frame, 100)); + mu_check(expansion_frame_get_remaining_size(&frame, 0, &remaining_size)); + mu_assert_int_eq(1, remaining_size); + mu_check(expansion_frame_get_remaining_size(&frame, 1, &remaining_size)); + mu_assert_int_eq(1, remaining_size); + mu_check(expansion_frame_get_remaining_size(&frame, 2, &remaining_size)); + mu_assert_int_eq(0, remaining_size); + mu_check(expansion_frame_get_remaining_size(&frame, 100, &remaining_size)); + mu_assert_int_eq(0, remaining_size); frame.header.type = ExpansionFrameTypeBaudRate; - mu_assert_int_eq(1, expansion_frame_get_remaining_size(&frame, 0)); - mu_assert_int_eq(4, expansion_frame_get_remaining_size(&frame, 1)); - mu_assert_int_eq(0, expansion_frame_get_remaining_size(&frame, 5)); - mu_assert_int_eq(0, expansion_frame_get_remaining_size(&frame, 100)); + mu_check(expansion_frame_get_remaining_size(&frame, 0, &remaining_size)); + mu_assert_int_eq(1, remaining_size); + mu_check(expansion_frame_get_remaining_size(&frame, 1, &remaining_size)); + mu_assert_int_eq(4, remaining_size); + mu_check(expansion_frame_get_remaining_size(&frame, 5, &remaining_size)); + mu_assert_int_eq(0, remaining_size); + mu_check(expansion_frame_get_remaining_size(&frame, 100, &remaining_size)); + mu_assert_int_eq(0, remaining_size); frame.header.type = ExpansionFrameTypeControl; - mu_assert_int_eq(1, expansion_frame_get_remaining_size(&frame, 0)); - mu_assert_int_eq(1, expansion_frame_get_remaining_size(&frame, 1)); - mu_assert_int_eq(0, expansion_frame_get_remaining_size(&frame, 2)); - mu_assert_int_eq(0, expansion_frame_get_remaining_size(&frame, 100)); + mu_check(expansion_frame_get_remaining_size(&frame, 0, &remaining_size)); + mu_assert_int_eq(1, remaining_size); + mu_check(expansion_frame_get_remaining_size(&frame, 1, &remaining_size)); + mu_assert_int_eq(1, remaining_size); + mu_check(expansion_frame_get_remaining_size(&frame, 2, &remaining_size)); + mu_assert_int_eq(0, remaining_size); + mu_check(expansion_frame_get_remaining_size(&frame, 100, &remaining_size)); + mu_assert_int_eq(0, remaining_size); frame.header.type = ExpansionFrameTypeData; frame.content.data.size = EXPANSION_PROTOCOL_MAX_DATA_SIZE; - mu_assert_int_eq(1, expansion_frame_get_remaining_size(&frame, 0)); - mu_assert_int_eq(1, expansion_frame_get_remaining_size(&frame, 1)); - mu_assert_int_eq( - EXPANSION_PROTOCOL_MAX_DATA_SIZE, expansion_frame_get_remaining_size(&frame, 2)); + mu_check(expansion_frame_get_remaining_size(&frame, 0, &remaining_size)); + mu_assert_int_eq(1, remaining_size); + mu_check(expansion_frame_get_remaining_size(&frame, 1, &remaining_size)); + mu_assert_int_eq(1, remaining_size); + mu_check(expansion_frame_get_remaining_size(&frame, 2, &remaining_size)); + mu_assert_int_eq(EXPANSION_PROTOCOL_MAX_DATA_SIZE, remaining_size); for(size_t i = 0; i <= EXPANSION_PROTOCOL_MAX_DATA_SIZE; ++i) { - mu_assert_int_eq( - EXPANSION_PROTOCOL_MAX_DATA_SIZE - i, - expansion_frame_get_remaining_size(&frame, i + 2)); + mu_check(expansion_frame_get_remaining_size(&frame, i + 2, &remaining_size)); + mu_assert_int_eq(EXPANSION_PROTOCOL_MAX_DATA_SIZE - i, remaining_size); } - mu_assert_int_eq(0, expansion_frame_get_remaining_size(&frame, 100)); + mu_check(expansion_frame_get_remaining_size(&frame, 100, &remaining_size)); + mu_assert_int_eq(0, remaining_size); } typedef struct { @@ -145,10 +170,28 @@ MU_TEST(test_expansion_encode_decode_frame) { mu_assert_mem_eq(&frame_in, &frame_out, encoded_size); } +MU_TEST(test_expansion_garbage_input) { + uint8_t garbage_data[EXPANSION_TEST_GARBAGE_BUF_SIZE]; + for(uint32_t i = 0; i < EXPANSION_TEST_GARBAGE_ITERATIONS; ++i) { + furi_hal_random_fill_buf(garbage_data, sizeof(garbage_data)); + size_t remaining_size = EXPANSION_TEST_GARBAGE_MAGIC; + if(expansion_frame_get_remaining_size( + (ExpansionFrame*)garbage_data, sizeof(garbage_data), &remaining_size)) { + // If by chance the garbage data is a valid frame, then the result + // must be 0 because the amount of data provided is more than enough + mu_assert_int_eq(0, remaining_size); + } else { + // If the frame is invalid, the remaining_size parameter should be untouched + mu_assert_int_eq(EXPANSION_TEST_GARBAGE_MAGIC, remaining_size); + } + } +} + MU_TEST_SUITE(test_expansion_suite) { MU_RUN_TEST(test_expansion_encoded_size); MU_RUN_TEST(test_expansion_remaining_size); MU_RUN_TEST(test_expansion_encode_decode_frame); + MU_RUN_TEST(test_expansion_garbage_input); } int run_minunit_test_expansion() { diff --git a/applications/main/gpio/gpio_app.c b/applications/main/gpio/gpio_app.c index 85c2ece84..06d377d39 100644 --- a/applications/main/gpio/gpio_app.c +++ b/applications/main/gpio/gpio_app.c @@ -24,6 +24,9 @@ static void gpio_app_tick_event_callback(void* context) { GpioApp* gpio_app_alloc() { GpioApp* app = malloc(sizeof(GpioApp)); + app->expansion = furi_record_open(RECORD_EXPANSION); + expansion_disable(app->expansion); + app->gui = furi_record_open(RECORD_GUI); app->gpio_items = gpio_items_alloc(); @@ -70,12 +73,7 @@ GpioApp* gpio_app_alloc() { GpioAppViewUsbUartCfg, variable_item_list_get_view(app->var_item_list)); - if(furi_hal_serial_control_is_busy(FuriHalSerialIdUsart) || - furi_hal_serial_control_is_busy(FuriHalSerialIdLpuart)) { - scene_manager_next_scene(app->scene_manager, GpioSceneErrorExpansion); - } else { - scene_manager_next_scene(app->scene_manager, GpioSceneStart); - } + scene_manager_next_scene(app->scene_manager, GpioSceneStart); return app; } @@ -104,6 +102,9 @@ void gpio_app_free(GpioApp* app) { furi_record_close(RECORD_GUI); furi_record_close(RECORD_NOTIFICATION); + expansion_enable(app->expansion); + furi_record_close(RECORD_EXPANSION); + gpio_items_free(app->gpio_items); free(app); } diff --git a/applications/main/gpio/gpio_app_i.h b/applications/main/gpio/gpio_app_i.h index d54ffd368..ce4cb6f55 100644 --- a/applications/main/gpio/gpio_app_i.h +++ b/applications/main/gpio/gpio_app_i.h @@ -17,8 +17,10 @@ #include "views/gpio_test.h" #include "views/gpio_usb_uart.h" #include +#include struct GpioApp { + Expansion* expansion; Gui* gui; NotificationApp* notifications; ViewDispatcher* view_dispatcher; diff --git a/applications/main/gpio/scenes/gpio_scene_config.h b/applications/main/gpio/scenes/gpio_scene_config.h index 3d3fb2f4e..d6fd24d19 100644 --- a/applications/main/gpio/scenes/gpio_scene_config.h +++ b/applications/main/gpio/scenes/gpio_scene_config.h @@ -3,5 +3,4 @@ ADD_SCENE(gpio, test, Test) ADD_SCENE(gpio, usb_uart, UsbUart) ADD_SCENE(gpio, usb_uart_cfg, UsbUartCfg) ADD_SCENE(gpio, usb_uart_close_rpc, UsbUartCloseRpc) -ADD_SCENE(gpio, error_expansion, ErrorExpansion) ADD_SCENE(gpio, exit_confirm, ExitConfirm) diff --git a/applications/main/gpio/scenes/gpio_scene_error_expansion.c b/applications/main/gpio/scenes/gpio_scene_error_expansion.c deleted file mode 100644 index 4f30f8b9d..000000000 --- a/applications/main/gpio/scenes/gpio_scene_error_expansion.c +++ /dev/null @@ -1,43 +0,0 @@ -#include "../gpio_app_i.h" -#include "../gpio_custom_event.h" - -void gpio_scene_error_expansion_on_enter(void* context) { - GpioApp* app = context; - - widget_add_icon_element(app->widget, 78, 0, &I_ActiveConnection_50x64); - widget_add_string_multiline_element( - app->widget, 3, 2, AlignLeft, AlignTop, FontPrimary, "Ext. Module\nis connected!"); - widget_add_string_multiline_element( - app->widget, - 3, - 30, - AlignLeft, - AlignTop, - FontSecondary, - "Disconnect External\n" - "Module\n" - "to use this function."); - - view_dispatcher_switch_to_view(app->view_dispatcher, GpioAppViewUsbUartCloseRpc); -} - -bool gpio_scene_error_expansion_on_event(void* context, SceneManagerEvent event) { - GpioApp* app = context; - bool consumed = false; - - if(event.type == SceneManagerEventTypeCustom) { - if(event.event == GpioCustomEventErrorBack) { - if(!scene_manager_previous_scene(app->scene_manager)) { - scene_manager_stop(app->scene_manager); - view_dispatcher_stop(app->view_dispatcher); - } - consumed = true; - } - } - return consumed; -} - -void gpio_scene_error_expansion_on_exit(void* context) { - GpioApp* app = context; - widget_reset(app->widget); -} diff --git a/applications/services/expansion/expansion.c b/applications/services/expansion/expansion.c index ca3b71444..e385734b7 100644 --- a/applications/services/expansion/expansion.c +++ b/applications/services/expansion/expansion.c @@ -394,30 +394,18 @@ void expansion_on_system_start(void* arg) { Expansion* instance = expansion_alloc(); furi_record_create(RECORD_EXPANSION, instance); - ExpansionSettings settings = {}; - if(!expansion_settings_load(&settings)) { - expansion_settings_save(&settings); - } else if(settings.uart_index < FuriHalSerialIdMax) { - expansion_enable(instance, settings.uart_index); - } + expansion_enable(instance); } // Public API functions -void expansion_enable(Expansion* instance, FuriHalSerialId serial_id) { - expansion_disable(instance); - - furi_check(furi_mutex_acquire(instance->state_mutex, FuriWaitForever) == FuriStatusOk); - - instance->serial_id = serial_id; - instance->state = ExpansionStateEnabled; - - furi_hal_serial_control_set_expansion_callback( - instance->serial_id, expansion_detect_callback, instance); - - furi_mutex_release(instance->state_mutex); - - FURI_LOG_D(TAG, "Detection enabled"); +void expansion_enable(Expansion* instance) { + ExpansionSettings settings = {}; + if(!expansion_settings_load(&settings)) { + expansion_settings_save(&settings); + } else if(settings.uart_index < FuriHalSerialIdMax) { + expansion_set_listen_serial(instance, settings.uart_index); + } } void expansion_disable(Expansion* instance) { @@ -435,3 +423,19 @@ void expansion_disable(Expansion* instance) { furi_mutex_release(instance->state_mutex); } + +void expansion_set_listen_serial(Expansion* instance, FuriHalSerialId serial_id) { + expansion_disable(instance); + + furi_check(furi_mutex_acquire(instance->state_mutex, FuriWaitForever) == FuriStatusOk); + + instance->serial_id = serial_id; + instance->state = ExpansionStateEnabled; + + furi_hal_serial_control_set_expansion_callback( + instance->serial_id, expansion_detect_callback, instance); + + furi_mutex_release(instance->state_mutex); + + FURI_LOG_D(TAG, "Detection enabled"); +} diff --git a/applications/services/expansion/expansion.h b/applications/services/expansion/expansion.h index 5e4a03f83..e169b3c15 100644 --- a/applications/services/expansion/expansion.h +++ b/applications/services/expansion/expansion.h @@ -21,18 +21,17 @@ extern "C" { typedef struct Expansion Expansion; /** - * @brief Enable support for expansion modules on designated serial port. + * @brief Enable support for expansion modules. * - * Only one serial port can be used to communicate with an expansion - * module at a time. + * Calling this function will load user settings and enable + * expansion module support on the serial port specified in said settings. * - * Calling this function when expansion module support is already enabled - * will first disable the previous setting, then enable the current one. + * If expansion module support was disabled in settings, this function + * does nothing. * * @param[in,out] instance pointer to the Expansion instance. - * @param[in] serial_id numerical identifier of the serial. */ -void expansion_enable(Expansion* instance, FuriHalSerialId serial_id); +void expansion_enable(Expansion* instance); /** * @brief Disable support for expansion modules. @@ -41,10 +40,34 @@ void expansion_enable(Expansion* instance, FuriHalSerialId serial_id); * expansion module (if any), release the serial handle and * reset the respective pins to the default state. * + * @note Applications requiring serial port access MUST call + * this function BEFORE calling furi_hal_serial_control_acquire(). + * Similarly, an expansion_enable() call MUST be made right AFTER + * a call to furi_hal_serial_control_release() to ensure that + * the user settings are properly restored. + * * @param[in,out] instance pointer to the Expansion instance. */ void expansion_disable(Expansion* instance); +/** + * @brief Enable support for expansion modules on designated serial port. + * + * Only one serial port can be used to communicate with an expansion + * module at a time. + * + * Calling this function when expansion module support is already enabled + * will first disable the previous setting, then enable the current one. + * + * @warning This function does not respect user settings for expansion modules, + * so calling it might leave the system in inconsistent state. Avoid using it + * unless absolutely necessary. + * + * @param[in,out] instance pointer to the Expansion instance. + * @param[in] serial_id numerical identifier of the serial. + */ +void expansion_set_listen_serial(Expansion* instance, FuriHalSerialId serial_id); + #ifdef __cplusplus } #endif diff --git a/applications/services/expansion/expansion_protocol.h b/applications/services/expansion/expansion_protocol.h index 37c56f15b..6ed818f82 100644 --- a/applications/services/expansion/expansion_protocol.h +++ b/applications/services/expansion/expansion_protocol.h @@ -193,11 +193,18 @@ static inline size_t expansion_frame_get_encoded_size(const ExpansionFrame* fram * * @param[in] frame pointer to the frame to be evaluated. * @param[in] received_size number of bytes currently availabe for evaluation. - * @returns number of bytes needed for a complete frame. + * @param[out] remaining_size pointer to the variable to contain the number of bytes needed for a complete frame. + * @returns true if the remaining size could be calculated, false on error. */ -static inline size_t - expansion_frame_get_remaining_size(const ExpansionFrame* frame, size_t received_size) { - if(received_size < sizeof(ExpansionFrameHeader)) return sizeof(ExpansionFrameHeader); +static inline bool expansion_frame_get_remaining_size( + const ExpansionFrame* frame, + size_t received_size, + size_t* remaining_size) { + if(received_size < sizeof(ExpansionFrameHeader)) { + // Frame type is unknown as of now + *remaining_size = sizeof(ExpansionFrameHeader); + return true; + } const size_t received_content_size = received_size - sizeof(ExpansionFrameHeader); size_t content_size; @@ -217,16 +224,26 @@ static inline size_t break; case ExpansionFrameTypeData: if(received_content_size < sizeof(frame->content.data.size)) { + // Data size is unknown as of now content_size = sizeof(frame->content.data.size); + } else if(frame->content.data.size > sizeof(frame->content.data.bytes)) { + // Malformed frame or garbage input + return false; } else { content_size = sizeof(frame->content.data.size) + frame->content.data.size; } break; default: - return SIZE_MAX; + return false; } - return content_size > received_content_size ? content_size - received_content_size : 0; + if(content_size > received_content_size) { + *remaining_size = content_size - received_content_size; + } else { + *remaining_size = 0; + } + + return true; } /** @@ -275,9 +292,7 @@ static inline ExpansionProtocolStatus expansion_protocol_decode( size_t remaining_size; while(true) { - remaining_size = expansion_frame_get_remaining_size(frame, total_size); - - if(remaining_size == SIZE_MAX) { + if(!expansion_frame_get_remaining_size(frame, total_size, &remaining_size)) { return ExpansionProtocolStatusErrorFormat; } else if(remaining_size == 0) { break; diff --git a/applications/settings/expansion_settings_app/expansion_settings_app.c b/applications/settings/expansion_settings_app/expansion_settings_app.c index 894015712..05e5f22e4 100644 --- a/applications/settings/expansion_settings_app/expansion_settings_app.c +++ b/applications/settings/expansion_settings_app/expansion_settings_app.c @@ -13,7 +13,7 @@ static void expansion_settings_app_uart_changed(VariableItem* item) { app->settings.uart_index = index; if(index < FuriHalSerialIdMax) { - expansion_enable(app->expansion, index); + expansion_set_listen_serial(app->expansion, index); } else { expansion_disable(app->expansion); } diff --git a/targets/f18/api_symbols.csv b/targets/f18/api_symbols.csv index 4a79c5553..ffb664a3e 100644 --- a/targets/f18/api_symbols.csv +++ b/targets/f18/api_symbols.csv @@ -1,5 +1,5 @@ entry,status,name,type,params -Version,+,53.0,, +Version,+,54.0,, Header,+,applications/services/bt/bt_service/bt.h,, Header,+,applications/services/cli/cli.h,, Header,+,applications/services/cli/cli_vcp.h,, @@ -790,7 +790,8 @@ Function,-,exp2,double,double Function,-,exp2f,float,float Function,-,exp2l,long double,long double Function,+,expansion_disable,void,Expansion* -Function,+,expansion_enable,void,"Expansion*, FuriHalSerialId" +Function,+,expansion_enable,void,Expansion* +Function,+,expansion_set_listen_serial,void,"Expansion*, FuriHalSerialId" Function,-,expf,float,float Function,-,expl,long double,long double Function,-,explicit_bzero,void,"void*, size_t" diff --git a/targets/f7/api_symbols.csv b/targets/f7/api_symbols.csv index 075d1049a..f852a69be 100644 --- a/targets/f7/api_symbols.csv +++ b/targets/f7/api_symbols.csv @@ -1,5 +1,5 @@ entry,status,name,type,params -Version,+,53.0,, +Version,+,54.0,, Header,+,applications/drivers/subghz/cc1101_ext/cc1101_ext_interconnect.h,, Header,+,applications/services/bt/bt_service/bt.h,, Header,+,applications/services/cli/cli.h,, @@ -879,7 +879,8 @@ Function,-,exp2,double,double Function,-,exp2f,float,float Function,-,exp2l,long double,long double Function,+,expansion_disable,void,Expansion* -Function,+,expansion_enable,void,"Expansion*, FuriHalSerialId" +Function,+,expansion_enable,void,Expansion* +Function,+,expansion_set_listen_serial,void,"Expansion*, FuriHalSerialId" Function,-,expf,float,float Function,-,expl,long double,long double Function,-,explicit_bzero,void,"void*, size_t"