From 90cefe7c7198d09c2bcaf43eff0e8ded54c761c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=81=82=E3=81=8F?= Date: Sat, 12 Nov 2022 17:46:04 +0900 Subject: [PATCH] [FL-2975] Bug fixes and improvements: Furi, Input, Cli (#2004) * Furi: configurable heap allocation tracking * Furi: relax restriction in thread heap setter asserts, apply heap tracking setting on app start instead of thread allocation * Furi: hide dangerous heap tracking levels in release build * Input: fix non-working debounce --- applications/services/cli/cli_commands.c | 96 +++++++++++++++---- .../desktop/scenes/desktop_scene_main.c | 7 ++ applications/services/input/input.c | 20 +++- applications/services/loader/loader.c | 9 +- .../settings/system/system_settings.c | 37 +++++++ firmware/targets/f7/api_symbols.csv | 4 +- .../targets/f7/furi_hal/furi_hal_resources.h | 2 +- firmware/targets/f7/furi_hal/furi_hal_rtc.c | 16 +++- .../targets/furi_hal_include/furi_hal_rtc.h | 11 +++ furi/core/thread.c | 9 +- 10 files changed, 186 insertions(+), 25 deletions(-) diff --git a/applications/services/cli/cli_commands.c b/applications/services/cli/cli_commands.c index 239434b7..64ff8ef7 100644 --- a/applications/services/cli/cli_commands.c +++ b/applications/services/cli/cli_commands.c @@ -7,6 +7,7 @@ #include #include #include +#include // Close to ISO, `date +'%Y-%m-%d %H:%M:%S %u'` #define CLI_DATE_FORMAT "%.4d-%.2d-%.2d %.2d:%.2d:%.2d %d" @@ -192,6 +193,83 @@ void cli_command_log(Cli* cli, FuriString* args, void* context) { furi_stream_buffer_free(ring); } +void cli_command_sysctl_debug(Cli* cli, FuriString* args, void* context) { + UNUSED(cli); + UNUSED(context); + if(!furi_string_cmp(args, "0")) { + furi_hal_rtc_reset_flag(FuriHalRtcFlagDebug); + loader_update_menu(); + printf("Debug disabled."); + } else if(!furi_string_cmp(args, "1")) { + furi_hal_rtc_set_flag(FuriHalRtcFlagDebug); + loader_update_menu(); + printf("Debug enabled."); + } else { + cli_print_usage("sysctl debug", "<1|0>", furi_string_get_cstr(args)); + } +} + +void cli_command_sysctl_heap_track(Cli* cli, FuriString* args, void* context) { + UNUSED(cli); + UNUSED(context); + if(!furi_string_cmp(args, "none")) { + furi_hal_rtc_set_heap_track_mode(FuriHalRtcHeapTrackModeNone); + printf("Heap tracking disabled"); + } else if(!furi_string_cmp(args, "main")) { + furi_hal_rtc_set_heap_track_mode(FuriHalRtcHeapTrackModeMain); + printf("Heap tracking enabled for application main thread"); +#if FURI_DEBUG + } else if(!furi_string_cmp(args, "tree")) { + furi_hal_rtc_set_heap_track_mode(FuriHalRtcHeapTrackModeTree); + printf("Heap tracking enabled for application main and child threads"); + } else if(!furi_string_cmp(args, "all")) { + furi_hal_rtc_set_heap_track_mode(FuriHalRtcHeapTrackModeAll); + printf("Heap tracking enabled for all threads"); +#endif + } else { + cli_print_usage("sysctl heap_track", "", furi_string_get_cstr(args)); + } +} + +void cli_command_sysctl_print_usage() { + printf("Usage:\r\n"); + printf("sysctl \r\n"); + printf("Cmd list:\r\n"); + + printf("\tdebug <0|1>\t - Enable or disable system debug\r\n"); +#if FURI_DEBUG + printf("\theap_track \t - Set heap allocation tracking mode\r\n"); +#else + printf("\theap_track \t - Set heap allocation tracking mode\r\n"); +#endif +} + +void cli_command_sysctl(Cli* cli, FuriString* args, void* context) { + FuriString* cmd; + cmd = furi_string_alloc(); + + do { + if(!args_read_string_and_trim(args, cmd)) { + cli_command_sysctl_print_usage(); + break; + } + + if(furi_string_cmp_str(cmd, "debug") == 0) { + cli_command_sysctl_debug(cli, args, context); + break; + } + + if(furi_string_cmp_str(cmd, "heap_track") == 0) { + cli_command_sysctl_heap_track(cli, args, context); + break; + } + + cli_command_sysctl_print_usage(); + } while(false); + + furi_string_free(cmd); +} + void cli_command_vibro(Cli* cli, FuriString* args, void* context) { UNUSED(cli); UNUSED(context); @@ -208,22 +286,6 @@ void cli_command_vibro(Cli* cli, FuriString* args, void* context) { } } -void cli_command_debug(Cli* cli, FuriString* args, void* context) { - UNUSED(cli); - UNUSED(context); - if(!furi_string_cmp(args, "0")) { - furi_hal_rtc_reset_flag(FuriHalRtcFlagDebug); - loader_update_menu(); - printf("Debug disabled."); - } else if(!furi_string_cmp(args, "1")) { - furi_hal_rtc_set_flag(FuriHalRtcFlagDebug); - loader_update_menu(); - printf("Debug enabled."); - } else { - cli_print_usage("debug", "<1|0>", furi_string_get_cstr(args)); - } -} - void cli_command_led(Cli* cli, FuriString* args, void* context) { UNUSED(cli); UNUSED(context); @@ -356,7 +418,7 @@ void cli_commands_init(Cli* cli) { cli_add_command(cli, "date", CliCommandFlagParallelSafe, cli_command_date, NULL); cli_add_command(cli, "log", CliCommandFlagParallelSafe, cli_command_log, NULL); - cli_add_command(cli, "debug", CliCommandFlagDefault, cli_command_debug, NULL); + cli_add_command(cli, "sysctl", CliCommandFlagDefault, cli_command_sysctl, NULL); cli_add_command(cli, "ps", CliCommandFlagParallelSafe, cli_command_ps, NULL); cli_add_command(cli, "free", CliCommandFlagParallelSafe, cli_command_free, NULL); cli_add_command(cli, "free_blocks", CliCommandFlagParallelSafe, cli_command_free_blocks, NULL); diff --git a/applications/services/desktop/scenes/desktop_scene_main.c b/applications/services/desktop/scenes/desktop_scene_main.c index 654de44d..4f01ad5b 100644 --- a/applications/services/desktop/scenes/desktop_scene_main.c +++ b/applications/services/desktop/scenes/desktop_scene_main.c @@ -45,6 +45,13 @@ static void desktop_switch_to_app(Desktop* desktop, const FlipperApplication* fl return; } + FuriHalRtcHeapTrackMode mode = furi_hal_rtc_get_heap_track_mode(); + if(mode > FuriHalRtcHeapTrackModeNone) { + furi_thread_enable_heap_trace(desktop->scene_thread); + } else { + furi_thread_disable_heap_trace(desktop->scene_thread); + } + furi_thread_set_name(desktop->scene_thread, flipper_app->name); furi_thread_set_stack_size(desktop->scene_thread, flipper_app->stack_size); furi_thread_set_callback(desktop->scene_thread, flipper_app->app); diff --git a/applications/services/input/input.c b/applications/services/input/input.c index d1aef9e8..1d02df1e 100644 --- a/applications/services/input/input.c +++ b/applications/services/input/input.c @@ -1,5 +1,7 @@ #include "input_i.h" +// #define INPUT_DEBUG + #define GPIO_Read(input_pin) (furi_hal_gpio_read(input_pin.pin->gpio) ^ (input_pin.pin->inverted)) static Input* input = NULL; @@ -72,6 +74,10 @@ int32_t input_srv(void* p) { input->event_pubsub = furi_pubsub_alloc(); furi_record_create(RECORD_INPUT_EVENTS, input->event_pubsub); +#if INPUT_DEBUG + furi_hal_gpio_init_simple(&gpio_ext_pa4, GpioModeOutputPushPull); +#endif + #ifdef SRV_CLI input->cli = furi_record_open(RECORD_CLI); if(input->cli) { @@ -95,10 +101,16 @@ int32_t input_srv(void* p) { bool is_changing = false; for(size_t i = 0; i < input_pins_count; i++) { bool state = GPIO_Read(input->pin_states[i]); + if(state) { + if(input->pin_states[i].debounce < INPUT_DEBOUNCE_TICKS) + input->pin_states[i].debounce += 1; + } else { + if(input->pin_states[i].debounce > 0) input->pin_states[i].debounce -= 1; + } + if(input->pin_states[i].debounce > 0 && input->pin_states[i].debounce < INPUT_DEBOUNCE_TICKS) { is_changing = true; - input->pin_states[i].debounce += (state ? 1 : -1); } else if(input->pin_states[i].state != state) { input->pin_states[i].state = state; @@ -129,8 +141,14 @@ int32_t input_srv(void* p) { } if(is_changing) { +#if INPUT_DEBUG + furi_hal_gpio_write(&gpio_ext_pa4, 1); +#endif furi_delay_tick(1); } else { +#if INPUT_DEBUG + furi_hal_gpio_write(&gpio_ext_pa4, 0); +#endif furi_thread_flags_wait(INPUT_THREAD_FLAG_ISR, FuriFlagWaitAny, FuriWaitForever); } } diff --git a/applications/services/loader/loader.c b/applications/services/loader/loader.c index 712576e1..93171972 100644 --- a/applications/services/loader/loader.c +++ b/applications/services/loader/loader.c @@ -21,6 +21,13 @@ static bool FURI_LOG_I(TAG, "Starting: %s", loader_instance->application->name); + FuriHalRtcHeapTrackMode mode = furi_hal_rtc_get_heap_track_mode(); + if(mode > FuriHalRtcHeapTrackModeNone) { + furi_thread_enable_heap_trace(loader_instance->application_thread); + } else { + furi_thread_disable_heap_trace(loader_instance->application_thread); + } + furi_thread_set_name(loader_instance->application_thread, loader_instance->application->name); furi_thread_set_stack_size( loader_instance->application_thread, loader_instance->application->stack_size); @@ -306,7 +313,7 @@ static Loader* loader_alloc() { Loader* instance = malloc(sizeof(Loader)); instance->application_thread = furi_thread_alloc(); - furi_thread_enable_heap_trace(instance->application_thread); + furi_thread_set_state_context(instance->application_thread, instance); furi_thread_set_state_callback(instance->application_thread, loader_thread_state_callback); diff --git a/applications/settings/system/system_settings.c b/applications/settings/system/system_settings.c index 7661413d..dfce11a2 100644 --- a/applications/settings/system/system_settings.c +++ b/applications/settings/system/system_settings.c @@ -45,6 +45,31 @@ static void debug_changed(VariableItem* item) { loader_update_menu(); } +const char* const heap_trace_mode_text[] = { + "None", + "Main", +#if FURI_DEBUG + "Tree", + "All", +#endif +}; + +const uint32_t heap_trace_mode_value[] = { + FuriHalRtcHeapTrackModeNone, + FuriHalRtcHeapTrackModeMain, +#if FURI_DEBUG + FuriHalRtcHeapTrackModeTree, + FuriHalRtcHeapTrackModeAll, +#endif +}; + +static void heap_trace_mode_changed(VariableItem* item) { + // SystemSettings* app = variable_item_get_context(item); + uint8_t index = variable_item_get_current_value_index(item); + variable_item_set_current_value_text(item, heap_trace_mode_text[index]); + furi_hal_rtc_set_heap_track_mode(heap_trace_mode_value[index]); +} + static uint32_t system_settings_exit(void* context) { UNUSED(context); return VIEW_NONE; @@ -79,6 +104,18 @@ SystemSettings* system_settings_alloc() { variable_item_set_current_value_index(item, value_index); variable_item_set_current_value_text(item, debug_text[value_index]); + item = variable_item_list_add( + app->var_item_list, + "Heap Trace", + COUNT_OF(heap_trace_mode_text), + heap_trace_mode_changed, + app); + value_index = value_index_uint32( + furi_hal_rtc_get_heap_track_mode(), heap_trace_mode_value, COUNT_OF(heap_trace_mode_text)); + furi_hal_rtc_set_heap_track_mode(heap_trace_mode_value[value_index]); + variable_item_set_current_value_index(item, value_index); + variable_item_set_current_value_text(item, heap_trace_mode_text[value_index]); + view_set_previous_callback( variable_item_list_get_view(app->var_item_list), system_settings_exit); view_dispatcher_add_view( diff --git a/firmware/targets/f7/api_symbols.csv b/firmware/targets/f7/api_symbols.csv index d6e522a2..1ad5efa7 100644 --- a/firmware/targets/f7/api_symbols.csv +++ b/firmware/targets/f7/api_symbols.csv @@ -1,5 +1,5 @@ entry,status,name,type,params -Version,+,7.4,, +Version,+,7.5,, Header,+,applications/services/bt/bt_service/bt.h,, Header,+,applications/services/cli/cli.h,, Header,+,applications/services/cli/cli_vcp.h,, @@ -1260,6 +1260,7 @@ Function,+,furi_hal_rtc_deinit_early,void, Function,+,furi_hal_rtc_get_boot_mode,FuriHalRtcBootMode, Function,+,furi_hal_rtc_get_datetime,void,FuriHalRtcDateTime* Function,+,furi_hal_rtc_get_fault_data,uint32_t, +Function,+,furi_hal_rtc_get_heap_track_mode,FuriHalRtcHeapTrackMode, Function,+,furi_hal_rtc_get_log_level,uint8_t, Function,+,furi_hal_rtc_get_pin_fails,uint32_t, Function,+,furi_hal_rtc_get_register,uint32_t,FuriHalRtcRegister @@ -1272,6 +1273,7 @@ Function,+,furi_hal_rtc_set_boot_mode,void,FuriHalRtcBootMode Function,+,furi_hal_rtc_set_datetime,void,FuriHalRtcDateTime* Function,+,furi_hal_rtc_set_fault_data,void,uint32_t Function,+,furi_hal_rtc_set_flag,void,FuriHalRtcFlag +Function,+,furi_hal_rtc_set_heap_track_mode,void,FuriHalRtcHeapTrackMode Function,+,furi_hal_rtc_set_log_level,void,uint8_t Function,+,furi_hal_rtc_set_pin_fails,void,uint32_t Function,+,furi_hal_rtc_set_register,void,"FuriHalRtcRegister, uint32_t" diff --git a/firmware/targets/f7/furi_hal/furi_hal_resources.h b/firmware/targets/f7/furi_hal/furi_hal_resources.h index 8d53ec48..64e5e19f 100644 --- a/firmware/targets/f7/furi_hal/furi_hal_resources.h +++ b/firmware/targets/f7/furi_hal/furi_hal_resources.h @@ -10,7 +10,7 @@ extern "C" { #endif /* Input Related Constants */ -#define INPUT_DEBOUNCE_TICKS 30 +#define INPUT_DEBOUNCE_TICKS 4 /* Input Keys */ typedef enum { diff --git a/firmware/targets/f7/furi_hal/furi_hal_rtc.c b/firmware/targets/f7/furi_hal/furi_hal_rtc.c index 14ece946..c38cbfec 100644 --- a/firmware/targets/f7/furi_hal/furi_hal_rtc.c +++ b/firmware/targets/f7/furi_hal/furi_hal_rtc.c @@ -30,7 +30,8 @@ typedef struct { uint8_t log_reserved : 4; uint8_t flags; uint8_t boot_mode : 4; - uint16_t reserved : 12; + uint8_t heap_track_mode : 2; + uint16_t reserved : 10; } DeveloperReg; _Static_assert(sizeof(DeveloperReg) == 4, "DeveloperReg size mismatch"); @@ -224,6 +225,19 @@ FuriHalRtcBootMode furi_hal_rtc_get_boot_mode() { return (FuriHalRtcBootMode)data->boot_mode; } +void furi_hal_rtc_set_heap_track_mode(FuriHalRtcHeapTrackMode mode) { + uint32_t data_reg = furi_hal_rtc_get_register(FuriHalRtcRegisterSystem); + DeveloperReg* data = (DeveloperReg*)&data_reg; + data->heap_track_mode = mode; + furi_hal_rtc_set_register(FuriHalRtcRegisterSystem, data_reg); +} + +FuriHalRtcHeapTrackMode furi_hal_rtc_get_heap_track_mode() { + uint32_t data_reg = furi_hal_rtc_get_register(FuriHalRtcRegisterSystem); + DeveloperReg* data = (DeveloperReg*)&data_reg; + return (FuriHalRtcHeapTrackMode)data->heap_track_mode; +} + void furi_hal_rtc_set_datetime(FuriHalRtcDateTime* datetime) { furi_assert(datetime); diff --git a/firmware/targets/furi_hal_include/furi_hal_rtc.h b/firmware/targets/furi_hal_include/furi_hal_rtc.h index 361225fb..5ce12227 100644 --- a/firmware/targets/furi_hal_include/furi_hal_rtc.h +++ b/firmware/targets/furi_hal_include/furi_hal_rtc.h @@ -39,6 +39,13 @@ typedef enum { FuriHalRtcBootModePostUpdate, /**< Boot to Update, post update */ } FuriHalRtcBootMode; +typedef enum { + FuriHalRtcHeapTrackModeNone = 0, /**< Disable allocation tracking */ + FuriHalRtcHeapTrackModeMain, /**< Enable allocation tracking for main application thread */ + FuriHalRtcHeapTrackModeTree, /**< Enable allocation tracking for main and children application threads */ + FuriHalRtcHeapTrackModeAll, /**< Enable allocation tracking for all threads */ +} FuriHalRtcHeapTrackMode; + typedef enum { FuriHalRtcRegisterHeader, /**< RTC structure header */ FuriHalRtcRegisterSystem, /**< Various system bits */ @@ -79,6 +86,10 @@ void furi_hal_rtc_set_boot_mode(FuriHalRtcBootMode mode); FuriHalRtcBootMode furi_hal_rtc_get_boot_mode(); +void furi_hal_rtc_set_heap_track_mode(FuriHalRtcHeapTrackMode mode); + +FuriHalRtcHeapTrackMode furi_hal_rtc_get_heap_track_mode(); + void furi_hal_rtc_set_datetime(FuriHalRtcDateTime* datetime); void furi_hal_rtc_get_datetime(FuriHalRtcDateTime* datetime); diff --git a/furi/core/thread.c b/furi/core/thread.c index 8320a47e..201b47fe 100644 --- a/furi/core/thread.c +++ b/furi/core/thread.c @@ -122,9 +122,14 @@ FuriThread* furi_thread_alloc() { thread->output.buffer = furi_string_alloc(); thread->is_service = false; - if(furi_thread_get_current_id()) { + FuriHalRtcHeapTrackMode mode = furi_hal_rtc_get_heap_track_mode(); + if(mode == FuriHalRtcHeapTrackModeAll) { + thread->heap_trace_enabled = true; + } else if(mode == FuriHalRtcHeapTrackModeTree && furi_thread_get_current_id()) { FuriThread* parent = pvTaskGetThreadLocalStoragePointer(NULL, 0); if(parent) thread->heap_trace_enabled = parent->heap_trace_enabled; + } else { + thread->heap_trace_enabled = false; } return thread; @@ -243,14 +248,12 @@ FuriThreadId furi_thread_get_id(FuriThread* thread) { void furi_thread_enable_heap_trace(FuriThread* thread) { furi_assert(thread); furi_assert(thread->state == FuriThreadStateStopped); - furi_assert(thread->heap_trace_enabled == false); thread->heap_trace_enabled = true; } void furi_thread_disable_heap_trace(FuriThread* thread) { furi_assert(thread); furi_assert(thread->state == FuriThreadStateStopped); - furi_assert(thread->heap_trace_enabled == true); thread->heap_trace_enabled = false; }