j T
2006-Jun-03 09:49 UTC
[Nut-upsdev] RFC: allow HID subdriver's to register ups.conf settings
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(-
Peter Selinger
2006-Jun-03 19:14 UTC
[Nut-upsdev] RFC: allow HID subdriver's to register ups.conf
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@lists.alioth.debian.org > http://lists.alioth.debian.org/mailman/listinfo/nut-upsdev >
Reasonably Related Threads
- RFC: allow HID subdriver's to register ups.conf
- Re: [nut-commits] svn commit r801 - in branches/megatec-usb: . drivers
- Newpoint 200897 UPS
- Re: [nut-commits] svn commit r808 - in trunk: . drivers
- Voltage override in megatec and megatec-over-usb [was: Re: nut-2.0.5 megatec + Online Xanto]