From 809418b9dabef8914f4f56badd12ae7a0d34f19c Mon Sep 17 00:00:00 2001 From: Sergey Gavrilov Date: Fri, 1 Sep 2023 04:23:37 +0300 Subject: [PATCH] [FL-3563] StorageListRequest: size filter (#3018) * Protobuf: size filter * Update protobuf * Scripts: types for fwflash.py * RPC: handle fliter for StorageListRequest * RPC: StorageListRequest tests for filtering * Fix unit tests configuration * Assets: sync protobuf with upstream Co-authored-by: Aleksandr Kutuzov --- applications/debug/unit_tests/rpc/rpc_test.c | 208 +++++++++++-------- applications/services/rpc/rpc_storage.c | 33 ++- assets/protobuf | 2 +- fbt_options.py | 1 + scripts/fwflash.py | 15 +- 5 files changed, 154 insertions(+), 105 deletions(-) diff --git a/applications/debug/unit_tests/rpc/rpc_test.c b/applications/debug/unit_tests/rpc/rpc_test.c index 533a8a9c..645e75e8 100644 --- a/applications/debug/unit_tests/rpc/rpc_test.c +++ b/applications/debug/unit_tests/rpc/rpc_test.c @@ -67,7 +67,6 @@ static RpcSessionContext rpc_session[TEST_RPC_SESSIONS]; } while(0) static void output_bytes_callback(void* ctx, uint8_t* got_bytes, size_t got_size); -static void clean_directory(Storage* fs_api, const char* clean_dir); static void test_rpc_add_empty_to_list(MsgList_t msg_list, PB_CommandStatus status, uint32_t command_id); static void test_rpc_encode_and_feed(MsgList_t msg_list, uint8_t session); @@ -149,11 +148,41 @@ static void test_rpc_teardown_second_session(void) { rpc_session[1].session = NULL; } +static void test_rpc_storage_clean_directory(Storage* fs_api, const char* clean_dir) { + furi_check(fs_api); + furi_check(clean_dir); + storage_simply_remove_recursive(fs_api, clean_dir); + FS_Error error = storage_common_mkdir(fs_api, clean_dir); + furi_check(error == FSE_OK); +} + +static void test_rpc_storage_create_file(Storage* fs_api, const char* path, size_t size) { + File* file = storage_file_alloc(fs_api); + + bool success = false; + do { + if(!storage_file_open(file, path, FSAM_WRITE, FSOM_CREATE_ALWAYS)) break; + if(!storage_file_seek(file, size, true)) break; + success = true; + } while(false); + + storage_file_close(file); + storage_file_free(file); + + furi_check(success); +} + static void test_rpc_storage_setup(void) { test_rpc_setup(); Storage* fs_api = furi_record_open(RECORD_STORAGE); - clean_directory(fs_api, TEST_DIR_NAME); + test_rpc_storage_clean_directory(fs_api, TEST_DIR_NAME); + test_rpc_storage_create_file(fs_api, TEST_DIR_NAME "/file100", 100); + test_rpc_storage_create_file(fs_api, TEST_DIR_NAME "/file250", 250); + test_rpc_storage_create_file(fs_api, TEST_DIR_NAME "/file500", 200); + test_rpc_storage_create_file(fs_api, TEST_DIR_NAME "/file1000", 1000); + test_rpc_storage_create_file(fs_api, TEST_DIR_NAME "/file2500", 2500); + test_rpc_storage_create_file(fs_api, TEST_DIR_NAME "/file5000", 5000); furi_record_close(RECORD_STORAGE); } @@ -161,7 +190,7 @@ static void test_rpc_storage_teardown(void) { test_rpc_teardown(); Storage* fs_api = furi_record_open(RECORD_STORAGE); - clean_directory(fs_api, TEST_DIR_NAME); + test_rpc_storage_clean_directory(fs_api, TEST_DIR_NAME); furi_record_close(RECORD_STORAGE); } @@ -179,36 +208,6 @@ static void test_rpc_session_terminated_callback(void* context) { xSemaphoreGive(callbacks_context->terminate_semaphore); } -static void clean_directory(Storage* fs_api, const char* clean_dir) { - furi_check(fs_api); - furi_check(clean_dir); - - File* dir = storage_file_alloc(fs_api); - if(storage_dir_open(dir, clean_dir)) { - FileInfo fileinfo; - char* name = malloc(MAX_NAME_LENGTH + 1); - while(storage_dir_read(dir, &fileinfo, name, MAX_NAME_LENGTH)) { - size_t size = strlen(clean_dir) + strlen(name) + 1 + 1; - char* fullname = malloc(size); - snprintf(fullname, size, "%s/%s", clean_dir, name); - if(file_info_is_dir(&fileinfo)) { - clean_directory(fs_api, fullname); - } - FS_Error error = storage_common_remove(fs_api, fullname); - furi_check(error == FSE_OK); - free(fullname); - } - free(name); - } else { - FS_Error error = storage_common_mkdir(fs_api, clean_dir); - (void)error; - furi_check(error == FSE_OK); - } - - storage_dir_close(dir); - storage_file_free(dir); -} - static void test_rpc_print_message_list(MsgList_t msg_list) { #if DEBUG_PRINT MsgList_reverse(msg_list); @@ -282,24 +281,40 @@ static void test_rpc_add_ping_to_list(MsgList_t msg_list, bool request, uint32_t response->which_content = (request == PING_REQUEST) ? PB_Main_system_ping_request_tag : PB_Main_system_ping_response_tag; } +static void test_rpc_fill_basic_message(PB_Main* message, uint16_t tag, uint32_t command_id) { + message->command_id = command_id; + message->command_status = PB_CommandStatus_OK; + message->cb_content.funcs.encode = NULL; + message->which_content = tag; + message->has_next = false; +} + +static void test_rpc_create_storage_list_request( + PB_Main* message, + const char* path, + bool include_md5, + uint32_t command_id, + uint32_t filter_max_size) { + furi_check(message); + furi_check(path); + test_rpc_fill_basic_message(message, PB_Main_storage_list_request_tag, command_id); + message->content.storage_list_request.path = strdup(path); + message->content.storage_list_request.include_md5 = include_md5; + message->content.storage_list_request.filter_max_size = filter_max_size; +} static void test_rpc_create_simple_message( PB_Main* message, uint16_t tag, const char* str, - uint32_t command_id, - bool flag) { + uint32_t command_id) { furi_check(message); char* str_copy = NULL; if(str) { str_copy = strdup(str); } - message->command_id = command_id; - message->command_status = PB_CommandStatus_OK; - message->cb_content.funcs.encode = NULL; - message->which_content = tag; - message->has_next = false; + test_rpc_fill_basic_message(message, tag, command_id); switch(tag) { case PB_Main_storage_info_request_tag: message->content.storage_info_request.path = str_copy; @@ -307,10 +322,6 @@ static void test_rpc_create_simple_message( case PB_Main_storage_stat_request_tag: message->content.storage_stat_request.path = str_copy; break; - case PB_Main_storage_list_request_tag: - message->content.storage_list_request.path = str_copy; - message->content.storage_list_request.include_md5 = flag; - break; case PB_Main_storage_mkdir_request_tag: message->content.storage_mkdir_request.path = str_copy; break; @@ -573,11 +584,29 @@ static void message->content.storage_list_response.file[2].name = str; } +static bool test_rpc_system_storage_list_filter( + const FileInfo* fileinfo, + const char* name, + size_t filter_max_size) { + bool result = false; + + do { + if(!path_contains_only_ascii(name)) break; + if(filter_max_size) { + if(fileinfo->size > filter_max_size) break; + } + result = true; + } while(false); + + return result; +} + static void test_rpc_storage_list_create_expected_list( MsgList_t msg_list, const char* path, uint32_t command_id, - bool append_md5) { + bool append_md5, + size_t filter_max_size) { Storage* fs_api = furi_record_open(RECORD_STORAGE); File* dir = storage_file_alloc(fs_api); @@ -615,7 +644,7 @@ static void test_rpc_storage_list_create_expected_list( i = 0; } - if(path_contains_only_ascii(name)) { + if(test_rpc_system_storage_list_filter(&fileinfo, name, filter_max_size)) { list->file[i].type = file_info_is_dir(&fileinfo) ? PB_Storage_File_FileType_DIR : PB_Storage_File_FileType_FILE; list->file[i].size = fileinfo.size; @@ -698,17 +727,21 @@ static void test_rpc_free_msg_list(MsgList_t msg_list) { MsgList_clear(msg_list); } -static void test_rpc_storage_list_run(const char* path, uint32_t command_id, bool md5) { +static void test_rpc_storage_list_run( + const char* path, + uint32_t command_id, + bool md5, + size_t filter_max_size) { PB_Main request; MsgList_t expected_msg_list; MsgList_init(expected_msg_list); - test_rpc_create_simple_message( - &request, PB_Main_storage_list_request_tag, path, command_id, md5); + test_rpc_create_storage_list_request(&request, path, md5, command_id, filter_max_size); if(!strcmp(path, "/")) { test_rpc_storage_list_create_expected_list_root(expected_msg_list, command_id); } else { - test_rpc_storage_list_create_expected_list(expected_msg_list, path, command_id, md5); + test_rpc_storage_list_create_expected_list( + expected_msg_list, path, command_id, md5, filter_max_size); } test_rpc_encode_and_feed_one(&request, 0); test_rpc_decode_and_compare(expected_msg_list, 0); @@ -718,25 +751,32 @@ static void test_rpc_storage_list_run(const char* path, uint32_t command_id, boo } MU_TEST(test_storage_list) { - test_rpc_storage_list_run("/", ++command_id, false); - test_rpc_storage_list_run(EXT_PATH("nfc"), ++command_id, false); - test_rpc_storage_list_run(STORAGE_INT_PATH_PREFIX, ++command_id, false); - test_rpc_storage_list_run(STORAGE_EXT_PATH_PREFIX, ++command_id, false); - test_rpc_storage_list_run(EXT_PATH("infrared"), ++command_id, false); - test_rpc_storage_list_run(EXT_PATH("ibutton"), ++command_id, false); - test_rpc_storage_list_run(EXT_PATH("lfrfid"), ++command_id, false); - test_rpc_storage_list_run("error_path", ++command_id, false); + test_rpc_storage_list_run("/", ++command_id, false, 0); + test_rpc_storage_list_run(EXT_PATH("nfc"), ++command_id, false, 0); + test_rpc_storage_list_run(STORAGE_INT_PATH_PREFIX, ++command_id, false, 0); + test_rpc_storage_list_run(STORAGE_EXT_PATH_PREFIX, ++command_id, false, 0); + test_rpc_storage_list_run(EXT_PATH("infrared"), ++command_id, false, 0); + test_rpc_storage_list_run(EXT_PATH("ibutton"), ++command_id, false, 0); + test_rpc_storage_list_run(EXT_PATH("lfrfid"), ++command_id, false, 0); + test_rpc_storage_list_run("error_path", ++command_id, false, 0); } MU_TEST(test_storage_list_md5) { - test_rpc_storage_list_run("/", ++command_id, true); - test_rpc_storage_list_run(EXT_PATH("nfc"), ++command_id, true); - test_rpc_storage_list_run(STORAGE_INT_PATH_PREFIX, ++command_id, true); - test_rpc_storage_list_run(STORAGE_EXT_PATH_PREFIX, ++command_id, true); - test_rpc_storage_list_run(EXT_PATH("infrared"), ++command_id, true); - test_rpc_storage_list_run(EXT_PATH("ibutton"), ++command_id, true); - test_rpc_storage_list_run(EXT_PATH("lfrfid"), ++command_id, true); - test_rpc_storage_list_run("error_path", ++command_id, true); + test_rpc_storage_list_run("/", ++command_id, true, 0); + test_rpc_storage_list_run(EXT_PATH("nfc"), ++command_id, true, 0); + test_rpc_storage_list_run(STORAGE_INT_PATH_PREFIX, ++command_id, true, 0); + test_rpc_storage_list_run(STORAGE_EXT_PATH_PREFIX, ++command_id, true, 0); + test_rpc_storage_list_run(EXT_PATH("infrared"), ++command_id, true, 0); + test_rpc_storage_list_run(EXT_PATH("ibutton"), ++command_id, true, 0); + test_rpc_storage_list_run(EXT_PATH("lfrfid"), ++command_id, true, 0); + test_rpc_storage_list_run("error_path", ++command_id, true, 0); +} + +MU_TEST(test_storage_list_size) { + test_rpc_storage_list_run(TEST_DIR_NAME, ++command_id, false, 0); + test_rpc_storage_list_run(TEST_DIR_NAME, ++command_id, false, 1); + test_rpc_storage_list_run(TEST_DIR_NAME, ++command_id, false, 1000); + test_rpc_storage_list_run(TEST_DIR_NAME, ++command_id, false, 2500); } static void @@ -804,8 +844,7 @@ static void test_storage_read_run(const char* path, uint32_t command_id) { MsgList_init(expected_msg_list); test_rpc_add_read_to_list_by_reading_real_file(expected_msg_list, path, command_id); - test_rpc_create_simple_message( - &request, PB_Main_storage_read_request_tag, path, command_id, false); + test_rpc_create_simple_message(&request, PB_Main_storage_read_request_tag, path, command_id); test_rpc_encode_and_feed_one(&request, 0); test_rpc_decode_and_compare(expected_msg_list, 0); @@ -859,8 +898,7 @@ static void test_rpc_storage_info_run(const char* path, uint32_t command_id) { MsgList_t expected_msg_list; MsgList_init(expected_msg_list); - test_rpc_create_simple_message( - &request, PB_Main_storage_info_request_tag, path, command_id, false); + test_rpc_create_simple_message(&request, PB_Main_storage_info_request_tag, path, command_id); PB_Main* response = MsgList_push_new(expected_msg_list); response->command_id = command_id; @@ -892,8 +930,7 @@ static void test_rpc_storage_stat_run(const char* path, uint32_t command_id) { MsgList_t expected_msg_list; MsgList_init(expected_msg_list); - test_rpc_create_simple_message( - &request, PB_Main_storage_stat_request_tag, path, command_id, false); + test_rpc_create_simple_message(&request, PB_Main_storage_stat_request_tag, path, command_id); Storage* fs_api = furi_record_open(RECORD_STORAGE); FileInfo fileinfo; @@ -1005,11 +1042,7 @@ static void test_storage_write_read_run( test_rpc_add_empty_to_list(expected_msg_list, PB_CommandStatus_OK, *command_id); test_rpc_create_simple_message( - MsgList_push_raw(input_msg_list), - PB_Main_storage_read_request_tag, - path, - ++*command_id, - false); + MsgList_push_raw(input_msg_list), PB_Main_storage_read_request_tag, path, ++*command_id); test_rpc_add_read_or_write_to_list( expected_msg_list, READ_RESPONSE, @@ -1082,8 +1115,7 @@ MU_TEST(test_storage_interrupt_continuous_same_system) { MsgList_push_new(input_msg_list), PB_Main_storage_mkdir_request_tag, TEST_DIR "dir1", - command_id + 1, - false); + command_id + 1); test_rpc_add_read_or_write_to_list( input_msg_list, WRITE_REQUEST, @@ -1163,8 +1195,7 @@ static void test_storage_delete_run( MsgList_t expected_msg_list; MsgList_init(expected_msg_list); - test_rpc_create_simple_message( - &request, PB_Main_storage_delete_request_tag, path, command_id, false); + test_rpc_create_simple_message(&request, PB_Main_storage_delete_request_tag, path, command_id); request.content.storage_delete_request.recursive = recursive; test_rpc_add_empty_to_list(expected_msg_list, status, command_id); @@ -1245,8 +1276,7 @@ static void test_storage_mkdir_run(const char* path, size_t command_id, PB_Comma MsgList_t expected_msg_list; MsgList_init(expected_msg_list); - test_rpc_create_simple_message( - &request, PB_Main_storage_mkdir_request_tag, path, command_id, false); + test_rpc_create_simple_message(&request, PB_Main_storage_mkdir_request_tag, path, command_id); test_rpc_add_empty_to_list(expected_msg_list, status, command_id); test_rpc_encode_and_feed_one(&request, 0); @@ -1297,12 +1327,11 @@ static void test_storage_md5sum_run( MsgList_t expected_msg_list; MsgList_init(expected_msg_list); - test_rpc_create_simple_message( - &request, PB_Main_storage_md5sum_request_tag, path, command_id, false); + test_rpc_create_simple_message(&request, PB_Main_storage_md5sum_request_tag, path, command_id); if(status == PB_CommandStatus_OK) { PB_Main* response = MsgList_push_new(expected_msg_list); test_rpc_create_simple_message( - response, PB_Main_storage_md5sum_response_tag, md5sum, command_id, false); + response, PB_Main_storage_md5sum_response_tag, md5sum, command_id); response->command_status = status; } else { test_rpc_add_empty_to_list(expected_msg_list, status, command_id); @@ -1461,6 +1490,7 @@ MU_TEST_SUITE(test_rpc_storage) { MU_RUN_TEST(test_storage_stat); MU_RUN_TEST(test_storage_list); MU_RUN_TEST(test_storage_list_md5); + MU_RUN_TEST(test_storage_list_size); MU_RUN_TEST(test_storage_read); MU_RUN_TEST(test_storage_write_read); MU_RUN_TEST(test_storage_write); @@ -1759,8 +1789,7 @@ MU_TEST(test_rpc_multisession_storage) { MsgList_push_raw(input_0), PB_Main_storage_read_request_tag, TEST_DIR "file0.txt", - ++command_id, - false); + ++command_id); test_rpc_add_read_or_write_to_list( expected_0, READ_RESPONSE, TEST_DIR "file0.txt", pattern, sizeof(pattern), 1, command_id); @@ -1768,8 +1797,7 @@ MU_TEST(test_rpc_multisession_storage) { MsgList_push_raw(input_1), PB_Main_storage_read_request_tag, TEST_DIR "file1.txt", - ++command_id, - false); + ++command_id); test_rpc_add_read_or_write_to_list( expected_1, READ_RESPONSE, TEST_DIR "file1.txt", pattern, sizeof(pattern), 1, command_id); diff --git a/applications/services/rpc/rpc_storage.c b/applications/services/rpc/rpc_storage.c index 93c7043e..ee024b82 100644 --- a/applications/services/rpc/rpc_storage.c +++ b/applications/services/rpc/rpc_storage.c @@ -242,6 +242,23 @@ static void rpc_system_storage_list_root(const PB_Main* request, void* context) rpc_send_and_release(session, &response); } +static bool rpc_system_storage_list_filter( + const PB_Storage_ListRequest* request, + const FileInfo* fileinfo, + const char* name) { + bool result = false; + + do { + if(!path_contains_only_ascii(name)) break; + if(request->filter_max_size) { + if(fileinfo->size > request->filter_max_size) break; + } + result = true; + } while(false); + + return result; +} + static void rpc_system_storage_list_process(const PB_Main* request, void* context) { furi_assert(request); furi_assert(context); @@ -253,9 +270,11 @@ static void rpc_system_storage_list_process(const PB_Main* request, void* contex RpcSession* session = rpc_storage->session; furi_assert(session); + const PB_Storage_ListRequest* list_request = &request->content.storage_list_request; + rpc_system_storage_reset_state(rpc_storage, session, true); - if(!strcmp(request->content.storage_list_request.path, "/")) { + if(!strcmp(list_request->path, "/")) { rpc_system_storage_list_root(request, context); return; } @@ -271,7 +290,7 @@ static void rpc_system_storage_list_process(const PB_Main* request, void* contex }; PB_Storage_ListResponse* list = &response.content.storage_list_response; - bool include_md5 = request->content.storage_list_request.include_md5; + bool include_md5 = list_request->include_md5; FuriString* md5 = furi_string_alloc(); FuriString* md5_path = furi_string_alloc(); File* file = storage_file_alloc(fs_api); @@ -279,7 +298,7 @@ static void rpc_system_storage_list_process(const PB_Main* request, void* contex bool finish = false; int i = 0; - if(!storage_dir_open(dir, request->content.storage_list_request.path)) { + if(!storage_dir_open(dir, list_request->path)) { response.command_status = rpc_system_storage_get_file_error(dir); response.which_content = PB_Main_empty_tag; finish = true; @@ -289,7 +308,7 @@ static void rpc_system_storage_list_process(const PB_Main* request, void* contex FileInfo fileinfo; char* name = malloc(MAX_NAME_LENGTH + 1); if(storage_dir_read(dir, &fileinfo, name, MAX_NAME_LENGTH)) { - if(path_contains_only_ascii(name)) { + if(rpc_system_storage_list_filter(list_request, &fileinfo, name)) { if(i == COUNT_OF(list->file)) { list->file_count = i; response.has_next = true; @@ -303,11 +322,7 @@ static void rpc_system_storage_list_process(const PB_Main* request, void* contex list->file[i].name = name; if(include_md5 && !file_info_is_dir(&fileinfo)) { - furi_string_printf( //-V576 - md5_path, - "%s/%s", - request->content.storage_list_request.path, - name); + furi_string_printf(md5_path, "%s/%s", list_request->path, name); //-V576 if(md5_string_calc_file(file, furi_string_get_cstr(md5_path), md5, NULL)) { char* md5sum = list->file[i].md5sum; diff --git a/assets/protobuf b/assets/protobuf index 7e011a95..327163d5 160000 --- a/assets/protobuf +++ b/assets/protobuf @@ -1 +1 @@ -Subproject commit 7e011a95863716e72e7c6b5d552bca241d688304 +Subproject commit 327163d5867c7aa3051334c93ced718d15bfe4da diff --git a/fbt_options.py b/fbt_options.py index b6fcc70f..bd804fc8 100644 --- a/fbt_options.py +++ b/fbt_options.py @@ -72,6 +72,7 @@ FIRMWARE_APPS = { "unit_tests": [ "basic_services", "updater_app", + "radio_device_cc1101_ext", "unit_tests", ], } diff --git a/scripts/fwflash.py b/scripts/fwflash.py index c119aaf8..6948bd7f 100755 --- a/scripts/fwflash.py +++ b/scripts/fwflash.py @@ -10,6 +10,7 @@ from abc import ABC, abstractmethod from dataclasses import dataclass, field from flipper.app import App +from serial.tools.list_ports_common import ListPortInfo # When adding an interface, also add it to SWD_TRANSPORT in fbt/ufbt options @@ -88,8 +89,9 @@ class OpenOCDProgrammer(Programmer): self._add_file(openocd_launch_params, self.interface.config_file) if self.serial: self._add_serial(openocd_launch_params, self.serial) - for additional_arg in self.interface.additional_args: - self._add_command(openocd_launch_params, additional_arg) + if self.interface.additional_args: + for additional_arg in self.interface.additional_args: + self._add_command(openocd_launch_params, additional_arg) self._add_file(openocd_launch_params, "target/stm32wbx.cfg") self._add_command(openocd_launch_params, "init") program_params = [ @@ -124,8 +126,9 @@ class OpenOCDProgrammer(Programmer): self._add_file(openocd_launch_params, self.interface.config_file) if self.serial: self._add_serial(openocd_launch_params, self.serial) - for additional_arg in self.interface.additional_args: - self._add_command(openocd_launch_params, additional_arg) + if self.interface.additional_args: + for additional_arg in self.interface.additional_args: + self._add_command(openocd_launch_params, additional_arg) self._add_file(openocd_launch_params, "target/stm32wbx.cfg") self._add_command(openocd_launch_params, "init") self._add_command(openocd_launch_params, "exit") @@ -167,7 +170,9 @@ def blackmagic_find_serial(serial: str): if not serial.startswith("\\\\.\\"): serial = f"\\\\.\\{serial}" - ports = list(list_ports.grep("blackmagic")) + # idk why, but python thinks that list_ports.grep returns tuple[str, str, str] + ports: list[ListPortInfo] = list(list_ports.grep("blackmagic")) # type: ignore + if len(ports) == 0: return None elif len(ports) > 2: