Nut upsdev - Jun 2006 - RFC: allow HID subdriver's to register ups.conf

j T wrote:
> >- allow the value to be NULL if a subdriver does not require
> >   initialization.
> 
> Am I right in thinking that this is simply a case of doing:
> 
> 	subdriver_t some_subdriver = {
> 		/* ... */
> 		some_format_serial,
> 		NULL,
> 	}
> 
> Rather than:
> 
> 	subdriver_t belkin_subdriver = {
> 		/* ... */
> 		belkin_format_serial,
> 		belkin_init,
> 	}
> in the subdrivers that don't need init()ing?
Yes.
 
> >- change the call to:
> >
> >   if (subdriver->init != NULL) {
> >      subdriver->init();
> >   }
> 
> As I've found out since I came up with the original idea  (see one of
my
> previous messages)  I can't call subdriver->function() from
makevartable as
> subdriver hasn't been given a sensible value at that point, instead,
I'm
> having to do:
> 
> 	for (i=0; subdriver_list[i] != NULL; i++) {
> 		subdriver_list[i]->extendvartable();
> 	}
> 
> So I assume that I should modify this to be:
> 
> 	for (i=0; subdriver_list[i] != NULL; i++) {
> 		if (subdriver[i]->init != NULL) {
> 			subdriver_list[i]->init();
> 		}
> 	}
This is a bad idea. It would mean that the variables of *all* the
subdrivers would be loaded, rather than just the correct subdriver.
Probably a better solution is to wait until the subdriver has been
determined, and then load the variables. But I don't know if this is
possible; it might break the driver higher up.
> As init() is more general-purpose than my extendvartable(), is it still ok 
> to have it be called from the end of upsdrv_makevartable()?
> 
> I was just thinking that, while it's reasonable to expect that the 
> subdriver's vartable additions would happen there (hence the name 
> "extendvartable"), it's not necessarily the most obvious
place for other
> kinds of initialisation to happen when a routine called
"makevartable" is
> getting called from main()...
> 
> Anyway, just a thought...
This is related to the question of when to define these variables. As
you pointed out, it cannot be done in upsdrv_makevartable(). So
perhaps it is more sensible to have a general init routine, and to
call it from within upsdrv_initups(), just after the subdriver has
been selected. 

Please keep the traffic on the mailing list.

-- Peter

Hi Peter

You see, this is the problem that I'm having; I *really* don't want to
be
polluting the vartable with all the options from all the different 
subdrivers, but the current design doesn't allow me to avoid this (atleast 
that I can tell).

Basically, main() does:

	makevartable()
	process command line (while loop)
	chroot, setup signals, etc
	initups()

Because main() processes the command line straight after it does 
makevartable(), there's no way to have the subdriver selected before the 
command line stuff happens. If the subdriver's init() is called from 
initups(), issuing a "-x wait" on the command line will always get
rejected
by the command line processing simply because the subdriver hasn't been 
init()ed at that point.

Also, the command line needs to have been processed before the initups() 
happens because it needs to know the regexs and so-on in order to match the 
relevent UPS.

This is the whole reason that it turned into a bit of a bodge... I rather 
hoped to avoid trying to sort the whole thing out; C isn't my forte, and I 
don't really feel confident enough in my ability to start trying to rework 
the main() routine.

Having said that, I did have one thought... what if each subdriver had it's 
own supplemental vartable?

I just need to work out how on earth to implement it (and use it from 
main())!!! ;-)

I might just have a look into this later today.

BTW; is it ok to use #ifdef preprocessor conditionals in NUT code? I don't 
remember reading anything in the developer notes against it, but I'd rather 
not go writing something using it if its unofficially tagged as being 
"yucky"!
>
>Please keep the traffic on the mailing list.
>
Shucks, did it manage to get sent without the mailing list CC'd in? I 
thought I'd caught it and modified the recipients... doh!


Jo Turner
  -)O(-

Hi Peter

That's a shame, cos I've got it working!

It's not exactly how I described earlier, but I'm rather pleased with it
:-)

And belkinunv still builds and works properly with the changes applied to 
main.c and main.h.

All the drivers normally built by "make all" build successfully using
GCC.

valgrind-ing both belkinunv and newhidups with the changes applied produces 
the same output as it does without them, so it doesn't look as though
I've
introduced any memory leaks, etc to any of the shared code.
--
With respect to providing command-line help on the different subdrivers, I 
was thinking of keeping them out of the main driver help. Instead, I was 
going to introducing a new "-x help=subdriver" option to newhidups:
- When "newhidups -h" where issued, you'd have a line like
"Get help on
subdriver-specific options : -x help=subdriver-name".
- When "newhidups -x help=subdriver-name" were issued, you would get 
help
relevent to the specified subdriver
I haven't implemented this yet though...
--
Anyway, I've attached a unified diff that contains all the changes necessary
to get subdriver options functional. The diff includes changes to belkin-hid 
to make it accept the same options as my revised belkinunv driver, and the 
tiny changes to the other HID driver .c files so that they shouldn't break 
(adding "NULL," at the end of the "vendor_subdriver"
struct).


Jo Turner
  -)O(-

-------------- next part --------------
A non-text attachment was scrubbed...
Name: subdrivers_have_options.diff
Type: text/x-patch
Size: 6975 bytes
Desc: not available
Url :
http://lists.alioth.debian.org/pipermail/nut-upsdev/attachments/20060605/e9e20ecf/subdrivers_have_options.bin