dstroy0 / InputHandler

Arduino input handler
https://dstroy0.github.io/InputHandler/
GNU General Public License v3.0
1 stars 0 forks source link

Wildcard commands #38

Closed dstroy0 closed 2 years ago

dstroy0 commented 2 years ago

Wildcards should be allowed in commands, Wildcard use will come with a performance hit depending on where the wildcard is located, or how many are in the command separated by match sequences.

The way I will implement it is as follows: let wildcard char = '*'

let command = "**" no performance hit, match any this strlen

let command = "**BBCCDDEE" no performance hit, one memcmp from command[0 + 2]

let command = "AA**CCDDEE" performance hit, 2 memcmp

let command = "*BCCDE" performance hit, 3 memcmp

2bndy5 commented 2 years ago

Could there also be an option to disable this? What if someone wants their command to be test*case*1, and they don't want to suffer a performance hit for literal * chars.

2bndy5 commented 2 years ago

I'm also assuming that the greedy ** glob pattern (which consumes some punctuation chars also) is not applied here - each * is a single char.

Anything more complex might just be bloat for microcontroller platforms.

2bndy5 commented 2 years ago

let command = "**" no performance hit, match any this strlen

What about conflicts? Imagine when a user sets up 2 cmds:

  1. for test cmd
  2. for any 4 letter cmd (like ****).

In that case I would imagine the wildcard pattern would have a lesser comparison priority than the explicit cmd (allowing test to match before **** does).


Sorry for polluting the thread, but this is a first class 🥇 feature you thought up!

2bndy5 commented 2 years ago

Oh! What if there are 2 cmds with wildcards that match the same cmd? I would hope to trigger both cmds if possible.

Example

dstroy0 commented 2 years ago

I think what will be happening behind the scenes is a bit flag array strlen of the command, which is what I will be switching all true/false bool arr to while adding this feature. So we have a bit flag per char of the command string and we can & it

If you want to launch two functions with the same arg set just pass them the UserInput pointer inside of the target, I’ll make a new method getArgument(size_t) that will return a pointer to the arg if one exists else NULL, so you can iterate over that arg set which will remain in scope for the sub functions in the target wrapper however you want. Please open a new issue for this feature request so that I don’t forget!

dstroy0 commented 2 years ago

two functions receive same arg set:

void uc_cmd(UserInput *inputProcess) // single arg set in target wrapper scope
{
   func1(inputProcess); // iterate args with getArgument(n)
   func2(inputProcess); // iterate same args with getArgument(n)
} // arg set goes out of scope
dstroy0 commented 2 years ago

So, we can add a switch flag to Parameters that is either contains_wildcards or no_wildcards .. I'm not stuck on those names if you've got better ones.. and a pointer to a binary array the len of the command string literal cmd_wildcard_pattern_bin_arr .. again not stuck on the name.. then we know the len of the binary array is strlen(command)

OR

we just add a pointer cmd_wildcard_pattern_bin_arr to Parameters and if THAT is not initialized as NULL then we know the command has wildcards and we need to compute the memcmp ranges, which we would do inside of addCommand.

Which is where the mIndex method will come in handy, we can store the memcmp ranges in a flat array so the performance hit should be n+1.

dstroy0 commented 2 years ago

Adding in that behavior would preserve the ability to match '*' as part of the command string. Separately I think addCommand should warn users that are making wildcard commands that they have something other than '*' in a char position if they defined it as a wildcard char. To help detect any data entry mistakes, make troubleshooting deployments easier.

2bndy5 commented 2 years ago

Typically, I tend to use "is" or "has" as prefix to boolean variable names. I think has_wildcard would be shortest and concise.

cmd_wildcard_pattern_bin_arr to Parameters and if THAT is not initialized as NULL then we know the command has wildcards

I don't have a better name idea, but I want to point out that not all compilers will auto-initialize a variable upon declaration. This depends on chosen compiler optimizations. If you're going to rely on this approach, then you'll need to decide a good init value to use (as best practice).

dstroy0 commented 2 years ago

Everything is aggregate init, so it can't be left blank, you must pass a value, init is guaranteed.

dstroy0 commented 2 years ago

Ok so I will also pass in our wildcard char to UserInput ctor prm and if 'has_wildcard' is true, in addCommand that command will be memchr for wildcards and compute the command memcmp ranges in the memchr while loop. For commands uint8_t in len or less each range is 16-bits 2x0-254 len>uint8_t each memcmp range pair is 32-bits. any uchar value wildcard should work, which is the desired behavior.

