enforce USER_INPUT_MAX_NUMBER_OF_COMMAND_ARGUMENTS in constructor

[dstroy0]

Enforce the limit in the constructor, throw compiler error.

[dstroy0]

Do you want to be able to pass a variadic number of arguments in a predefined range? I think you mentioned right now num_args functions as max_num_args I can add a min_num_args with a default of max_num_args. Then you can change min_num_args and it will accept args in the range of num_args >= min_num_args && num_args <= max_num_args

[dstroy0]

I don't understand the goal. What is the purpose of adding it?

[dstroy0]

Ok, so here's where I'm at. I made it so that if the user tries to specify more than USER_INPUT_MAX_NUMBER_OF_COMMAND_ARGUMENTS it wraps to that value. I can't figure out a way to throw a compiler error. We're pretty limited, I was trying to use static assert but I don't know what I need to do to make number_of_arguments const for static_assert.

[2bndy5]

I was never sure about what the goal for this issue was.

As for the Max/Min number of args, I think that comment is about #11, so I'll reply with an example snippet there.

[2bndy5]

Last time I looked into trying to purposely throw a compiler error in Arduino IDE, I wasn't able to do it.

[dstroy0]

so it looks like I can throw an abort somewhere in init, probably in addCommand

void UserInput::addCommand(CommandConstructor &command)
{
    CommandConstructor **cmd_head = &_commands_head_;
    CommandConstructor **cmd_tail = &_commands_tail_;
    size_t *cmd_count = &_commands_count_;
    command.next_command_parameters = NULL;
    if (*cmd_head == NULL)
    {
        *cmd_head = *cmd_tail = &command;
    }
    else
    {
        (*cmd_tail)->next_command_parameters = &command;
        *cmd_tail = &command;
    }
    (*cmd_count)++;
}

I would put a for loop right after (*cmd_count)++ to iterate through the parameters array and look at whatever we want to throw compile errors for, and if those conditions match we can set up some private prototypes like this with different error messages:

void _abort_ce( void ) __attribute__ ((error("constant exceeded")));
void _abort_neg( void ) __attribute__ ((error("negative value illegal")));

[dstroy0]

// abort prototypes
    void _abort_ce( void ) __attribute__((error("constant exceeded")));
    void _abort_neg( void ) __attribute__((error("negative value illegal")));
    // end abort prototypes
    // abort compile comparator
    void _abort_compile(Parameters& prm);

void UserInput::_abort_compile(Parameters& prm)
{
    if (__builtin_constant_p(prm.max_num_args) > UI_MAX_ARGS ||
        __builtin_constant_p(prm.num_args) > UI_MAX_ARGS)
    {
        _abort_ce();
    }
}

something like this

[dstroy0]

and this change to addCommand,

void UserInput::addCommand(CommandConstructor &command)
{
    CommandConstructor **cmd_head = &_commands_head_;
    CommandConstructor **cmd_tail = &_commands_tail_;
    size_t *cmd_count = &_commands_count_;
    command.next_command_parameters = NULL;
    if (*cmd_head == NULL)
    {
        *cmd_head = *cmd_tail = &command;
    }
    else
    {
        (*cmd_tail)->next_command_parameters = &command;
        *cmd_tail = &command;
    }
    (*cmd_count)++;
    Parameters prm;
    for (size_t i = 0; i < command._param_array_len; ++i)
    {
        memcpy_P(&prm, &(command.prm[i]), sizeof(prm));
        UserInput::_abort_compile(prm);
    }
}

ram use does not increase

[dstroy0]

I also think I dont need that size_t cmd_count, but that is a separate issue.

[dstroy0]

I tried a variety of things but still am not happy with how any of that worked... This was pretty fun though, I learned lots of neat new tricks.

[dstroy0]

I guess I have been overthinking this, I'm about to push a change that is the start of the Parameters error checking framework. It checks input Parameters structs before they are added to the linked-list, in addCommand(), if they're erroneous the command is rejected and the process continues.

[dstroy0]

I had to compromise the compiler error, instead we just reject commands with incorrect Parameters and direct the user to the error.

[2bndy5]

We also do this in the RF24 lib. If an invalid input is provided, we use any of the following:

• a default value
• only the applicable bits from the input
• a clamped value based on the input

Nonetheless, we have to suppress what would normally call for a thrown exception.

Maybe I should've mentioned this earlier, but I had hope that your fresh pair of eyes could find something new or something we missed.

[dstroy0]

I went pretty deep but I don't really know what I'm doing. I tried a small implementation of make_index_sequence, because I can template in and do static asserts. If you want to give that a try I can put it back in src/utility/

The information that is output now is very useful to users, points you right to where the Parameters error is.

[dstroy0]

I uploaded index_sequence.h to src/utility/

[dstroy0]

This is how progmem uses it:

template<size_t N>
struct progstr
{
    constexpr progstr(const char (&str)[N])
        : progstr(str, make_index_sequence<N> {}) {}

    template<size_t... Is>
    constexpr progstr(const char (&str)[N], index_sequence<Is...>)
        : _str{str[Is]...} {}

    constexpr operator const char*() const {
        return _str;
    }

    const char _str[N] PROGMEM;
};

[dstroy0]

I know we can use static_assert with constexpr, I just cant figure out the syntax to make it work.

like, how do I wrap this up in a way so that init for Parameters is still straightforward?

We can require a begin or a setup:

constexpr bool test(int i) 
{
    return i < 42;
}

void InputHandler_setup() 
{
  static_assert(test(43), "The test failed!");
}

struct Parameters
{
    void (*function)(UserInput *); ///< function pointer
    char command[UI_MAX_CMD_LEN];  ///< command string array
    uint16_t command_length;       ///< command length in characters
    uint8_t depth;                 ///< command tree depth
    uint8_t sub_commands;          ///< how many subcommands does this command have
    UI_ARG_HANDLING argument_flag; ///< argument handling flag
    uint8_t num_args;              ///< minimum number of arguments this command expects
    uint8_t max_num_args;          ///< maximum number of arguments this command expects
    UITYPE _arg_type[UI_MAX_ARGS]; ///< argument type array
};

[dstroy0]

I'm closing this again.