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 >> >> >> >> >> >> >> >>