I think it will just require a slight modification to readCommandFromBuffer, to add in an if cmd.has_wildcards == true and for loop through the memcmp ranges.

readCommandFromBuffer needs to be split, several subfuncs and setup. Probably a _getCommand or something.

dstroy0 commented 2 years ago

Very simple scan method, if the memcmp range pair is equal then the memcmp is 1 char wide

void UserInput::_calcCmdMemcmpRanges(CommandParameters& prm)
{
    if (prm.has_wildcards == true)
    {
        IH_wcc wcc;
        size_t cmd_str_pos = 0;
        uint8_t memcmp_ranges[32]{};
        uint8_t memcmp_ranges_idx = 0;
        bool start_memcmp_range = true;        
        memcpy_P(&wcc, _input_prm_.pwcc, sizeof(wcc));                
        while (cmd_str_pos < prm.command_length)
        {            
            if (prm.command[cmd_str_pos] != wcc[0] && start_memcmp_range == true)
            {
                memcmp_ranges[memcmp_ranges_idx] = cmd_str_pos;
                memcmp_ranges_idx++;
                start_memcmp_range = false;
            }
            if (prm.command[cmd_str_pos] == wcc[0] && prm.command[cmd_str_pos - 1] != wcc[0]) // end memcmp range
            {
                memcmp_ranges[memcmp_ranges_idx] = cmd_str_pos - 1;
                memcmp_ranges_idx++;
                start_memcmp_range = true;
            }
            cmd_str_pos++;
        }
        if (memcmp_ranges_idx % 2 != 0) // memcmp ranges needs to be / 2
        {
            memcmp_ranges[memcmp_ranges_idx] = cmd_str_pos;
            memcmp_ranges_idx++;            
        }
        Serial.println(F("memcmp ranges"));
        for (size_t i = 0; i < memcmp_ranges_idx; ++i)
        {
            Serial.println(memcmp_ranges[i]);
        }
    }
}

output for "h*lpme*" command string: 0 0 2 5

I'm going to play with this for a little bit just to make sure it really does what I think it's doing. After the ranges are being calculated correctly, then the generated array can be copied into process memory.

dstroy0 commented 2 years ago

new command compare method:

bool UserInput::_compareCommandToString(CommandConstructor* cmd, CommandParameters& prm, size_t prm_idx, char* str)
{
    if ((bool)pgm_read_byte(&cmd->prm[prm_idx].has_wildcards) == true)
    {
        for (size_t i = 0; i < cmd->calc->num_memcmp_ranges; i = i + 2)
        {
            size_t size = ((size_t)abs((int)cmd->calc->memcmp_ranges_arr[i + 1] - (int)cmd->calc->memcmp_ranges_arr[i]) == 0)
                ? 1
                : (size_t)abs(cmd->calc->memcmp_ranges_arr[i + 1] - cmd->calc->memcmp_ranges_arr[i]);
            char* cmp_ptr = &str[cmd->calc->memcmp_ranges_arr[i]];
            if (memcmp_P(cmp_ptr, &(cmd->prm[prm_idx].command[cmd->calc->memcmp_ranges_arr[i]]), size) == true) // doesn't match
            {
                return false;
            }
        }
    }
    else
    {
        size_t cmd_len_pgm = pgm_read_dword(&(cmd->prm[prm_idx].command_length)); 
        if (memcmp_P(&str, cmd->prm[prm_idx].command, cmd_len_pgm) == true)
        {
            return false;
        }
    }
    return true;
}
dstroy0 commented 2 years ago

Just a little tweaking and the command compare method is working fine, I just need to decide how to allocate the memory for the memcmp ranges and wildcards should be functional after debugging. Probably sometime tomorrow unless I get busy with other stuff again.

dstroy0 commented 2 years ago

I'm intending to allow max wildcards to be command_len - 1 so there's an "anchor" for memcmp

dstroy0 commented 2 years ago

Maybe I should allow it and include warnings in the docs, and reject commands in addCommand if they don't follow the rules and warn the user?

If you make an all wildcard char command that is shorter in length than some of your other commands in the same process the longer commands will never match because of limitations in the process.

Not allowing all wildcard commands seems like a silly imposition.

dstroy0 commented 2 years ago

I think at root it's process-wide, and at subcommand level it's depth-wide. Edit: the all-wildcard problem

dstroy0 commented 2 years ago

prelim wildcard cmds just pushed. I need to change a couple things to make it work with all wildcard commands.

dstroy0 commented 2 years ago

all wcc commands are being handled correctly in this latest push, I'm going to make a wildcard command example to illustrate the caveats

dstroy0 commented 2 years ago

Feature is implemented