Can CommandParameters be nested?

[2bndy5]

Some UART Serial implementations use a prefixed "address" byte followed by a slave device command (conditionally followed by command args). Generally this is done to mimic I2C protocol. This is rather similar to how CLI interfaces handle sub commands:

git pull
git submodule add https://github.com/nRF24/RF24Log.git
git submodule update --init

The the above is implemented as nested commands in which

• the first command is always git
  • the sub-commands would be pull, add, and submodule
    • command args are --init (optional) and https://github.com/nRF24/RF24Log.git (required)

I'm not worried about the Posix style prefix -- for optional commands' args here. I just used the git CLI as an example of nested commands.

---

Alternately, the command prefix could be specified as part of the command, but this would lead to repetitively occupying memory with identical bytes.

[2bndy5]

Maybe the UI_ARGUMENT_FLAG_ENUM could use a command_type_array to do this? I'm not fluent with all the recent changes yet.

[dstroy0]

I think some sort of templating will put us on the right track. one template with the function pointer and a variable number of

    char command[USER_INPUT_MAX_COMMAND_LENGTH];
    uint16_t command_length;
    UI_ARGUMENT_FLAG_ENUM argument_flag;
    uint8_t num_args;
    uint8_t max_num_args;
    UITYPE _arg_type[USER_INPUT_MAX_NUMBER_OF_COMMAND_ARGUMENTS];

as sub templates

would introduce the requested functionality

[2bndy5]

Have you ever used inheritance or polymorphism? I just had the idea that sub-commands could be declared like arguments. This is building of my idea to abstract the behavior of validateUserInput() and getToken() functions into polymorphic objects of a base class UITYPE.

Its hard to put this inheritance idea into words, and a pseudo coded snippet may not actually capture the idea of abstracting the methods' behavior (plus it would be a long snippet). Not to mention, I'm not still not very fluent with the current source, but the inheritance idea would beckon a larger overhaul of the internal API.

[dstroy0]

I've got the nested structs working,I just want to add an enumeration or something to make it clear like CommandParameters When dealing with the template param

But the struct is built like: function ptr Parameters opt[variadic] num_subcommands

I'm trying to slim down how much program space each Parameters struct uses as well. Having every member fixed width is wasteful of pgm space.

[2bndy5]

I'm working up a rough draft of my idea for inherited arg types. I'm doing my best to limit memory allocation as well.

[dstroy0]

I have not intentionally used polymorphism, I didn't know what that was I had to google it.

[2bndy5]

Yeah, I kinda figured that was the case because polymorphism is covered as a more advanced topic for a bachelor degree in computer science.

[2bndy5]

I'm also trying to see what this would look like without the assumption that everything is a c-str (which is causing a lot more classes of inherited input types). Also, I'm trying to keep it compatible with C++ iostream lib. This rough draft will be a doozy to review... I may end up starting my own repo for it.

Is one of goals of this project to specifically handle c-str input buffers?

[dstroy0]

My latest commit introduced the functionality, I was just checking that we can still access our PGM variables.

const Parameters help_param[2] PROGMEM =
{
  { // command
    "help",       // command string
    4,            // command string characters
    no_arguments, // argument handling
    0,            // minimum expected number of arguments
    0,            // maximum expected number of arguments
    /*
      UITYPE arguments
    */
    {
      UITYPE::NO_ARGS // use NO_ARGS if the function expects no arguments
    }
  },  // end command
  {   // begin first subcommand
    "in",       // command string
    2,            // command string characters
    single_type_argument, // argument handling
    1,            // minimum expected number of arguments
    1,            // maximum expected number of arguments
    /*
      UITYPE arguments
    */
    {
      UITYPE::UINT8_T // use NO_ARGS if the function expects no arguments
    }
  }
};
const CommandParameters _help_(uc_help, 1, help_param);
CommandConstructor uc_help_(&_help_); //  uc_help_ has a command string, and function specified

only ListCommands is working currently, but it shows the ptr structure for accessing params. I verified that adding n params does not increase ram use beyond what adding the constructor to CommandParameters did globally. It counts as dynamic init so it doesn't go into PGM anymore.

[dstroy0]

CommandParameters can maybe get rolled into CommandConstructor?

[dstroy0]

I rolled CommandParameters into CommandConstructor.

[dstroy0]

I moved the function launching logic to its own function, launchLogic with the intention of reducing duplicate code.

[dstroy0]

