[FL-2955], [FL-2953] SubGhz: fix RAW "Send never ends" (#1979)

* SubGhz: fix RAW "Send never ends"
* SubGhz: delete comments
* SubGhz: RAW file parsing speed increase
* SubGhz: fix level_duration_is_wait
* SubGhz: modification furi_hal_subghz_async_tx_refill
* SubGhz: furi_hal_subghz_stop_async_rx
* SubGhz: hal unit test and better async tx yield handling
* FuriHal: proper async tx end in subghz, vibro on power off
* FuriHal: variable naming in subghz
* SubGhz,FuriHal: extreme timings in subghz hal unit tests, remove memset in async tx buffer fill routine
* FuriHal: small refinements in subghz

Co-authored-by: あく <alleteam@gmail.com>

applications/debug/unit_tests/subghz/subghz_test.c

@@ -209,6 +209,149 @@ MU_TEST(subghz_keystore_test) {
         "Test keystore error");
 }
 
+typedef enum {
+    SubGhzHalAsyncTxTestTypeNormal,
+    SubGhzHalAsyncTxTestTypeInvalidStart,
+    SubGhzHalAsyncTxTestTypeInvalidMid,
+    SubGhzHalAsyncTxTestTypeInvalidEnd,
+    SubGhzHalAsyncTxTestTypeResetStart,
+    SubGhzHalAsyncTxTestTypeResetMid,
+    SubGhzHalAsyncTxTestTypeResetEnd,
+} SubGhzHalAsyncTxTestType;
+
+typedef struct {
+    SubGhzHalAsyncTxTestType type;
+    size_t pos;
+} SubGhzHalAsyncTxTest;
+
+#define SUBGHZ_HAL_TEST_DURATION 1
+
+static LevelDuration subghz_hal_async_tx_test_yield(void* context) {
+    SubGhzHalAsyncTxTest* test = context;
+    bool is_odd = test->pos % 2;
+
+    if(test->type == SubGhzHalAsyncTxTestTypeNormal) {
+        if(test->pos < API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL * 8) {
+            test->pos++;
+            return level_duration_make(is_odd, SUBGHZ_HAL_TEST_DURATION);
+        } else if(test->pos == API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL * 8) {
+            test->pos++;
+            return level_duration_reset();
+        } else {
+            furi_crash("Yield after reset");
+        }
+    } else if(test->type == SubGhzHalAsyncTxTestTypeInvalidStart) {
+        if(test->pos == 0) {
+            test->pos++;
+            return level_duration_make(!is_odd, SUBGHZ_HAL_TEST_DURATION);
+        } else if(test->pos < API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL * 8) {
+            test->pos++;
+            return level_duration_make(is_odd, SUBGHZ_HAL_TEST_DURATION);
+        } else if(test->pos == API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL * 8) {
+            test->pos++;
+            return level_duration_reset();
+        } else {
+            furi_crash("Yield after reset");
+        }
+    } else if(test->type == SubGhzHalAsyncTxTestTypeInvalidMid) {
+        if(test->pos == API_HAL_SUBGHZ_ASYNC_TX_BUFFER_HALF / 2) {
+            test->pos++;
+            return level_duration_make(!is_odd, SUBGHZ_HAL_TEST_DURATION);
+        } else if(test->pos < API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL * 8) {
+            test->pos++;
+            return level_duration_make(is_odd, SUBGHZ_HAL_TEST_DURATION);
+        } else if(test->pos == API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL * 8) {
+            test->pos++;
+            return level_duration_reset();
+        } else {
+            furi_crash("Yield after reset");
+        }
+    } else if(test->type == SubGhzHalAsyncTxTestTypeInvalidEnd) {
+        if(test->pos == API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL - 1) {
+            test->pos++;
+            return level_duration_make(!is_odd, SUBGHZ_HAL_TEST_DURATION);
+        } else if(test->pos < API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL * 8) {
+            test->pos++;
+            return level_duration_make(is_odd, SUBGHZ_HAL_TEST_DURATION);
+        } else if(test->pos == API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL * 8) {
+            test->pos++;
+            return level_duration_reset();
+        } else {
+            furi_crash("Yield after reset");
+        }
+    } else if(test->type == SubGhzHalAsyncTxTestTypeResetStart) {
+        if(test->pos == 0) {
+            test->pos++;
+            return level_duration_reset();
+        } else {
+            furi_crash("Yield after reset");
+        }
+    } else if(test->type == SubGhzHalAsyncTxTestTypeResetMid) {
+        if(test->pos < API_HAL_SUBGHZ_ASYNC_TX_BUFFER_HALF / 2) {
+            test->pos++;
+            return level_duration_make(is_odd, SUBGHZ_HAL_TEST_DURATION);
+        } else if(test->pos == API_HAL_SUBGHZ_ASYNC_TX_BUFFER_HALF / 2) {
+            test->pos++;
+            return level_duration_reset();
+        } else {
+            furi_crash("Yield after reset");
+        }
+    } else if(test->type == SubGhzHalAsyncTxTestTypeResetEnd) {
+        if(test->pos < API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL - 1) {
+            test->pos++;
+            return level_duration_make(is_odd, SUBGHZ_HAL_TEST_DURATION);
+        } else if(test->pos == API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL - 1) {
+            test->pos++;
+            return level_duration_reset();
+        } else {
+            furi_crash("Yield after reset");
+        }
+    } else {
+        furi_crash("Programming error");
+    }
+}
+
+bool subghz_hal_async_tx_test_run(SubGhzHalAsyncTxTestType type) {
+    SubGhzHalAsyncTxTest test = {0};
+    test.type = type;
+    furi_hal_subghz_reset();
+    furi_hal_subghz_load_preset(FuriHalSubGhzPresetOok650Async);
+    furi_hal_subghz_set_frequency_and_path(433920000);
+
+    furi_hal_subghz_start_async_tx(subghz_hal_async_tx_test_yield, &test);
+    while(!furi_hal_subghz_is_async_tx_complete()) {
+        furi_delay_ms(10);
+    }
+    furi_hal_subghz_stop_async_tx();
+    furi_hal_subghz_sleep();
+
+    return true;
+}
+
+MU_TEST(subghz_hal_async_tx_test) {
+    mu_assert(
+        subghz_hal_async_tx_test_run(SubGhzHalAsyncTxTestTypeNormal),
+        "Test furi_hal_async_tx normal");
+    mu_assert(
+        subghz_hal_async_tx_test_run(SubGhzHalAsyncTxTestTypeInvalidStart),
+        "Test furi_hal_async_tx invalid start");
+    mu_assert(
+        subghz_hal_async_tx_test_run(SubGhzHalAsyncTxTestTypeInvalidMid),
+        "Test furi_hal_async_tx invalid mid");
+    mu_assert(
+        subghz_hal_async_tx_test_run(SubGhzHalAsyncTxTestTypeInvalidEnd),
+        "Test furi_hal_async_tx invalid end");
+    mu_assert(
+        subghz_hal_async_tx_test_run(SubGhzHalAsyncTxTestTypeResetStart),
+        "Test furi_hal_async_tx reset start");
+    mu_assert(
+        subghz_hal_async_tx_test_run(SubGhzHalAsyncTxTestTypeResetMid),
+        "Test furi_hal_async_tx reset mid");
+    mu_assert(
+        subghz_hal_async_tx_test_run(SubGhzHalAsyncTxTestTypeResetEnd),
+        "Test furi_hal_async_tx reset end");
+}
+
 //test decoders
 MU_TEST(subghz_decoder_came_atomo_test) {
     mu_assert(
@@ -579,6 +722,8 @@ MU_TEST_SUITE(subghz) {
     subghz_test_init();
     MU_RUN_TEST(subghz_keystore_test);
 
+    MU_RUN_TEST(subghz_hal_async_tx_test);
+
     MU_RUN_TEST(subghz_decoder_came_atomo_test);
     MU_RUN_TEST(subghz_decoder_came_test);
     MU_RUN_TEST(subghz_decoder_came_twee_test);

firmware/targets/f7/furi_hal/furi_hal_power.c

@@ -1,6 +1,7 @@
 #include <furi_hal_power.h>
 #include <furi_hal_clock.h>
 #include <furi_hal_bt.h>
+#include <furi_hal_vibro.h>
 #include <furi_hal_resources.h>
 #include <furi_hal_uart.h>
 
@@ -308,11 +309,13 @@ void furi_hal_power_shutdown() {
 void furi_hal_power_off() {
     // Crutch: shutting down with ext 3V3 off is causing LSE to stop
     furi_hal_power_enable_external_3_3v();
-    furi_delay_us(1000);
+    furi_hal_vibro_on(true);
+    furi_delay_us(50000);
     // Send poweroff to charger
     furi_hal_i2c_acquire(&furi_hal_i2c_handle_power);
     bq25896_poweroff(&furi_hal_i2c_handle_power);
     furi_hal_i2c_release(&furi_hal_i2c_handle_power);
+    furi_hal_vibro_on(false);
 }
 
 void furi_hal_power_reset() {

firmware/targets/f7/furi_hal/furi_hal_subghz.c

@@ -488,13 +488,9 @@ void furi_hal_subghz_stop_async_rx() {
     furi_hal_gpio_init(&gpio_cc1101_g0, GpioModeAnalog, GpioPullNo, GpioSpeedLow);
 }
 
-#define API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL (256)
-#define API_HAL_SUBGHZ_ASYNC_TX_BUFFER_HALF (API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL / 2)
-#define API_HAL_SUBGHZ_ASYNC_TX_GUARD_TIME 333
-
 typedef struct {
     uint32_t* buffer;
-    bool flip_flop;
+    LevelDuration carry_ld;
     FuriHalSubGhzAsyncTxCallback callback;
     void* callback_context;
     uint64_t duty_high;
@@ -504,37 +500,48 @@ typedef struct {
 static FuriHalSubGhzAsyncTx furi_hal_subghz_async_tx = {0};
 
 static void furi_hal_subghz_async_tx_refill(uint32_t* buffer, size_t samples) {
+    furi_assert(furi_hal_subghz.state == SubGhzStateAsyncTx);
     while(samples > 0) {
         bool is_odd = samples % 2;
-        LevelDuration ld =
+        LevelDuration ld;
-            furi_hal_subghz_async_tx.callback(furi_hal_subghz_async_tx.callback_context);
+        if(level_duration_is_reset(furi_hal_subghz_async_tx.carry_ld)) {
+            ld = furi_hal_subghz_async_tx.callback(furi_hal_subghz_async_tx.callback_context);
+        } else {
+            ld = furi_hal_subghz_async_tx.carry_ld;
+            furi_hal_subghz_async_tx.carry_ld = level_duration_reset();
+        }
 
         if(level_duration_is_wait(ld)) {
-            return;
-        } else if(level_duration_is_reset(ld)) {
-            // One more even sample required to end at low level
-            if(is_odd) {
             *buffer = API_HAL_SUBGHZ_ASYNC_TX_GUARD_TIME;
             buffer++;
             samples--;
-                furi_hal_subghz_async_tx.duty_low += API_HAL_SUBGHZ_ASYNC_TX_GUARD_TIME;
+        } else if(level_duration_is_reset(ld)) {
-            }
+            *buffer = 0;
+            buffer++;
+            samples--;
+            LL_DMA_DisableIT_HT(DMA1, LL_DMA_CHANNEL_1);
+            LL_DMA_DisableIT_TC(DMA1, LL_DMA_CHANNEL_1);
+            LL_TIM_EnableIT_UPDATE(TIM2);
             break;
         } else {
-            // Inject guard time if level is incorrect
             bool level = level_duration_get_level(ld);
-            if(is_odd == level) {
+
+            // Inject guard time if level is incorrect
+            if(is_odd != level) {
                 *buffer = API_HAL_SUBGHZ_ASYNC_TX_GUARD_TIME;
                 buffer++;
                 samples--;
-                if(!level) {
+                if(is_odd) {
                     furi_hal_subghz_async_tx.duty_high += API_HAL_SUBGHZ_ASYNC_TX_GUARD_TIME;
                 } else {
                     furi_hal_subghz_async_tx.duty_low += API_HAL_SUBGHZ_ASYNC_TX_GUARD_TIME;
                 }
-                // This code must be invoked only once: when encoder starts with low level.
+
-                // Otherwise whole thing will crash.
+                // Special case: prevent buffer overflow if sample is last
-                furi_check(samples > 0);
+                if(samples == 0) {
+                    furi_hal_subghz_async_tx.carry_ld = ld;
+                    break;
+                }
             }
 
             uint32_t duration = level_duration_get_duration(ld);
@@ -543,22 +550,17 @@ static void furi_hal_subghz_async_tx_refill(uint32_t* buffer, size_t samples) {
             buffer++;
             samples--;
 
-            if(level) {
+            if(is_odd) {
                 furi_hal_subghz_async_tx.duty_high += duration;
             } else {
                 furi_hal_subghz_async_tx.duty_low += duration;
             }
         }
     }
-
-    memset(buffer, 0, samples * sizeof(uint32_t));
 }
 
 static void furi_hal_subghz_async_tx_dma_isr() {
-    furi_assert(
+    furi_assert(furi_hal_subghz.state == SubGhzStateAsyncTx);
-        furi_hal_subghz.state == SubGhzStateAsyncTx ||
-        furi_hal_subghz.state == SubGhzStateAsyncTxEnd ||
-        furi_hal_subghz.state == SubGhzStateAsyncTxLast);
     if(LL_DMA_IsActiveFlag_HT1(DMA1)) {
         LL_DMA_ClearFlag_HT1(DMA1);
         furi_hal_subghz_async_tx_refill(
@@ -578,11 +580,14 @@ static void furi_hal_subghz_async_tx_timer_isr() {
         if(LL_TIM_GetAutoReload(TIM2) == 0) {
             if(furi_hal_subghz.state == SubGhzStateAsyncTx) {
                 furi_hal_subghz.state = SubGhzStateAsyncTxLast;
+                LL_DMA_DisableChannel(DMA1, LL_DMA_CHANNEL_1);
+            } else if(furi_hal_subghz.state == SubGhzStateAsyncTxLast) {
+                furi_hal_subghz.state = SubGhzStateAsyncTxEnd;
                 //forcibly pulls the pin to the ground so that there is no carrier
                 furi_hal_gpio_init(&gpio_cc1101_g0, GpioModeInput, GpioPullDown, GpioSpeedLow);
-            } else {
-                furi_hal_subghz.state = SubGhzStateAsyncTxEnd;
                 LL_TIM_DisableCounter(TIM2);
+            } else {
+                furi_crash(NULL);
             }
         }
     }
@@ -605,8 +610,6 @@ bool furi_hal_subghz_start_async_tx(FuriHalSubGhzAsyncTxCallback callback, void*
 
     furi_hal_subghz_async_tx.buffer =
         malloc(API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL * sizeof(uint32_t));
-    furi_hal_subghz_async_tx_refill(
-        furi_hal_subghz_async_tx.buffer, API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL);
 
     // Connect CC1101_GD0 to TIM2 as output
     furi_hal_gpio_init_ex(
@@ -647,14 +650,16 @@ bool furi_hal_subghz_start_async_tx(FuriHalSubGhzAsyncTxCallback callback, void*
     TIM_OC_InitStruct.OCState = LL_TIM_OCSTATE_DISABLE;
     TIM_OC_InitStruct.OCNState = LL_TIM_OCSTATE_DISABLE;
     TIM_OC_InitStruct.CompareValue = 0;
-    TIM_OC_InitStruct.OCPolarity = LL_TIM_OCPOLARITY_HIGH;
+    TIM_OC_InitStruct.OCPolarity = LL_TIM_OCPOLARITY_LOW;
     LL_TIM_OC_Init(TIM2, LL_TIM_CHANNEL_CH2, &TIM_OC_InitStruct);
     LL_TIM_OC_DisableFast(TIM2, LL_TIM_CHANNEL_CH2);
     LL_TIM_DisableMasterSlaveMode(TIM2);
 
     furi_hal_interrupt_set_isr(FuriHalInterruptIdTIM2, furi_hal_subghz_async_tx_timer_isr, NULL);
 
-    LL_TIM_EnableIT_UPDATE(TIM2);
+    furi_hal_subghz_async_tx_refill(
+        furi_hal_subghz_async_tx.buffer, API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL);
+
     LL_TIM_EnableDMAReq_UPDATE(TIM2);
     LL_TIM_CC_EnableChannel(TIM2, LL_TIM_CHANNEL_CH2);
 
@@ -673,8 +678,8 @@ bool furi_hal_subghz_start_async_tx(FuriHalSubGhzAsyncTxCallback callback, void*
         &SUBGHZ_DEBUG_CC1101_PIN, GpioModeOutputPushPull, GpioPullNo, GpioSpeedVeryHigh);
 
     const GpioPin* gpio = &SUBGHZ_DEBUG_CC1101_PIN;
-    subghz_debug_gpio_buff[0] = gpio->pin;
+    subghz_debug_gpio_buff[0] = (uint32_t)gpio->pin << GPIO_NUMBER;
-    subghz_debug_gpio_buff[1] = (uint32_t)gpio->pin << GPIO_NUMBER;
+    subghz_debug_gpio_buff[1] = gpio->pin;
 
     dma_config.MemoryOrM2MDstAddress = (uint32_t)subghz_debug_gpio_buff;
     dma_config.PeriphOrM2MSrcAddress = (uint32_t) & (gpio->port->BSRR);

firmware/targets/furi_hal_include/furi_hal_subghz.h

@@ -14,6 +14,11 @@
 extern "C" {
 #endif
 
+/** Low level buffer dimensions and guard times */
+#define API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL (256)
+#define API_HAL_SUBGHZ_ASYNC_TX_BUFFER_HALF (API_HAL_SUBGHZ_ASYNC_TX_BUFFER_FULL / 2)
+#define API_HAL_SUBGHZ_ASYNC_TX_GUARD_TIME 999
+
 /** Radio Presets */
 typedef enum {
     FuriHalSubGhzPresetIDLE, /**< default configuration */

lib/subghz/subghz_file_encoder_worker.c

@@ -18,6 +18,7 @@ struct SubGhzFileEncoderWorker {
     volatile bool worker_running;
     volatile bool worker_stoping;
     bool level;
+    bool is_storage_slow;
     FuriString* str_data;
     FuriString* file_path;
 
@@ -86,7 +87,7 @@ LevelDuration subghz_file_encoder_worker_get_level_duration(void* context) {
     if(ret == sizeof(int32_t)) {
         LevelDuration level_duration = {.level = LEVEL_DURATION_RESET};
         if(duration < 0) {
-            level_duration = level_duration_make(false, duration * -1);
+            level_duration = level_duration_make(false, -duration);
         } else if(duration > 0) {
             level_duration = level_duration_make(true, duration);
         } else if(duration == 0) {
@@ -96,7 +97,7 @@ LevelDuration subghz_file_encoder_worker_get_level_duration(void* context) {
         }
         return level_duration;
     } else {
-        FURI_LOG_E(TAG, "Slow flash read");
+        instance->is_storage_slow = true;
         return level_duration_wait();
     }
 }
@@ -110,6 +111,7 @@ static int32_t subghz_file_encoder_worker_thread(void* context) {
     SubGhzFileEncoderWorker* instance = context;
     FURI_LOG_I(TAG, "Worker start");
     bool res = false;
+    instance->is_storage_slow = false;
     Stream* stream = flipper_format_get_raw_stream(instance->flipper_format);
     do {
         if(!flipper_format_file_open_existing(
@@ -139,21 +141,21 @@ static int32_t subghz_file_encoder_worker_thread(void* context) {
                 furi_string_trim(instance->str_data);
                 if(!subghz_file_encoder_worker_data_parse(
                        instance, furi_string_get_cstr(instance->str_data))) {
-                    //to stop DMA correctly
                     subghz_file_encoder_worker_add_level_duration(instance, LEVEL_DURATION_RESET);
-                    subghz_file_encoder_worker_add_level_duration(instance, LEVEL_DURATION_RESET);
-
                     break;
                 }
             } else {
-                subghz_file_encoder_worker_add_level_duration(instance, LEVEL_DURATION_RESET);
                 subghz_file_encoder_worker_add_level_duration(instance, LEVEL_DURATION_RESET);
                 break;
             }
+        } else {
+            furi_delay_ms(1);
         }
-        furi_delay_ms(5);
     }
     //waiting for the end of the transfer
+    if(instance->is_storage_slow) {
+        FURI_LOG_E(TAG, "Storage is slow");
+    }
     FURI_LOG_I(TAG, "End read file");
     while(!furi_hal_subghz_is_async_tx_complete() && instance->worker_running) {
         furi_delay_ms(5);