Merge remote-tracking branch 'OFW/porta/cli-event-ordering' into dev

.gitignore

@@ -73,3 +73,7 @@ PVS-Studio.log
 
 # JS packages
 node_modules/
+
+# cli_perf script output in case of errors
+/block.bin
+/return_block.bin

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;
@@ -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);
 }
 
@@ -195,6 +196,16 @@ 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_maybe_send_data(cli_vcp);
+    }
+
     if(event & CliVcpInternalEventDisconnected) {
         if(!cli_vcp->is_connected) return;
         FURI_LOG_D(TAG, "Disconnected");
@@ -234,17 +245,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);
-    }
 }
 
 // ============

lib/toolbox/cli/cli_registry.c

@@ -3,16 +3,16 @@
 #include <toolbox/pipe.h>
 #include <storage/storage.h>
 
-#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 &registry->commands;
 }

lib/toolbox/cli/cli_registry_i.h

@@ -6,7 +6,7 @@
 #pragma once
 
 #include <furi.h>
-#include <m-bptree.h>
+#include <m-dict.h>
 #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
 }

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");
     }
@@ -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;
 }

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);
                 }

scripts/serial_cli_perf.py

@@ -32,16 +32,27 @@ 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,6 +60,7 @@ def main():
     end_time = time()
     delta = end_time - start_time
     speed = args.length / delta
+    if success:
         print(f"Speed: {speed/1024:.2f} KiB/s")
 
     port.write(b"\x03")  # Ctrl+C