From 940ec36a0b8e61b1a481e33980a675173e1aaae5 Mon Sep 17 00:00:00 2001 From: Skorpionm <85568270+Skorpionm@users.noreply.github.com> Date: Wed, 23 Aug 2023 23:51:32 +0400 Subject: [PATCH] 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 --- .../drivers/subghz/cc1101_ext/cc1101_ext.c | 3 +- .../helpers/subghz_txrx_create_protocol_key.c | 67 +++++++++---------- .../helpers/subghz_txrx_create_protocol_key.h | 20 +++--- .../scenes/subghz_scene_radio_setting.c | 34 +++++++--- .../subghz/scenes/subghz_scene_set_type.c | 8 +-- applications/main/subghz/subghz_i.c | 5 +- .../targets/f7/furi_hal/furi_hal_subghz.c | 3 +- lib/subghz/protocols/bin_raw.c | 2 - lib/subghz/protocols/keeloq.c | 9 ++- lib/subghz/protocols/secplus_v2.c | 1 - lib/subghz/subghz_tx_rx_worker.c | 4 +- lib/subghz/types.h | 1 + 12 files changed, 83 insertions(+), 74 deletions(-) diff --git a/applications/drivers/subghz/cc1101_ext/cc1101_ext.c b/applications/drivers/subghz/cc1101_ext/cc1101_ext.c index eb0ec56a..0a9599d0 100644 --- a/applications/drivers/subghz/cc1101_ext/cc1101_ext.c +++ b/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) { + if((status->NUM_RXBYTES > 0) || (status->RXFIFO_OVERFLOW == 0)) { return true; } else { return false; diff --git a/applications/main/subghz/helpers/subghz_txrx_create_protocol_key.c b/applications/main/subghz/helpers/subghz_txrx_create_protocol_key.c index b5d47cea..0413173e 100644 --- a/applications/main/subghz/helpers/subghz_txrx_create_protocol_key.c +++ b/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( - instance->decoder_result, instance->fff_data, instance->preset) != - SubGhzProtocolStatusOk) { + ret = subghz_protocol_decoder_base_serialize( + instance->decoder_result, instance->fff_data, instance->preset); + 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; - if(subghz_txrx_gen_data_protocol(instance, preset_name, frequency, protocol_name, key, bit)) { + SubGhzProtocolStatus ret = + 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, - name_preset, - frequency, - SUBGHZ_PROTOCOL_SECPLUS_V1_NAME, - (uint64_t)serial << 32 | 0xE6000000, - 42)) { - ret = true; - } - return ret; -} \ No newline at end of file + + return subghz_txrx_gen_data_protocol( + instance, + name_preset, + frequency, + SUBGHZ_PROTOCOL_SECPLUS_V1_NAME, + (uint64_t)serial << 32 | 0xE6000000, + 42); +} diff --git a/applications/main/subghz/helpers/subghz_txrx_create_protocol_key.h b/applications/main/subghz/helpers/subghz_txrx_create_protocol_key.h index 514a5733..edc6e541 100644 --- a/applications/main/subghz/helpers/subghz_txrx_create_protocol_key.h +++ b/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); \ No newline at end of file diff --git a/applications/main/subghz/scenes/subghz_scene_radio_setting.c b/applications/main/subghz/scenes/subghz_scene_radio_setting.c index de05417c..1f8e4d83 100644 --- a/applications/main/subghz/scenes/subghz_scene_radio_setting.c +++ b/applications/main/subghz/scenes/subghz_scene_radio_setting.c @@ -1,6 +1,7 @@ #include "../subghz_i.h" #include #include +#include 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); - - if(!subghz_txrx_radio_device_is_external_connected( - 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; - } + uint8_t current_index = variable_item_get_current_value_index(item); + uint8_t index = + subghz_scene_radio_settings_next_index_connect_ext_device(subghz, current_index); 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", diff --git a/applications/main/subghz/scenes/subghz_scene_set_type.c b/applications/main/subghz/scenes/subghz_scene_set_type.c index d0571f1b..8c040cc9 100644 --- a/applications/main/subghz/scenes/subghz_scene_set_type.c +++ b/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; diff --git a/applications/main/subghz/subghz_i.c b/applications/main/subghz/subghz_i.c index 55f33d20..9ff61837 100644 --- a/applications/main/subghz/subghz_i.c +++ b/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); diff --git a/firmware/targets/f7/furi_hal/furi_hal_subghz.c b/firmware/targets/f7/furi_hal/furi_hal_subghz.c index 327e42a8..bd724f0b 100644 --- a/firmware/targets/f7/furi_hal/furi_hal_subghz.c +++ b/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) { + if((status->NUM_RXBYTES > 0) || (status->RXFIFO_OVERFLOW == 0)) { return true; } else { return false; diff --git a/lib/subghz/protocols/bin_raw.c b/lib/subghz/protocols/bin_raw.c index c3dbf84f..9509280c 100644 --- a/lib/subghz/protocols/bin_raw.c +++ b/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 diff --git a/lib/subghz/protocols/keeloq.c b/lib/subghz/protocols/keeloq.c index 8789a0f2..cfd83887 100644 --- a/lib/subghz/protocols/keeloq.c +++ b/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)) { - //TODO FL-3560: if you need to add a callback to automatically update the data on the display - } else { + if(!subghz_protocol_keeloq_gen_data(instance, btn)) { return false; } diff --git a/lib/subghz/protocols/secplus_v2.c b/lib/subghz/protocols/secplus_v2.c index 48ee6698..12d2fac7 100644 --- a/lib/subghz/protocols/secplus_v2.c +++ b/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); diff --git a/lib/subghz/subghz_tx_rx_worker.c b/lib/subghz/subghz_tx_rx_worker.c index 833d90bf..daf046f4 100644 --- a/lib/subghz/subghz_tx_rx_worker.c +++ b/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"); } } } diff --git a/lib/subghz/types.h b/lib/subghz/types.h index d87a0dc7..cccf70ba 100644 --- a/lib/subghz/types.h +++ b/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;