Merge branch 'ofw_dev' into dev

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

applications/debug/unit_tests/expansion/expansion_test.c

@@ -1,8 +1,14 @@
 #include "../minunit.h"
 
 #include <furi.h>
+#include <furi_hal_random.h>
+
 #include <expansion/expansion_protocol.h>
 
+#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() {

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

applications/main/gpio/gpio_app_i.h

@@ -17,8 +17,10 @@
 #include "views/gpio_test.h"
 #include "views/gpio_usb_uart.h"
 #include <assets_icons.h>
+#include <expansion/expansion.h>
 
 struct GpioApp {
+    Expansion* expansion;
     Gui* gui;
     NotificationApp* notifications;
     ViewDispatcher* view_dispatcher;

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)

applications/services/expansion/expansion.c

@@ -394,29 +394,17 @@ void expansion_on_system_start(void* arg) {
     Expansion* instance = expansion_alloc();
     furi_record_create(RECORD_EXPANSION, instance);
 
-    ExpansionSettings settings = {};
-    expansion_settings_load(&settings);
-    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 = {};
+    expansion_settings_load(&settings);
+    if(settings.uart_index < FuriHalSerialIdMax) {
+        expansion_set_listen_serial(instance, settings.uart_index);
+    }
 }
 
 void expansion_disable(Expansion* instance) {
@@ -434,3 +422,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");
+}

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