I have made a widespread commit to flip the sense of the default value for the 'bool secure' argument in RegisterCommand() to be true rather than false. Very few instances of RegisterCommand() took advantage of the argument defaults because 'secure' defaulted to false when most instances needed true. In addition, the default for the 'max' arg was INT_MAX whereas most instances should have max = 0, so that default is also changed. And lastly, a new default value NULL was added for the 'execute' function pointer. It was not necessary to change any of the calls to RegisterCommand() other than those that used the secure = false default. However, I decided to go ahead and review all the instances in the code and change them to make maximal use of the arg defaults so that the existing code can serve as a useful pattern for the addition of new commands. In particular, for secure, non-terminal commands, such as the top-level "framework" commands, the model should simply be: RegisterCommand("name", "Title"); For secure, terminal (sub)commands that don't require any additional parameters, the model should be: RegisterCommand("name", "Title", execute); This model also applies if the command has children but the command itself wants to execute a default operation if no subcommand is specified. It is incorrect to specify min = 0, max = 1 to indicate an optional subcommand; that is indicated by the presence of the execute function at the same time as a non-empty children array. The default usage string will list all the children. Any command with required or optional parameters should provide a usage string hinting about the parameters in addition to specifying the min and max number of parameters: RegisterCommand("name", "Title", execute, "usage", min, max); I found several instances of commands with parameters that left the usage string empty. If you recognize some of these as yours, please add hints for the parameters in the usage string: cmd_ecu->RegisterCommand("list","Show OBDII ECU pid list",obd2ecu_list, "", 0, 1); cmd_re->RegisterCommand("list","List RE records",re_list, "", 0, 1); cmd_dbc->RegisterCommand("list","List RE DBC records",re_dbc_list, "", 0, 1); cmd_discover_list->RegisterCommand("changed","List changed records",re_list_changed, "", 0, 1); cmd_discover_list->RegisterCommand("discovered","List discovered records",re_list_discovered, "", 0, 1); cmd_test->RegisterCommand("strverscmp", "Test strverscmp function", test_strverscmp, "", 2, 2); The following one includes a usage string, but since the parameter is optional (indicated by min = 0), the usage should be "[<url>]", so I made that change: cmd_otaflash->RegisterCommand("http","OTA flash http",ota_flash_http,"<url>",0,1,true); The iMiEV and Kia Soul command hierarchies marked the top-level command as secure but all the subcommands as not secure. This does not make sense as the subcommands could not be reached if not enabled. Furthermore, a subcommand to unlock the doors should be secure indeed. This commit changes all the subcommands to be secure. Similarly, the 'status' subcommands for 'server v2' and 'server v3' were marked not secure, apparently with the intention of letting the status be examined without enabling, but the top-level commands were marked secure so the status command could not be reached. The status subcommands are now marked secure. -- Steve