Not sure I have a strong opinion on this one. My initial reaction was option 1
but I can understand why some of the other options might be better.
> On Jun 25, 2018, at 4:07 AM, Michał Narajowski
> <michal.narajow...@codecoup.pl> wrote:
>
> Hi Paul,
>
> IMO Option 1 seems like the best option here. I don't think the special
> command processing code will require a lot of work. Of course, if you think
> that Option 2 is better for your use case you can implement it and submit a
> PR.
>
> BR
> Michał Narajowski
>
> pon., 25 cze 2018 o 04:10 p...@wrada.com <p...@wrada.com> napisał(a):
>
>> I'm checking with the group on how folks have done this in the past in
>> case I am missing something. Also proposing how I would solve the problem
>> looking for feedback.
>>
>> I have a module that you may want to instantiate more than once (create
>> two or more objects). For example suppose I have a device driver that may
>> instantiate multiple times but with different context.
>>
>> I’m trying to determine how I will provide shell commands for them. I have
>> a few options.
>>
>> 1. use the first argument of the shell command as the object instance
>> and queue the object onto some list that I can search in the command to
>> execute the command on the right object
>> 2. Try to modify the the shell command to include a void * somewhere in
>> the registration and have the shell commands pass it back during execution.
>> 3. Have the callback return the module name string which I can cast
>> back or search for combining the flexibility of #1 and #2
>>
>> In the first option I would queue my instance objects on a queue and then
>> search for them by name or number
>>
>> * foo 0 reset
>> * foo 1 reset
>>
>> When there is only once instance (most typical) then the command has the
>> burden of carrying the extra notation or I have to provide provisional
>> processing to do some default behavior when the number is left out of the
>> command. For this functionality I need to queue the objects which is not
>> currently necessary (its only a few bytes for an SLIST).
>>
>> In the second choice I could have the following changes to shell
>>
>> * include a void * in the shell_register that is passed in the
>> callback like
>> * typedef int (*shell_cmd_func_t)(int argc, char *argv[], void
>> *arg);
>>
>> * int shell_register(const char *shell_name, const struct shell_cmd
>> *shell_commands, void *arg)
>>
>> * shell_register(“sample_object0", cmd, (void*) &obj0)
>>
>> * shell_register(“sample_object1", cmd, (void*) &obj1)
>> * Modify the internal definition of the shell module to have
>> * struct shell_module {
>> * const char *name;
>> * const struct shell_cmd *commands;
>> * void *arg;
>> * };
>>
>> In this method the commands would look like this
>>
>> * foo0 reset
>> * foo1 reset
>>
>> Of course, the first instance of this could also just be called foo and
>> then the latter could have numbers
>>
>> * Foo reset
>> * foo1 reset
>> * foo2 reset
>>
>> NOTE: because of mempools, logs and stats I already keep a char *name in
>> the object to differentiate each instance (e.g. foo0 foo1 etc). So I
>> already have the means to pass the name.
>>
>> In the 3rd option, it would not change any of the APIs to register or any
>> of the internal structures , but will still require a change to the
>> callback to pass the string back to the caller like this
>>
>> * typedef int (*shell_cmd_func_t)(int argc, char *argv[], const char
>> *shell_name);
>>
>> And I could cast that back to my object using something like
>>
>> Struct foo {
>> /* blah blah */
>> char name[32];
>> };
>>
>> Struct foo *pf = (struct foo*) ( (uint8_t*) shell_name - (int) ((struct
>> foo *) 0)->name))
>>
>> Or I could keep them on a list and search the list for a matching string
>> in shell_name.
>>
>> In short
>>
>> * Option 1 — requires no change to shell API. Requires queueing
>> objects and some special command processing code
>> * Option 2 — simple to use, but requires API change and an extra void
>> * storage for each shell module
>> * Option 3 — more complicated to use, requires API change but does not
>> require extra storage for each shell module
>>
>>
>>
>>
>>
>>
>>
>>
