From 16e48c8994143f07d2322ecf69e8e1cd195bb988 Mon Sep 17 00:00:00 2001 From: Anna Antonenko Date: Mon, 7 Apr 2025 20:39:58 +0400 Subject: [PATCH 1/9] cli_vcp: handle tx/rx before connext/disconnect --- applications/services/cli/cli_vcp.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/applications/services/cli/cli_vcp.c b/applications/services/cli/cli_vcp.c index f4b539e26..97cd15df3 100644 --- a/applications/services/cli/cli_vcp.c +++ b/applications/services/cli/cli_vcp.c @@ -15,6 +15,7 @@ #define VCP_IF_NUM 0 #define VCP_MESSAGE_Q_LEN 8 +#define CLI_VCP_TRACE #ifdef CLI_VCP_TRACE #define VCP_TRACE(...) FURI_LOG_T(__VA_ARGS__) #else @@ -195,6 +196,17 @@ static void cli_vcp_internal_event_happened(void* context) { CliVcpInternalEvent event = furi_thread_flags_wait(CliVcpInternalEventAll, FuriFlagWaitAny, 0); furi_check(!(event & FuriFlagError)); + if(event & CliVcpInternalEventRx) { + VCP_TRACE(TAG, "Rx"); + cli_vcp_maybe_receive_data(cli_vcp); + } + + if(event & CliVcpInternalEventTxDone) { + VCP_TRACE(TAG, "TxDone"); + cli_vcp->is_currently_transmitting = false; + cli_vcp_maybe_send_data(cli_vcp); + } + if(event & CliVcpInternalEventDisconnected) { if(!cli_vcp->is_connected) return; FURI_LOG_D(TAG, "Disconnected"); @@ -234,17 +246,6 @@ static void cli_vcp_internal_event_happened(void* context) { cli_main_motd, NULL, cli_vcp->shell_pipe, cli_vcp->main_registry, &cli_main_ext_config); cli_shell_start(cli_vcp->shell); } - - if(event & CliVcpInternalEventRx) { - VCP_TRACE(TAG, "Rx"); - cli_vcp_maybe_receive_data(cli_vcp); - } - - if(event & CliVcpInternalEventTxDone) { - VCP_TRACE(TAG, "TxDone"); - cli_vcp->is_currently_transmitting = false; - cli_vcp_maybe_send_data(cli_vcp); - } } // ============ From 7f45050c87d0ea58283e02c98b6794fe85ddc867 Mon Sep 17 00:00:00 2001 From: Anna Antonenko Date: Mon, 7 Apr 2025 20:40:34 +0400 Subject: [PATCH 2/9] cli_vcp: disable trace --- applications/services/cli/cli_vcp.c | 1 - 1 file changed, 1 deletion(-) diff --git a/applications/services/cli/cli_vcp.c b/applications/services/cli/cli_vcp.c index 97cd15df3..8c32c1bfa 100644 --- a/applications/services/cli/cli_vcp.c +++ b/applications/services/cli/cli_vcp.c @@ -15,7 +15,6 @@ #define VCP_IF_NUM 0 #define VCP_MESSAGE_Q_LEN 8 -#define CLI_VCP_TRACE #ifdef CLI_VCP_TRACE #define VCP_TRACE(...) FURI_LOG_T(__VA_ARGS__) #else From b39912af11a8b3efe8c6a8b6a78e12dc85b80e19 Mon Sep 17 00:00:00 2001 From: Anna Antonenko Date: Mon, 7 Apr 2025 23:30:39 +0400 Subject: [PATCH 3/9] cli_perf: advanced error reporting --- .gitignore | 4 ++++ scripts/serial_cli_perf.py | 16 +++++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 84b8e8319..79f2a8058 100644 --- a/.gitignore +++ b/.gitignore @@ -67,3 +67,7 @@ PVS-Studio.log # JS packages node_modules/ + +# cli_perf script output in case of errors +/block.bin +/return_block.bin diff --git a/scripts/serial_cli_perf.py b/scripts/serial_cli_perf.py index 0fed7c393..0db53dd6a 100644 --- a/scripts/serial_cli_perf.py +++ b/scripts/serial_cli_perf.py @@ -32,16 +32,25 @@ def main(): bytes_to_send = args.length block_size = 1024 + success = True while bytes_to_send: actual_size = min(block_size, bytes_to_send) # can't use 0x03 because that's ASCII ETX, or Ctrl+C - block = bytes([randint(4, 255) for _ in range(actual_size)]) + # block = bytes([randint(4, 255) for _ in range(actual_size)]) + block = bytes([4 + (i // 64) for i in range(actual_size)]) port.write(block) return_block = port.read(actual_size) if return_block != block: - logger.error("Incorrect block received") + with open("block.bin", "wb") as f: + f.write(block) + with open("return_block.bin", "wb") as f: + f.write(return_block) + + logger.error("Incorrect block received. Saved to `block.bin' and `return_block.bin'.") + logger.error(f"{bytes_to_send} bytes left. Aborting.") + success = False break bytes_to_send -= actual_size @@ -49,7 +58,8 @@ def main(): end_time = time() delta = end_time - start_time speed = args.length / delta - print(f"Speed: {speed/1024:.2f} KiB/s") + if success: + print(f"Speed: {speed/1024:.2f} KiB/s") port.write(b"\x03") # Ctrl+C port.close() From 29b865ed21d5e33cab14e444235218cfdd05ac9f Mon Sep 17 00:00:00 2001 From: Anna Antonenko Date: Mon, 7 Apr 2025 23:54:13 +0400 Subject: [PATCH 4/9] cli_vcp: reset tx flag directly in event handler --- applications/services/cli/cli_vcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/applications/services/cli/cli_vcp.c b/applications/services/cli/cli_vcp.c index 8c32c1bfa..f23f813a2 100644 --- a/applications/services/cli/cli_vcp.c +++ b/applications/services/cli/cli_vcp.c @@ -106,6 +106,7 @@ static void cli_vcp_signal_internal_event(CliVcp* cli_vcp, CliVcpInternalEvent e static void cli_vcp_cdc_tx_done(void* context) { CliVcp* cli_vcp = context; + cli_vcp->is_currently_transmitting = false; cli_vcp_signal_internal_event(cli_vcp, CliVcpInternalEventTxDone); } @@ -202,7 +203,6 @@ static void cli_vcp_internal_event_happened(void* context) { if(event & CliVcpInternalEventTxDone) { VCP_TRACE(TAG, "TxDone"); - cli_vcp->is_currently_transmitting = false; cli_vcp_maybe_send_data(cli_vcp); } From c8cf76f75e2bac68deeb99e646d2b147de8d8be5 Mon Sep 17 00:00:00 2001 From: Anna Antonenko Date: Mon, 7 Apr 2025 23:56:09 +0400 Subject: [PATCH 5/9] fix formatting --- scripts/serial_cli_perf.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/serial_cli_perf.py b/scripts/serial_cli_perf.py index 0db53dd6a..3d612e6de 100644 --- a/scripts/serial_cli_perf.py +++ b/scripts/serial_cli_perf.py @@ -48,7 +48,9 @@ def main(): with open("return_block.bin", "wb") as f: f.write(return_block) - logger.error("Incorrect block received. Saved to `block.bin' and `return_block.bin'.") + logger.error( + "Incorrect block received. Saved to `block.bin' and `return_block.bin'." + ) logger.error(f"{bytes_to_send} bytes left. Aborting.") success = False break From e712375edc9515fae16f3909325055ed4c0ce65c Mon Sep 17 00:00:00 2001 From: Anna Antonenko Date: Mon, 7 Apr 2025 23:59:10 +0400 Subject: [PATCH 6/9] cli_vcp: make tx flag volatile --- applications/services/cli/cli_vcp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/applications/services/cli/cli_vcp.c b/applications/services/cli/cli_vcp.c index f23f813a2..99f2d7880 100644 --- a/applications/services/cli/cli_vcp.c +++ b/applications/services/cli/cli_vcp.c @@ -50,7 +50,7 @@ struct CliVcp { PipeSide* own_pipe; PipeSide* shell_pipe; - bool is_currently_transmitting; + volatile bool is_currently_transmitting; size_t previous_tx_length; CliRegistry* main_registry; From 6b7a7c570922c50b4aee1b45eaae22fd05cb361f Mon Sep 17 00:00:00 2001 From: Anna Antonenko Date: Tue, 8 Apr 2025 00:25:31 +0400 Subject: [PATCH 7/9] storage_settings: fix scene ids --- applications/settings/storage_settings/storage_settings.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/applications/settings/storage_settings/storage_settings.c b/applications/settings/storage_settings/storage_settings.c index b4cde7b6b..07656431c 100644 --- a/applications/settings/storage_settings/storage_settings.c +++ b/applications/settings/storage_settings/storage_settings.c @@ -5,8 +5,8 @@ const SubmenuSettingsHelperDescriptor descriptor_template = { .options_cnt = 6, .options = { - {.name = "About Internal Storage", .scene_id = StorageSettingsSDInfo}, - {.name = "About SD Card", .scene_id = StorageSettingsInternalInfo}, + {.name = "About Internal Storage", .scene_id = StorageSettingsInternalInfo}, + {.name = "About SD Card", .scene_id = StorageSettingsSDInfo}, {.name = "Unmount SD Card", .scene_id = StorageSettingsUnmountConfirm}, {.name = "Format SD Card", .scene_id = StorageSettingsFormatConfirm}, {.name = "Benchmark SD Card", .scene_id = StorageSettingsBenchmarkConfirm}, From 9a6aa17beee3e4c9cfe34dd3dc84cc445133aa0a Mon Sep 17 00:00:00 2001 From: Anna Antonenko Date: Tue, 8 Apr 2025 16:02:33 +0400 Subject: [PATCH 8/9] cli_shell: add safety check to set_prompt --- lib/toolbox/cli/shell/cli_shell.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/toolbox/cli/shell/cli_shell.c b/lib/toolbox/cli/shell/cli_shell.c index daf5065ec..445a5f2aa 100644 --- a/lib/toolbox/cli/shell/cli_shell.c +++ b/lib/toolbox/cli/shell/cli_shell.c @@ -474,5 +474,6 @@ void cli_shell_join(CliShell* shell) { void cli_shell_set_prompt(CliShell* shell, const char* prompt) { furi_check(shell); + furi_check(furi_thread_get_state(shell->thread) == FuriThreadStateStopped); shell->prompt = prompt; } From 4140952605a60b42a8c282bc46a5ca6505eb9bf2 Mon Sep 17 00:00:00 2001 From: Anna Antonenko Date: Tue, 8 Apr 2025 18:05:08 +0400 Subject: [PATCH 9/9] cli_registry: move from bptree to dict, fix memory leak --- lib/toolbox/cli/cli_registry.c | 32 +++++++++---------- lib/toolbox/cli/cli_registry_i.h | 18 +++-------- lib/toolbox/cli/shell/cli_shell.c | 14 ++++---- lib/toolbox/cli/shell/cli_shell_completions.c | 6 ++-- 4 files changed, 29 insertions(+), 41 deletions(-) diff --git a/lib/toolbox/cli/cli_registry.c b/lib/toolbox/cli/cli_registry.c index 35bed19f2..91f7c4046 100644 --- a/lib/toolbox/cli/cli_registry.c +++ b/lib/toolbox/cli/cli_registry.c @@ -3,16 +3,16 @@ #include #include -#define TAG "cli" +#define TAG "CliRegistry" struct CliRegistry { - CliCommandTree_t commands; + CliCommandDict_t commands; FuriMutex* mutex; }; CliRegistry* cli_registry_alloc(void) { CliRegistry* registry = malloc(sizeof(CliRegistry)); - CliCommandTree_init(registry->commands); + CliCommandDict_init(registry->commands); registry->mutex = furi_mutex_alloc(FuriMutexTypeRecursive); return registry; } @@ -20,7 +20,7 @@ CliRegistry* cli_registry_alloc(void) { void cli_registry_free(CliRegistry* registry) { furi_check(furi_mutex_acquire(registry->mutex, FuriWaitForever) == FuriStatusOk); furi_mutex_free(registry->mutex); - CliCommandTree_clear(registry->commands); + CliCommandDict_clear(registry->commands); free(registry); } @@ -61,7 +61,7 @@ void cli_registry_add_command_ex( }; furi_check(furi_mutex_acquire(registry->mutex, FuriWaitForever) == FuriStatusOk); - CliCommandTree_set_at(registry->commands, name_str, command); + CliCommandDict_set_at(registry->commands, name_str, command); furi_check(furi_mutex_release(registry->mutex) == FuriStatusOk); furi_string_free(name_str); @@ -79,7 +79,7 @@ void cli_registry_delete_command(CliRegistry* registry, const char* name) { } while(name_replace != FURI_STRING_FAILURE); furi_check(furi_mutex_acquire(registry->mutex, FuriWaitForever) == FuriStatusOk); - CliCommandTree_erase(registry->commands, name_str); + CliCommandDict_erase(registry->commands, name_str); furi_check(furi_mutex_release(registry->mutex) == FuriStatusOk); furi_string_free(name_str); @@ -91,7 +91,7 @@ bool cli_registry_get_command( CliRegistryCommand* result) { furi_assert(registry); furi_check(furi_mutex_acquire(registry->mutex, FuriWaitForever) == FuriStatusOk); - CliRegistryCommand* data = CliCommandTree_get(registry->commands, command); + CliRegistryCommand* data = CliCommandDict_get(registry->commands, command); if(data) *result = *data; furi_check(furi_mutex_release(registry->mutex) == FuriStatusOk); @@ -103,16 +103,14 @@ void cli_registry_remove_external_commands(CliRegistry* registry) { furi_check(registry); furi_check(furi_mutex_acquire(registry->mutex, FuriWaitForever) == FuriStatusOk); - // FIXME FL-3977: memory leak somewhere within this function - - CliCommandTree_t internal_cmds; - CliCommandTree_init(internal_cmds); + CliCommandDict_t internal_cmds; + CliCommandDict_init(internal_cmds); for - M_EACH(item, registry->commands, CliCommandTree_t) { - if(!(item->value_ptr->flags & CliCommandFlagExternal)) - CliCommandTree_set_at(internal_cmds, *item->key_ptr, *item->value_ptr); + M_EACH(item, registry->commands, CliCommandDict_t) { + if(!(item->value.flags & CliCommandFlagExternal)) + CliCommandDict_set_at(internal_cmds, item->key, item->value); } - CliCommandTree_move(registry->commands, internal_cmds); + CliCommandDict_move(registry->commands, internal_cmds); furi_check(furi_mutex_release(registry->mutex) == FuriStatusOk); } @@ -148,7 +146,7 @@ void cli_registry_reload_external_commands( .execute_callback = NULL, .flags = CliCommandFlagExternal, }; - CliCommandTree_set_at(registry->commands, plugin_name, command); + CliCommandDict_set_at(registry->commands, plugin_name, command); } furi_string_free(plugin_name); @@ -172,7 +170,7 @@ void cli_registry_unlock(CliRegistry* registry) { furi_mutex_release(registry->mutex); } -CliCommandTree_t* cli_registry_get_commands(CliRegistry* registry) { +CliCommandDict_t* cli_registry_get_commands(CliRegistry* registry) { furi_assert(registry); return ®istry->commands; } diff --git a/lib/toolbox/cli/cli_registry_i.h b/lib/toolbox/cli/cli_registry_i.h index 95b7c55da..31995832f 100644 --- a/lib/toolbox/cli/cli_registry_i.h +++ b/lib/toolbox/cli/cli_registry_i.h @@ -6,7 +6,7 @@ #pragma once #include -#include +#include #include "cli_registry.h" #ifdef __cplusplus @@ -22,19 +22,9 @@ typedef struct { size_t stack_depth; } CliRegistryCommand; -#define CLI_COMMANDS_TREE_RANK 4 +DICT_DEF2(CliCommandDict, FuriString*, FURI_STRING_OPLIST, CliRegistryCommand, M_POD_OPLIST); -// -V:BPTREE_DEF2:1103 -// -V:BPTREE_DEF2:524 -BPTREE_DEF2( - CliCommandTree, - CLI_COMMANDS_TREE_RANK, - FuriString*, - FURI_STRING_OPLIST, - CliRegistryCommand, - M_POD_OPLIST); - -#define M_OPL_CliCommandTree_t() BPTREE_OPLIST2(CliCommandTree, FURI_STRING_OPLIST, M_POD_OPLIST) +#define M_OPL_CliCommandDict_t() DICT_OPLIST(CliCommandDict, FURI_STRING_OPLIST, M_POD_OPLIST) bool cli_registry_get_command( CliRegistry* registry, @@ -48,7 +38,7 @@ void cli_registry_unlock(CliRegistry* registry); /** * @warning Surround calls to this function with `cli_registry_[un]lock` */ -CliCommandTree_t* cli_registry_get_commands(CliRegistry* registry); +CliCommandDict_t* cli_registry_get_commands(CliRegistry* registry); #ifdef __cplusplus } diff --git a/lib/toolbox/cli/shell/cli_shell.c b/lib/toolbox/cli/shell/cli_shell.c index 445a5f2aa..8aa7c387a 100644 --- a/lib/toolbox/cli/shell/cli_shell.c +++ b/lib/toolbox/cli/shell/cli_shell.c @@ -103,15 +103,15 @@ void cli_command_help(PipeSide* pipe, FuriString* args, void* context) { printf("Available commands:\r\n" ANSI_FG_GREEN); cli_registry_lock(registry); - CliCommandTree_t* commands = cli_registry_get_commands(registry); - size_t commands_count = CliCommandTree_size(*commands); + CliCommandDict_t* commands = cli_registry_get_commands(registry); + size_t commands_count = CliCommandDict_size(*commands); - CliCommandTree_it_t iterator; - CliCommandTree_it(iterator, *commands); + CliCommandDict_it_t iterator; + CliCommandDict_it(iterator, *commands); for(size_t i = 0; i < commands_count; i++) { - const CliCommandTree_itref_t* item = CliCommandTree_cref(iterator); - printf("%-30s", furi_string_get_cstr(*item->key_ptr)); - CliCommandTree_next(iterator); + const CliCommandDict_itref_t* item = CliCommandDict_cref(iterator); + printf("%-30s", furi_string_get_cstr(item->key)); + CliCommandDict_next(iterator); if(i % columns == columns - 1) printf("\r\n"); } diff --git a/lib/toolbox/cli/shell/cli_shell_completions.c b/lib/toolbox/cli/shell/cli_shell_completions.c index 823f91fb9..7a178705d 100644 --- a/lib/toolbox/cli/shell/cli_shell_completions.c +++ b/lib/toolbox/cli/shell/cli_shell_completions.c @@ -111,10 +111,10 @@ void cli_shell_completions_fill_variants(CliShellCompletions* completions) { if(segment.type == CliShellCompletionSegmentTypeCommand) { CliRegistry* registry = completions->registry; cli_registry_lock(registry); - CliCommandTree_t* commands = cli_registry_get_commands(registry); + CliCommandDict_t* commands = cli_registry_get_commands(registry); for - M_EACH(registered_command, *commands, CliCommandTree_t) { - FuriString* command_name = *registered_command->key_ptr; + M_EACH(registered_command, *commands, CliCommandDict_t) { + FuriString* command_name = registered_command->key; if(furi_string_start_with(command_name, input)) { CommandCompletions_push_back(completions->variants, command_name); }