SubGhz: fix todo (#2984)

* [FL-3501] SubGhz: fix Handle multiple external cc1101 modules
* [FL-3502] SubGhz: fix Protocol not found error message
* [FL-3503] SubGhz: fix Handle rx buffer overflow
* {FL-3520] SubGhz: Handle RX buffer overflow with external cc1101
* [FL-3548] SubGhz: Security+ 2.0 counter start value
* [FL-3552] Sub-GHz: Check saved file
* [FL-3555] [FL-3554] Sub-GHz: RX buffer overflow handling and check that buffer has been properly written
* [FL-3557] Sub-GHz: No optimization required
* [FL-3558] Sub-GHz: Keeloq 0 discriminator
* [FL-3559] Sub-GHz: Keeloq unknown learning
* [FL-3560] Sub-GHz: callback for updating keeloq data on display
* SubGhz: fix RXFIFO_OVERFLOW

Co-authored-by: Aleksandr Kutuzov <alleteam@gmail.com>

applications/drivers/subghz/cc1101_ext/cc1101_ext.c

@@ -333,8 +333,7 @@ bool subghz_device_cc1101_ext_rx_pipe_not_empty() {
         (CC1101_STATUS_RXBYTES) | CC1101_BURST,
         (uint8_t*)status);
     furi_hal_spi_release(subghz_device_cc1101_ext->spi_bus_handle);
-    // TODO FL-3520: you can add a buffer overflow flag if needed
+    if((status->NUM_RXBYTES > 0) || (status->RXFIFO_OVERFLOW == 0)) {
-    if(status->NUM_RXBYTES > 0) {
         return true;
     } else {
         return false;

applications/main/subghz/helpers/subghz_txrx_create_protocol_key.c

@@ -13,7 +13,7 @@
 
 #define TAG "SubGhzCreateProtocolKey"
 
-bool subghz_txrx_gen_data_protocol(
+SubGhzProtocolStatus subghz_txrx_gen_data_protocol(
     void* context,
     const char* preset_name,
     uint32_t frequency,
@@ -23,30 +23,29 @@ bool subghz_txrx_gen_data_protocol(
     furi_assert(context);
     SubGhzTxRx* instance = context;
 
-    bool res = false;
+    SubGhzProtocolStatus ret = SubGhzProtocolStatusOk;
 
     subghz_txrx_set_preset(instance, preset_name, frequency, NULL, 0);
     instance->decoder_result =
         subghz_receiver_search_decoder_base_by_name(instance->receiver, protocol_name);
 
     if(instance->decoder_result == NULL) {
-        //TODO FL-3502: Error
-        // furi_string_set(error_str, "Protocol not\nfound!");
-        // scene_manager_next_scene(scene_manager, SubGhzSceneShowErrorSub);
         FURI_LOG_E(TAG, "Protocol not found!");
-        return false;
+        ret = SubGhzProtocolStatusErrorProtocolNotFound;
+        return ret;
     }
 
     do {
         Stream* fff_data_stream = flipper_format_get_raw_stream(instance->fff_data);
         stream_clean(fff_data_stream);
-        if(subghz_protocol_decoder_base_serialize(
+        ret = subghz_protocol_decoder_base_serialize(
-               instance->decoder_result, instance->fff_data, instance->preset) !=
+            instance->decoder_result, instance->fff_data, instance->preset);
-           SubGhzProtocolStatusOk) {
+        if(ret != SubGhzProtocolStatusOk) {
             FURI_LOG_E(TAG, "Unable to serialize");
             break;
         }
         if(!flipper_format_update_uint32(instance->fff_data, "Bit", &bit, 1)) {
+            ret = SubGhzProtocolStatusErrorParserOthers;
             FURI_LOG_E(TAG, "Unable to update Bit");
             break;
         }
@@ -56,15 +55,15 @@ bool subghz_txrx_gen_data_protocol(
             key_data[sizeof(uint64_t) - i - 1] = (key >> (i * 8)) & 0xFF;
         }
         if(!flipper_format_update_hex(instance->fff_data, "Key", key_data, sizeof(uint64_t))) {
+            ret = SubGhzProtocolStatusErrorParserOthers;
             FURI_LOG_E(TAG, "Unable to update Key");
             break;
         }
-        res = true;
     } while(false);
-    return res;
+    return ret;
 }
 
-bool subghz_txrx_gen_data_protocol_and_te(
+SubGhzProtocolStatus subghz_txrx_gen_data_protocol_and_te(
     SubGhzTxRx* instance,
     const char* preset_name,
     uint32_t frequency,
@@ -73,18 +72,18 @@ bool subghz_txrx_gen_data_protocol_and_te(
     uint32_t bit,
     uint32_t te) {
     furi_assert(instance);
-    bool ret = false;
+    SubGhzProtocolStatus ret =
-    if(subghz_txrx_gen_data_protocol(instance, preset_name, frequency, protocol_name, key, bit)) {
+        subghz_txrx_gen_data_protocol(instance, preset_name, frequency, protocol_name, key, bit);
+    if(ret == SubGhzProtocolStatusOk) {
         if(!flipper_format_update_uint32(instance->fff_data, "TE", (uint32_t*)&te, 1)) {
+            ret = SubGhzProtocolStatusErrorParserOthers;
             FURI_LOG_E(TAG, "Unable to update Te");
-        } else {
-            ret = true;
         }
     }
     return ret;
 }
 
-bool subghz_txrx_gen_keeloq_protocol(
+SubGhzProtocolStatus subghz_txrx_gen_keeloq_protocol(
     SubGhzTxRx* instance,
     const char* name_preset,
     uint32_t frequency,
@@ -94,7 +93,7 @@ bool subghz_txrx_gen_keeloq_protocol(
     uint16_t cnt) {
     furi_assert(instance);
 
-    bool ret = false;
+    SubGhzProtocolStatus ret = SubGhzProtocolStatusError;
     serial &= 0x0FFFFFFF;
     instance->transmitter =
         subghz_transmitter_alloc_init(instance->environment, SUBGHZ_PROTOCOL_KEELOQ_NAME);
@@ -108,13 +107,13 @@ bool subghz_txrx_gen_keeloq_protocol(
             cnt,
             name_sysmem,
             instance->preset);
-        ret = true;
+        ret = SubGhzProtocolStatusOk;
     }
     subghz_transmitter_free(instance->transmitter);
     return ret;
 }
 
-bool subghz_txrx_gen_secplus_v2_protocol(
+SubGhzProtocolStatus subghz_txrx_gen_secplus_v2_protocol(
     SubGhzTxRx* instance,
     const char* name_preset,
     uint32_t frequency,
@@ -123,10 +122,11 @@ bool subghz_txrx_gen_secplus_v2_protocol(
     uint32_t cnt) {
     furi_assert(instance);
 
-    bool ret = false;
+    SubGhzProtocolStatus ret = SubGhzProtocolStatusError;
     instance->transmitter =
         subghz_transmitter_alloc_init(instance->environment, SUBGHZ_PROTOCOL_SECPLUS_V2_NAME);
     subghz_txrx_set_preset(instance, name_preset, frequency, NULL, 0);
+
     if(instance->transmitter) {
         subghz_protocol_secplus_v2_create_data(
             subghz_transmitter_get_protocol_instance(instance->transmitter),
@@ -135,30 +135,27 @@ bool subghz_txrx_gen_secplus_v2_protocol(
             btn,
             cnt,
             instance->preset);
-        ret = true;
+        ret = SubGhzProtocolStatusOk;
     }
     return ret;
 }
 
-bool subghz_txrx_gen_secplus_v1_protocol(
+SubGhzProtocolStatus subghz_txrx_gen_secplus_v1_protocol(
     SubGhzTxRx* instance,
     const char* name_preset,
     uint32_t frequency) {
     furi_assert(instance);
 
-    bool ret = false;
     uint32_t serial = (uint32_t)rand();
     while(!subghz_protocol_secplus_v1_check_fixed(serial)) {
         serial = (uint32_t)rand();
     }
-    if(subghz_txrx_gen_data_protocol(
+
-           instance,
+    return subghz_txrx_gen_data_protocol(
-           name_preset,
+        instance,
-           frequency,
+        name_preset,
-           SUBGHZ_PROTOCOL_SECPLUS_V1_NAME,
+        frequency,
-           (uint64_t)serial << 32 | 0xE6000000,
+        SUBGHZ_PROTOCOL_SECPLUS_V1_NAME,
-           42)) {
+        (uint64_t)serial << 32 | 0xE6000000,
-        ret = true;
+        42);
-    }
+}
-    return ret;
-}

applications/main/subghz/helpers/subghz_txrx_create_protocol_key.h

@@ -11,9 +11,9 @@
  * @param protocol_name Name of protocol
  * @param key Key
  * @param bit Bit
- * @return bool True if success
+ * @return SubGhzProtocolStatus
  */
-bool subghz_txrx_gen_data_protocol(
+SubGhzProtocolStatus subghz_txrx_gen_data_protocol(
     void* context,
     const char* preset_name,
     uint32_t frequency,
@@ -31,9 +31,9 @@ bool subghz_txrx_gen_data_protocol(
  * @param key Key
  * @param bit Bit
  * @param te Te
- * @return bool True if success
+ * @return SubGhzProtocolStatus
  */
-bool subghz_txrx_gen_data_protocol_and_te(
+SubGhzProtocolStatus subghz_txrx_gen_data_protocol_and_te(
     SubGhzTxRx* instance,
     const char* preset_name,
     uint32_t frequency,
@@ -52,9 +52,9 @@ bool subghz_txrx_gen_data_protocol_and_te(
  * @param serial Serial number
  * @param btn Button
  * @param cnt Counter
- * @return bool True if success
+ * @return SubGhzProtocolStatus
  */
-bool subghz_txrx_gen_keeloq_protocol(
+SubGhzProtocolStatus subghz_txrx_gen_keeloq_protocol(
     SubGhzTxRx* instance,
     const char* name_preset,
     uint32_t frequency,
@@ -72,9 +72,9 @@ bool subghz_txrx_gen_keeloq_protocol(
  * @param serial Serial number
  * @param btn Button
  * @param cnt Counter
- * @return bool True if success
+ * @return SubGhzProtocolStatus
  */
-bool subghz_txrx_gen_secplus_v2_protocol(
+SubGhzProtocolStatus subghz_txrx_gen_secplus_v2_protocol(
     SubGhzTxRx* instance,
     const char* name_preset,
     uint32_t frequency,
@@ -88,9 +88,9 @@ bool subghz_txrx_gen_secplus_v2_protocol(
  * @param instance Pointer to a SubGhzTxRx
  * @param name_preset Name of preset
  * @param frequency Frequency in Hz
- * @return bool True if success
+ * @return SubGhzProtocolStatus
  */
-bool subghz_txrx_gen_secplus_v1_protocol(
+SubGhzProtocolStatus subghz_txrx_gen_secplus_v1_protocol(
     SubGhzTxRx* instance,
     const char* name_preset,
     uint32_t frequency);

applications/main/subghz/scenes/subghz_scene_radio_setting.c

@@ -1,6 +1,7 @@
 #include "../subghz_i.h"
 #include <lib/toolbox/value_index.h>
 #include <applications/drivers/subghz/cc1101_ext/cc1101_ext_interconnect.h>
+#include <lib/subghz/devices/cc1101_int/cc1101_int_interconnect.h>
 
 enum SubGhzRadioSettingIndex {
     SubGhzRadioSettingIndexDevice,
@@ -17,16 +18,29 @@ const uint32_t radio_device_value[RADIO_DEVICE_COUNT] = {
     SubGhzRadioDeviceTypeExternalCC1101,
 };
 
+const char* const radio_device_name[RADIO_DEVICE_COUNT] = {
+    SUBGHZ_DEVICE_CC1101_INT_NAME,
+    SUBGHZ_DEVICE_CC1101_EXT_NAME,
+};
+
+static uint8_t subghz_scene_radio_settings_next_index_connect_ext_device(
+    SubGhz* subghz,
+    uint8_t current_index) {
+    uint8_t index = 0;
+    for(index = current_index; index < RADIO_DEVICE_COUNT; index++) {
+        if(subghz_txrx_radio_device_is_external_connected(subghz->txrx, radio_device_name[index])) {
+            break;
+        }
+    }
+    if(index == RADIO_DEVICE_COUNT) index = 0;
+    return index;
+}
+
 static void subghz_scene_radio_settings_set_device(VariableItem* item) {
     SubGhz* subghz = variable_item_get_context(item);
-    uint8_t index = variable_item_get_current_value_index(item);
+    uint8_t current_index = variable_item_get_current_value_index(item);
-
+    uint8_t index =
-    if(!subghz_txrx_radio_device_is_external_connected(
+        subghz_scene_radio_settings_next_index_connect_ext_device(subghz, current_index);
-           subghz->txrx, SUBGHZ_DEVICE_CC1101_EXT_NAME) &&
-       radio_device_value[index] == SubGhzRadioDeviceTypeExternalCC1101) {
-        // TODO FL-3501: correct if there is more than 1 module
-        index = 0;
-    }
     variable_item_set_current_value_text(item, radio_device_text[index]);
     subghz_txrx_radio_device_set(subghz->txrx, radio_device_value[index]);
 }
@@ -37,9 +51,11 @@ void subghz_scene_radio_settings_on_enter(void* context) {
     uint8_t value_index;
 
     uint8_t value_count_device = RADIO_DEVICE_COUNT;
+
     if(subghz_txrx_radio_device_get(subghz->txrx) == SubGhzRadioDeviceTypeInternal &&
-       !subghz_txrx_radio_device_is_external_connected(subghz->txrx, SUBGHZ_DEVICE_CC1101_EXT_NAME))
+       !subghz_scene_radio_settings_next_index_connect_ext_device(subghz, 1))
         value_count_device = 1;
+
     item = variable_item_list_add(
         subghz->variable_item_list,
         "Module",

applications/main/subghz/scenes/subghz_scene_set_type.c

@@ -118,7 +118,7 @@ void subghz_scene_set_type_on_enter(void* context) {
 
 bool subghz_scene_set_type_on_event(void* context, SceneManagerEvent event) {
     SubGhz* subghz = context;
-    bool generated_protocol = false;
+    SubGhzProtocolStatus generated_protocol = SubGhzProtocolStatusError;
 
     if(event.type == SceneManagerEventTypeCustom) {
         uint32_t key = (uint32_t)rand();
@@ -174,7 +174,7 @@ bool subghz_scene_set_type_on_event(void* context, SceneManagerEvent event) {
         case SubmenuIndexDoorHan_433_92:
             generated_protocol = subghz_txrx_gen_keeloq_protocol(
                 subghz->txrx, "AM650", 433920000, "DoorHan", key, 0x2, 0x0003);
-            if(!generated_protocol) {
+            if(generated_protocol != SubGhzProtocolStatusOk) {
                 furi_string_set(
                     subghz->error_str, "Function requires\nan SD card with\nfresh databases.");
                 scene_manager_next_scene(subghz->scene_manager, SubGhzSceneShowError);
@@ -183,7 +183,7 @@ bool subghz_scene_set_type_on_event(void* context, SceneManagerEvent event) {
         case SubmenuIndexDoorHan_315_00:
             generated_protocol = subghz_txrx_gen_keeloq_protocol(
                 subghz->txrx, "AM650", 315000000, "DoorHan", key, 0x2, 0x0003);
-            if(!generated_protocol) {
+            if(generated_protocol != SubGhzProtocolStatusOk) {
                 furi_string_set(
                     subghz->error_str, "Function requires\nan SD card with\nfresh databases.");
                 scene_manager_next_scene(subghz->scene_manager, SubGhzSceneShowError);
@@ -216,7 +216,7 @@ bool subghz_scene_set_type_on_event(void* context, SceneManagerEvent event) {
 
         scene_manager_set_scene_state(subghz->scene_manager, SubGhzSceneSetType, event.event);
 
-        if(generated_protocol) {
+        if(generated_protocol == SubGhzProtocolStatusOk) {
             subghz_file_name_clear(subghz);
             scene_manager_next_scene(subghz->scene_manager, SubGhzSceneSaveName);
             return true;

applications/main/subghz/subghz_i.c

@@ -289,10 +289,13 @@ bool subghz_save_protocol_to_file(
         if(!storage_simply_remove(storage, dev_file_name)) {
             break;
         }
-        //TODO FL-3552: check Write
         stream_seek(flipper_format_stream, 0, StreamOffsetFromStart);
         stream_save_to_file(flipper_format_stream, storage, dev_file_name, FSOM_CREATE_ALWAYS);
 
+        if(storage_common_stat(storage, dev_file_name, NULL) != FSE_OK) {
+            break;
+        }
+
         saved = true;
     } while(0);
     furi_string_free(file_dir);

firmware/targets/f7/furi_hal/furi_hal_subghz.c

@@ -207,8 +207,7 @@ bool furi_hal_subghz_rx_pipe_not_empty() {
     cc1101_read_reg(
         &furi_hal_spi_bus_handle_subghz, (CC1101_STATUS_RXBYTES) | CC1101_BURST, (uint8_t*)status);
     furi_hal_spi_release(&furi_hal_spi_bus_handle_subghz);
-    // TODO FL-3503: you can add a buffer overflow flag if needed
+    if((status->NUM_RXBYTES > 0) || (status->RXFIFO_OVERFLOW == 0)) {
-    if(status->NUM_RXBYTES > 0) {
         return true;
     } else {
         return false;

lib/subghz/protocols/bin_raw.c

@@ -744,7 +744,6 @@ static bool
 
                             bin_raw_debug("\r\n\r\n");
 #endif
-                            //TODO FL-3557: can be optimized
                             BinRAW_Markup markup_temp[BIN_RAW_MAX_MARKUP_COUNT];
                             memcpy(
                                 markup_temp,
@@ -770,7 +769,6 @@ static bool
                 }
             }
         }
-        //TODO FL-3557: can be optimized
         if(bin_raw_type == BinRAWTypeGap) {
             if(data_temp != 0) { //there are sequences with the same number of bits
 

lib/subghz/protocols/keeloq.c

@@ -122,7 +122,7 @@ static bool subghz_protocol_keeloq_gen_data(SubGhzProtocolEncoderKeeloq* instanc
     uint32_t fix = (uint32_t)btn << 28 | instance->generic.serial;
     uint32_t decrypt = (uint32_t)btn << 28 |
                        (instance->generic.serial & 0x3FF)
-                           << 16 | //TODO FL-3558: in some protocols the discriminator is 0
+                           << 16 | // In some protocols the discriminator is 0
                        instance->generic.cnt;
     uint32_t hop = 0;
     uint64_t man = 0;
@@ -149,7 +149,8 @@ static bool subghz_protocol_keeloq_gen_data(SubGhzProtocolEncoderKeeloq* instanc
                     hop = subghz_protocol_keeloq_common_encrypt(decrypt, man);
                     break;
                 case KEELOQ_LEARNING_UNKNOWN:
-                    hop = 0; //TODO FL-3559
+                    //Invalid or missing encoding type in keeloq_mfcodes
+                    hop = 0;
                     break;
                 }
                 break;
@@ -199,9 +200,7 @@ static bool
     furi_assert(instance);
 
     //gen new key
-    if(subghz_protocol_keeloq_gen_data(instance, btn)) {
+    if(!subghz_protocol_keeloq_gen_data(instance, btn)) {
-        //TODO FL-3560: if you need to add a callback to automatically update the data on the display
-    } else {
         return false;
     }
 

lib/subghz/protocols/secplus_v2.c

@@ -380,7 +380,6 @@ static void subghz_protocol_secplus_v2_encode(SubGhzProtocolEncoderSecPlus_v2* i
     uint8_t roll_2[9] = {0};
 
     instance->generic.cnt++;
-    //TODO Fl-3548: it is not known what value the counter starts
     if(instance->generic.cnt > 0xFFFFFFF) instance->generic.cnt = 0xE500000;
     uint32_t rolling = subghz_protocol_blocks_reverse_key(instance->generic.cnt, 28);
 

lib/subghz/subghz_tx_rx_worker.c

@@ -165,7 +165,6 @@ static int32_t subghz_tx_rx_worker_thread(void* context) {
                     SUBGHZ_TXRX_WORKER_TIMEOUT_READ_WRITE_BUF);
                 subghz_tx_rx_worker_tx(instance, data, SUBGHZ_TXRX_WORKER_MAX_TXRX_SIZE);
             } else {
-                //TODO FL-3554: checking that it managed to write all the data to the TX buffer
                 furi_stream_buffer_receive(
                     instance->stream_tx, &data, size_tx, SUBGHZ_TXRX_WORKER_TIMEOUT_READ_WRITE_BUF);
                 subghz_tx_rx_worker_tx(instance, data, size_tx);
@@ -178,7 +177,6 @@ static int32_t subghz_tx_rx_worker_thread(void* context) {
                        furi_stream_buffer_bytes_available(instance->stream_rx) == 0) {
                         callback_rx = true;
                     }
-                    //TODO FL-3554: checking that it managed to write all the data to the RX buffer
                     furi_stream_buffer_send(
                         instance->stream_rx,
                         &data,
@@ -189,7 +187,7 @@ static int32_t subghz_tx_rx_worker_thread(void* context) {
                         callback_rx = false;
                     }
                 } else {
-                    //TODO FL-3555: RX buffer overflow
+                    FURI_LOG_E(TAG, "Receive buffer overflow, over-the-air transmission too fast");
                 }
             }
         }

lib/subghz/types.h

@@ -57,6 +57,7 @@ typedef enum {
     // Encoder issue
     SubGhzProtocolStatusErrorEncoderGetUpload = (-12), ///< Payload encoder failure
     // Special Values
+    SubGhzProtocolStatusErrorProtocolNotFound = (-13), ///< Protocol not found
     SubGhzProtocolStatusReserved = 0x7FFFFFFF, ///< Prevents enum down-size compiler optimization.
 } SubGhzProtocolStatus;