From 7c692a9f3600b3aad50901327a0e0480944accdb Mon Sep 17 00:00:00 2001 From: gornekich Date: Thu, 21 Apr 2022 18:36:53 +0300 Subject: [PATCH] [FL-2432], [FL-2487] Rework bt keys load and save (#1139) * bt keys: rework load and save with saved_struct * bt: rename bt keys storage functions * furi_hal_nfc: allow context switch during emilation * bt settings: rework with saved struct * infrared: replace file worker with dialogs and storage * Core, Loader: fix thread allocation tracking, much better, so wow. Co-authored-by: Aleksandr Kutuzov --- applications/bt/bt_service/bt.c | 6 +- applications/bt/bt_service/bt_keys_storage.c | 53 +++++++------- applications/bt/bt_service/bt_keys_storage.h | 6 +- applications/bt/bt_settings.c | 46 +++---------- applications/bt/bt_settings.h | 3 - applications/infrared/infrared_app.cpp | 6 ++ applications/infrared/infrared_app.h | 6 ++ .../infrared/infrared_app_brute_force.cpp | 1 - .../infrared/infrared_app_remote_manager.cpp | 69 +++++++++++-------- .../infrared_app_scene_learn_success.cpp | 8 +-- .../scene/infrared_app_scene_remote_list.cpp | 6 +- applications/loader/loader.c | 19 +---- applications/loader/loader_i.h | 1 - core/furi/memmgr_heap.c | 11 ++- core/furi/thread.c | 1 + firmware/targets/f7/furi_hal/furi_hal_nfc.c | 4 +- 16 files changed, 112 insertions(+), 134 deletions(-) diff --git a/applications/bt/bt_service/bt.c b/applications/bt/bt_service/bt.c index 720f4854..8ce7f9ce 100755 --- a/applications/bt/bt_service/bt.c +++ b/applications/bt/bt_service/bt.c @@ -320,7 +320,7 @@ int32_t bt_srv() { Bt* bt = bt_alloc(); // Read keys - if(!bt_load_key_storage(bt)) { + if(!bt_keys_storage_load(bt)) { FURI_LOG_W(TAG, "Failed to load bonding keys"); } @@ -365,11 +365,11 @@ int32_t bt_srv() { // Display PIN code bt_pin_code_show(bt, message.data.pin_code); } else if(message.type == BtMessageTypeKeysStorageUpdated) { - bt_save_key_storage(bt); + bt_keys_storage_save(bt); } else if(message.type == BtMessageTypeSetProfile) { bt_change_profile(bt, &message); } else if(message.type == BtMessageTypeForgetBondedDevices) { - bt_delete_key_storage(bt); + bt_keys_storage_delete(bt); } } return 0; diff --git a/applications/bt/bt_service/bt_keys_storage.c b/applications/bt/bt_service/bt_keys_storage.c index 74df1020..e4f426c8 100644 --- a/applications/bt/bt_service/bt_keys_storage.c +++ b/applications/bt/bt_service/bt_keys_storage.c @@ -1,46 +1,47 @@ #include "bt_keys_storage.h" + #include -#include +#include -#define BT_KEYS_STORAGE_TAG "bt keys storage" #define BT_KEYS_STORAGE_PATH "/int/bt.keys" +#define BT_KEYS_STORAGE_VERSION (0) +#define BT_KEYS_STORAGE_MAGIC (0x18) -bool bt_load_key_storage(Bt* bt) { +bool bt_keys_storage_load(Bt* bt) { furi_assert(bt); - bool file_loaded = false; - furi_hal_bt_get_key_storage_buff(&bt->bt_keys_addr_start, &bt->bt_keys_size); - FileWorker* file_worker = file_worker_alloc(true); - if(file_worker_open(file_worker, BT_KEYS_STORAGE_PATH, FSAM_READ, FSOM_OPEN_EXISTING)) { - furi_hal_bt_nvm_sram_sem_acquire(); - if(file_worker_read(file_worker, bt->bt_keys_addr_start, bt->bt_keys_size)) { - file_loaded = true; - } - furi_hal_bt_nvm_sram_sem_release(); - } - file_worker_free(file_worker); + furi_hal_bt_get_key_storage_buff(&bt->bt_keys_addr_start, &bt->bt_keys_size); + furi_hal_bt_nvm_sram_sem_acquire(); + file_loaded = saved_struct_load( + BT_KEYS_STORAGE_PATH, + bt->bt_keys_addr_start, + bt->bt_keys_size, + BT_KEYS_STORAGE_MAGIC, + BT_KEYS_STORAGE_VERSION); + furi_hal_bt_nvm_sram_sem_release(); + return file_loaded; } -bool bt_save_key_storage(Bt* bt) { +bool bt_keys_storage_save(Bt* bt) { furi_assert(bt); furi_assert(bt->bt_keys_addr_start); - bool file_saved = false; - FileWorker* file_worker = file_worker_alloc(true); - if(file_worker_open(file_worker, BT_KEYS_STORAGE_PATH, FSAM_WRITE, FSOM_OPEN_ALWAYS)) { - furi_hal_bt_nvm_sram_sem_acquire(); - if(file_worker_write(file_worker, bt->bt_keys_addr_start, bt->bt_keys_size)) { - file_saved = true; - } - furi_hal_bt_nvm_sram_sem_release(); - } - file_worker_free(file_worker); + + furi_hal_bt_nvm_sram_sem_acquire(); + file_saved = saved_struct_save( + BT_KEYS_STORAGE_PATH, + bt->bt_keys_addr_start, + bt->bt_keys_size, + BT_KEYS_STORAGE_MAGIC, + BT_KEYS_STORAGE_VERSION); + furi_hal_bt_nvm_sram_sem_release(); + return file_saved; } -bool bt_delete_key_storage(Bt* bt) { +bool bt_keys_storage_delete(Bt* bt) { furi_assert(bt); bool delete_succeed = false; bool bt_is_active = furi_hal_bt_is_active(); diff --git a/applications/bt/bt_service/bt_keys_storage.h b/applications/bt/bt_service/bt_keys_storage.h index e763e6a7..68d4476a 100644 --- a/applications/bt/bt_service/bt_keys_storage.h +++ b/applications/bt/bt_service/bt_keys_storage.h @@ -2,8 +2,8 @@ #include "bt_i.h" -bool bt_load_key_storage(Bt* bt); +bool bt_keys_storage_load(Bt* bt); -bool bt_save_key_storage(Bt* bt); +bool bt_keys_storage_save(Bt* bt); -bool bt_delete_key_storage(Bt* bt); +bool bt_keys_storage_delete(Bt* bt); diff --git a/applications/bt/bt_settings.c b/applications/bt/bt_settings.c index 7b52321c..dbb2fd05 100644 --- a/applications/bt/bt_settings.c +++ b/applications/bt/bt_settings.c @@ -1,50 +1,22 @@ #include "bt_settings.h" -#include -#include -#define TAG "BtSettings" +#include +#include + #define BT_SETTINGS_PATH "/int/bt.settings" +#define BT_SETTINGS_VERSION (0) +#define BT_SETTINGS_MAGIC (0x19) bool bt_settings_load(BtSettings* bt_settings) { furi_assert(bt_settings); - bool file_loaded = false; - BtSettings settings = {}; - FURI_LOG_I(TAG, "Loading settings from \"%s\"", BT_SETTINGS_PATH); - FileWorker* file_worker = file_worker_alloc(true); - if(file_worker_open(file_worker, BT_SETTINGS_PATH, FSAM_READ, FSOM_OPEN_EXISTING)) { - if(file_worker_read(file_worker, &settings, sizeof(settings))) { - file_loaded = true; - } - } - file_worker_free(file_worker); - - if(file_loaded) { - FURI_LOG_I(TAG, "Settings load success"); - if(settings.version != BT_SETTINGS_VERSION) { - FURI_LOG_E(TAG, "Settings version mismatch"); - } else { - osKernelLock(); - *bt_settings = settings; - osKernelUnlock(); - } - } else { - FURI_LOG_E(TAG, "Settings load failed"); - } - return file_loaded; + return saved_struct_load( + BT_SETTINGS_PATH, bt_settings, sizeof(BtSettings), BT_SETTINGS_MAGIC, BT_SETTINGS_VERSION); } bool bt_settings_save(BtSettings* bt_settings) { furi_assert(bt_settings); - bool result = false; - FileWorker* file_worker = file_worker_alloc(true); - if(file_worker_open(file_worker, BT_SETTINGS_PATH, FSAM_WRITE, FSOM_OPEN_ALWAYS)) { - if(file_worker_write(file_worker, bt_settings, sizeof(BtSettings))) { - FURI_LOG_I(TAG, "Settings saved to \"%s\"", BT_SETTINGS_PATH); - result = true; - } - } - file_worker_free(file_worker); - return result; + return saved_struct_save( + BT_SETTINGS_PATH, bt_settings, sizeof(BtSettings), BT_SETTINGS_MAGIC, BT_SETTINGS_VERSION); } diff --git a/applications/bt/bt_settings.h b/applications/bt/bt_settings.h index d0a539ea..1a98668a 100644 --- a/applications/bt/bt_settings.h +++ b/applications/bt/bt_settings.h @@ -3,10 +3,7 @@ #include #include -#define BT_SETTINGS_VERSION (0) - typedef struct { - uint8_t version; bool enabled; } BtSettings; diff --git a/applications/infrared/infrared_app.cpp b/applications/infrared/infrared_app.cpp index 35d814dc..64bdb2b2 100644 --- a/applications/infrared/infrared_app.cpp +++ b/applications/infrared/infrared_app.cpp @@ -49,12 +49,14 @@ int32_t InfraredApp::run(void* args) { InfraredApp::InfraredApp() { furi_check(InfraredAppRemoteManager::max_button_name_length < get_text_store_size()); notification = static_cast(furi_record_open("notification")); + dialogs = static_cast(furi_record_open("dialogs")); infrared_worker = infrared_worker_alloc(); } InfraredApp::~InfraredApp() { infrared_worker_free(infrared_worker); furi_record_close("notification"); + furi_record_close("dialogs"); for(auto& [key, scene] : scenes) delete scene; } @@ -248,6 +250,10 @@ void InfraredApp::notify_blink_green() { notification_message(notification, &sequence); } +DialogsApp* InfraredApp::get_dialogs() { + return dialogs; +} + void InfraredApp::notify_green_on() { notification_message(notification, &sequence_set_only_green_255); } diff --git a/applications/infrared/infrared_app.h b/applications/infrared/infrared_app.h index 39897272..f1580402 100644 --- a/applications/infrared/infrared_app.h +++ b/applications/infrared/infrared_app.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include "scene/infrared_app_scene.h" @@ -228,6 +229,9 @@ public: /** Blink green light */ void notify_blink_green(); + /** Get Dialogs instance */ + DialogsApp* get_dialogs(); + /** Text input callback * * @param context - context to pass to callback @@ -286,6 +290,8 @@ private: /** Notification instance */ NotificationApp* notification; + /** Dialogs instance */ + DialogsApp* dialogs; /** View manager instance */ InfraredAppViewManager view_manager; /** Remote manager instance */ diff --git a/applications/infrared/infrared_app_brute_force.cpp b/applications/infrared/infrared_app_brute_force.cpp index 24be4d3e..2910a399 100644 --- a/applications/infrared/infrared_app_brute_force.cpp +++ b/applications/infrared/infrared_app_brute_force.cpp @@ -5,7 +5,6 @@ #include #include #include -#include void InfraredAppBruteForce::add_record(int index, const char* name) { records[name].index = index; diff --git a/applications/infrared/infrared_app_remote_manager.cpp b/applications/infrared/infrared_app_remote_manager.cpp index 9fbef84f..4fd64553 100644 --- a/applications/infrared/infrared_app_remote_manager.cpp +++ b/applications/infrared/infrared_app_remote_manager.cpp @@ -1,4 +1,3 @@ -#include #include #include "infrared_app_remote_manager.h" #include "infrared/helpers/infrared_parser.h" @@ -22,26 +21,34 @@ std::string InfraredAppRemoteManager::make_full_name( } std::string InfraredAppRemoteManager::find_vacant_remote_name(const std::string& name) { - bool exist = true; - FileWorkerCpp file_worker; + std::string result_name; + Storage* storage = static_cast(furi_record_open("storage")); - if(!file_worker.is_file_exist( - make_full_name(InfraredApp::infrared_directory, name).c_str(), &exist)) { - return std::string(); - } else if(!exist) { - return name; + FS_Error error = storage_common_stat( + storage, make_full_name(InfraredApp::infrared_directory, name).c_str(), NULL); + + if(error == FSE_NOT_EXIST) { + result_name = name; + } else if(error != FSE_OK) { + result_name = std::string(); + } else { + /* if suggested name is occupied, try another one (name2, name3, etc) */ + uint32_t i = 1; + std::string new_name; + do { + new_name = make_full_name(InfraredApp::infrared_directory, name + std::to_string(++i)); + error = storage_common_stat(storage, new_name.c_str(), NULL); + } while(error == FSE_OK); + + if(error == FSE_NOT_EXIST) { + result_name = name + std::to_string(i); + } else { + result_name = std::string(); + } } - /* if suggested name is occupied, try another one (name2, name3, etc) */ - uint32_t i = 1; - bool file_worker_result = false; - std::string new_name; - do { - new_name = make_full_name(InfraredApp::infrared_directory, name + std::to_string(++i)); - file_worker_result = file_worker.is_file_exist(new_name.c_str(), &exist); - } while(file_worker_result && exist); - - return !exist ? name + std::to_string(i) : std::string(); + furi_record_close("storage"); + return result_name; } bool InfraredAppRemoteManager::add_button(const char* button_name, const InfraredAppSignal& signal) { @@ -84,12 +91,14 @@ const InfraredAppSignal& InfraredAppRemoteManager::get_button_data(size_t index) } bool InfraredAppRemoteManager::delete_remote() { - bool result; - FileWorkerCpp file_worker; - result = file_worker.remove(make_full_name(remote->path, remote->name).c_str()); + Storage* storage = static_cast(furi_record_open("storage")); + FS_Error error = + storage_common_remove(storage, make_full_name(remote->path, remote->name).c_str()); reset_remote(); - return result; + + furi_record_close("storage"); + return (error == FSE_OK || error == FSE_NOT_EXIST); } void InfraredAppRemoteManager::reset_remote() { @@ -129,14 +138,15 @@ bool InfraredAppRemoteManager::rename_remote(const char* str) { return false; } - FileWorkerCpp file_worker; + Storage* storage = static_cast(furi_record_open("storage")); + std::string old_filename = make_full_name(remote->path, remote->name); std::string new_filename = make_full_name(remote->path, new_name); - bool result = file_worker.rename(old_filename.c_str(), new_filename.c_str()); - + FS_Error error = storage_common_rename(storage, old_filename.c_str(), new_filename.c_str()); remote->name = new_name; - return result; + furi_record_close("storage"); + return (error == FSE_OK || error == FSE_EXIST); } bool InfraredAppRemoteManager::rename_button(uint32_t index, const char* str) { @@ -155,11 +165,10 @@ size_t InfraredAppRemoteManager::get_number_of_buttons() { bool InfraredAppRemoteManager::store(void) { bool result = false; - FileWorkerCpp file_worker; - - if(!file_worker.mkdir(InfraredApp::infrared_directory)) return false; - Storage* storage = static_cast(furi_record_open("storage")); + + if(!storage_simply_mkdir(storage, InfraredApp::infrared_directory)) return false; + FlipperFormat* ff = flipper_format_file_alloc(storage); FURI_LOG_I( diff --git a/applications/infrared/scene/infrared_app_scene_learn_success.cpp b/applications/infrared/scene/infrared_app_scene_learn_success.cpp index 61d60ab5..b620a5ed 100644 --- a/applications/infrared/scene/infrared_app_scene_learn_success.cpp +++ b/applications/infrared/scene/infrared_app_scene_learn_success.cpp @@ -1,5 +1,4 @@ #include -#include #include #include @@ -88,13 +87,8 @@ bool InfraredAppSceneLearnSuccess::on_event(InfraredApp* app, InfraredAppEvent* break; case DialogExResultRight: { consumed = true; - FileWorkerCpp file_worker; if(!button_pressed) { - if(file_worker.check_errors()) { - app->switch_to_next_scene(InfraredApp::Scene::LearnEnterName); - } else { - app->switch_to_previous_scene(); - } + app->switch_to_next_scene(InfraredApp::Scene::LearnEnterName); } break; } diff --git a/applications/infrared/scene/infrared_app_scene_remote_list.cpp b/applications/infrared/scene/infrared_app_scene_remote_list.cpp index defebf76..95ef2e41 100644 --- a/applications/infrared/scene/infrared_app_scene_remote_list.cpp +++ b/applications/infrared/scene/infrared_app_scene_remote_list.cpp @@ -1,12 +1,10 @@ #include "../infrared_app.h" #include "infrared/infrared_app_event.h" #include -#include void InfraredAppSceneRemoteList::on_enter(InfraredApp* app) { furi_assert(app); - FileWorkerCpp file_worker; bool result = false; bool file_select_result; auto remote_manager = app->get_remote_manager(); @@ -15,13 +13,15 @@ void InfraredAppSceneRemoteList::on_enter(InfraredApp* app) { last_selected_remote.size() ? last_selected_remote.c_str() : nullptr; auto filename_ts = std::make_unique(InfraredAppRemoteManager::max_remote_name_length); + DialogsApp* dialogs = app->get_dialogs(); InfraredAppViewManager* view_manager = app->get_view_manager(); ButtonMenu* button_menu = view_manager->get_button_menu(); button_menu_reset(button_menu); view_manager->switch_to(InfraredAppViewManager::ViewId::ButtonMenu); - file_select_result = file_worker.file_select( + file_select_result = dialog_file_select_show( + dialogs, InfraredApp::infrared_directory, InfraredApp::infrared_extension, filename_ts->text, diff --git a/applications/loader/loader.c b/applications/loader/loader.c index 738479cf..3882647e 100644 --- a/applications/loader/loader.c +++ b/applications/loader/loader.c @@ -239,26 +239,11 @@ static void loader_thread_state_callback(FuriThreadState thread_state, void* con event.type = LoaderEventTypeApplicationStarted; furi_pubsub_publish(loader_instance->pubsub, &event); furi_hal_power_insomnia_enter(); - - // Snapshot current memory usage - instance->free_heap_size = memmgr_get_free_heap(); } else if(thread_state == FuriThreadStateStopped) { - /* - * Current Leak Sanitizer assumes that memory is allocated and freed - * inside one thread. Timers are allocated in one task, but freed in - * Timer-Task thread, and xTimerDelete() just put command to queue. - * To avoid some bad cases there are few fixes: - * 1) delay for Timer to process commands - * 2) there are 'heap diff' which shows difference in heap before task - * started and after task completed. In process of leakage monitoring - * both values should be taken into account. - */ - furi_hal_delay_ms(20); - int heap_diff = instance->free_heap_size - memmgr_get_free_heap(); FURI_LOG_I( TAG, - "Application thread stopped. Heap allocation balance: %d. Thread allocation balance: %d.", - heap_diff, + "Application thread stopped. Free heap: %d. Thread allocation balance: %d.", + memmgr_get_free_heap(), furi_thread_get_heap_size(instance->application_thread)); if(loader_instance->application_arguments) { diff --git a/applications/loader/loader_i.h b/applications/loader/loader_i.h index a8b8bfc0..4c937b94 100644 --- a/applications/loader/loader_i.h +++ b/applications/loader/loader_i.h @@ -30,7 +30,6 @@ struct Loader { Submenu* debug_menu; Submenu* settings_menu; - size_t free_heap_size; volatile uint8_t lock_count; FuriPubSub* pubsub; diff --git a/core/furi/memmgr_heap.c b/core/furi/memmgr_heap.c index 9455371e..8544092e 100644 --- a/core/furi/memmgr_heap.c +++ b/core/furi/memmgr_heap.c @@ -171,7 +171,16 @@ size_t memmgr_heap_get_thread_memory(osThreadId_t thread_id) { !MemmgrHeapAllocDict_end_p(alloc_dict_it); MemmgrHeapAllocDict_next(alloc_dict_it)) { MemmgrHeapAllocDict_itref_t* data = MemmgrHeapAllocDict_ref(alloc_dict_it); - leftovers += data->value; + if(data->key != 0) { + uint8_t* puc = (uint8_t*)data->key; + puc -= xHeapStructSize; + BlockLink_t* pxLink = (void*)puc; + + if((pxLink->xBlockSize & xBlockAllocatedBit) != 0 && + pxLink->pxNextFreeBlock == NULL) { + leftovers += data->value; + } + } } } memmgr_heap_thread_trace_depth--; diff --git a/core/furi/thread.c b/core/furi/thread.c index ee86f62e..a2898c1a 100644 --- a/core/furi/thread.c +++ b/core/furi/thread.c @@ -45,6 +45,7 @@ static void furi_thread_body(void* context) { thread->ret = thread->callback(thread->context); if(thread->heap_trace_enabled == true) { + osDelay(33); thread->heap_size = memmgr_heap_get_thread_memory(thread_id); memmgr_heap_disable_thread_trace(thread_id); } diff --git a/firmware/targets/f7/furi_hal/furi_hal_nfc.c b/firmware/targets/f7/furi_hal/furi_hal_nfc.c index ae776b53..bf9ea18c 100755 --- a/firmware/targets/f7/furi_hal/furi_hal_nfc.c +++ b/firmware/targets/f7/furi_hal/furi_hal_nfc.c @@ -230,7 +230,7 @@ bool furi_hal_nfc_listen( rfalNfcDeactivate(true); return false; } - osThreadYield(); + osDelay(1); } return true; } @@ -498,7 +498,7 @@ bool furi_hal_nfc_tx_rx(FuriHalNfcTxRxContext* tx_rx, uint16_t timeout_ms) { } else { start = DWT->CYCCNT; } - osThreadYield(); + osDelay(1); } if(tx_rx->tx_rx_type == FuriHalNfcTxRxTypeRaw) {