subcommand logic is working, error output is currently incorrect.

[dstroy0]

error output behavior is fixed for subcommands

[dstroy0]

I added a depth variable to Parameters and moved subcommands to Parameters, so we can build trees. I guess in this scenario sub_commands can be thought of as height? because it will be zero at the leaf farthest from the base?

[dstroy0]

I need to do some recursion here:

for (size_t j = 0; j < cmd->prm->sub_commands; ++j)
                {
                    memcpy_P(&prm, &(cmd->prm[j + 1]), sizeof(prm)); // move working subcommand variables into ram
                    if (strcmp(data_pointers[tokens], prm.command) == 0)  // if the subcommand matched
                    {
                        subcommand_matched = true; // clear subcommand match error
                        // try to launch target function
                        launchLogic(cmd, prm, data, len,
                                    all_arguments_valid,
                                    data_index, match,
                                    input_type_match_flag);
                        failed_on_subcommand = j + 1; // set error index
                        break;
                    }
                }

To get to the furthest leaf.

Maybe adding an id to parameters will make error checking easier? But that's one more layer of complexity.

[2bndy5]

This is all more complex than I'd originally imagined.

[dstroy0]

I am pretty amazed that it works.

[dstroy0]

Some sort of depth first search would be faster on very large commands but I don't think we need to do that for embedded systems.

[2bndy5]

Depth search isn't a good idea for MCUs because it usually involves large memory allocation to handle the entire tree (unless I'm remembering that class lesson wrong). This is the classic debate of memory usage vs CPU cycles.

[2bndy5]

I'm still debugging my revision (using inheritance for arg types), but I've been consistently seeing

Checking size .pio\build\adafruit_itsybitsy_m4\firmware.elf
Advanced Memory Usage is available via "PlatformIO Home > Project Inspect"
RAM:   [          ]   1.4% (used 2756 bytes from 196608 bytes)
Flash: [          ]   2.4% (used 12544 bytes from 524288 bytes)

Although, the example I'm testing is very simple - expected cmd is

test <raw_uint8_t>[ <c-str_uint>]

[dstroy0]

The nested example on my branch with 3 commands and 512 byte output buffer uses 846 bytes of ram and 15k pgm space on avr. There is a lot of room for pgm space reduction and performance optimization in my branch.

[dstroy0]

This is way more useful than what I initially envisioned.

[dstroy0]

Should subcommands have their own function pointers? I think yes, but I wanted to ask what you think. That change would move the function pointer out of ram and into progmem, and let subcommands launch the same function as the parent or a different one entirely, which would sort of let you group similar interfaces together.

[2bndy5]

I suppose it could be the same function pointer, but the callback would need to be able to know what command it is handling (in an easy manner).

[dstroy0]

The way it works now, whatever subcommand it terminates at, it only looks at that subcommand args and it would only look at that subcommand function pointer. It only has access to one Parameters container at a time.

[2bndy5]

Without testing it thoroughly, I prefer the way it is now (from a user perspective). It seems like you'd be putting extra work on the user if all subcommands shared the same function ptr.

[dstroy0]

You're right. Now I'm working through the subcommand search in pseudocode, it works fine if it's a flat structure but not if it is branched because it lacks a comparator, if it were represented by a coordinate system it loses track of where it is in the X plane. I think it just needs a buffer with the previous Parameters' command string, and then if that matches we load the new Parameters' command into the buffer if it has subcommands, decrement the depth.... on and on

[dstroy0]

let the command string = "command subcommand subcommand subcommand", a depth of 3. We should:

//pseudo
Parameters current_parameters
char buffer
for (i = depth; i > depth; --i)  // dig
  for (j = 0; j < parameter_array_elements; ++J) // through the command list
    if (subcommand_match && current_parameters != have_a_subcommand)
      launchLogic()
      break
   else if (subcommand_match && current_parameters == have_a_subcommand)
     memcpy buffer current_parameters.command

[dstroy0]

I need to poke at it more but this feels close to right.

[dstroy0]

I'm thinking that reading in the whole input at once would be faster than nibbling tokens, and make this search behavior more obvious.

[dstroy0]

It would make a lot of stuff vestigial, that could get rolled into existing behavior or factored out.

[dstroy0]

The IH_input_behavior_refactor branch is where I'm going to work on those changes.

[dstroy0]

I'm debugging the subcommand search in launchLogic

[dstroy0]

Fixed.