[Nut-upsdev] RFC: allow HID subdriver's to register ups.conf

[Peter Selinger]

Hi Jo,

this sounds reasonable, but I would change it as follows:

- call the function "init" instead of "extendvartable" -
  this could be used by subdrivers to do any necessary
  initializations. 

- it doesn't matter where in the struct it goes; the end is fine.

- allow the value to be NULL if a subdriver does not require
  initialization.

- change the call to:

  if (subdriver->init != NULL) {
     subdriver->init();
  }

-- Peter

j T wrote:
> 
> Hi
> 
> I'd like to know what you guys think to the following idea.
> 
> Please be aware, C/C++ are not exactly my strongpoint (couldn't be much 
> further from it actually..!), so I'm not sure whether this is all legal 
> code.
> 
> --
> 
> SYNOPSIS:
> 
> Allow a newhidups subdriver to register that it can handle settings in 
> ups.conf (and via -x on the command line), as per the normal serial drivers.
> 
> DESCRIPTION OF CHANGES:
> 
> - In newhidups.h, add an extra line to the subdriver_s struct;
> 
>                 void (*extendvartable)(void);
> 
>   so it becomes;
> 
>         struct subdriver_s {
>                 char *name;
>                 int (*claim)(HIDDevice *hd);
>                 usage_tables_t *utab;
>                 hid_info_t *hid2nut;
>                 int (*shutdown)(int ondelay, int offdelay);
>                 char *(*format_model)(HIDDevice *hd);
>                 char *(*format_mfr)(HIDDevice *hd);
>                 char *(*format_serial)(HIDDevice *hd);
>                 void (*extendvartable)(void);
>         };
> 
> - In the subdriver source, add an extra function, 
> subdriver_extendvartable(). I'm currently
>   working on belkin-hid.c, so it would have;
> 
>         void belkin_extendvartable(void) {
>                 addvar(VAR_FLAG, "wait", "Wait for AC power                  
>           ");
>                 addvar(VAR_VALUE,"wait", "Wait for AC power and battery 
> level ");
>                 /* ... */
>         }
> 
>   The other HID drivers, I guess, could simply have empty functions at the 
> moment.
> 
>   Next, modify the subdriver_t chunk (again, this is shown for the 
> belkin-hid) to be;
> 
>         subdriver_t belkin_subdriver = {
>                 BELKIN_HID_VERSION,
>                 belkin_claim,
>                 belkin_utab,
>                 belkin_hid2nut,
>                 belkin_shutdown,
>                 belkin_format_model,
>                 belkin_format_mfr,
>                 belkin_format_serial,
>                 belkin_extendvartable,
>         };
> 
> - In newhidups.c, at the end of upsdrv_makevartable(), add a single line 
> that does:
> 
>                 subdriver->extendvartable();
> 
>   to actually extend the table of configuration options for the specific 
> subdriver.
> 
> --
> 
> Assuming that this sounds like a good idea, I also have a query;
> 
> With respect to the subdriver_s struct, would it be better to have the "void 
> (*extendvartable) (void);" line at the end of the struct (as shown above), 
> or would some other position within the struct be more appropriate???
> 
> Thanks
> 
> Jo Turner
>   -)O(-
> 
> 
> 
> _______________________________________________
> Nut-upsdev mailing list
> Nut-upsdev at lists.alioth.debian.org
> http://lists.alioth.debian.org/mailman/listinfo/nut-upsdev
>