Nut upsdev - Jun 2007 - Fwd: Megatec - modem control lines [impact to megatec_usb]

Hi,

Sorry for the forwarding. I sent this with the wrong "from" address
and got rejected by the list.

---------- Forwarded message ----------
From: Carlos Rodrigues <cefrodrigues at mail.telepac.pt>
Date: Jun 18, 2007 9:34 PM
Subject: Re: Megatec - modem control lines [impact to megatec_usb]
To: Arjen de Korte <nut+devel at de-korte.org>
Cc: ups-dev-list <nut-upsdev at lists.alioth.debian.org>


On 6/18/07, Carlos Rodrigues <carlos.efr at mail.telepac.pt>
wrote:
> On 6/18/07, Arjen de Korte <nut+devel at de-korte.org> wrote:
> > So far, the megatec driver doesn't do anything with the modem
control
> > lines at all. This may be OK if these are indeed not used by the
> > interface (for instance if only TX and RX are wired). But if they are
> > used to provide cable power this is not right. The build in NUT
> > functions don't do anything with the modem control lines and will
leave
> > them in the state they are in before the driver started. If that
happens
> > to be the right state, fine, but if they are not, we have a problem.
> > Therefor, I want to propose the following patch:
> >
> [snip]
> >
> > This should be backwards compatible, since we never did anything with
> > these lines before, so the state would have been 'random'
until now.Note
> > that at power up, these often will be cleared so usually they will be
> > cleared, but if the port had been used before by other applications,
the
> > state they're in is undefined.
> >
> > The least this patch will do, is to define a state for the modem
control
> > lines.
>
> Well, in principle I don't have a problem with this. I'll test it
> later at night.
Ok, I've tested it and it seems ok. However, this needs to be done in
a way that doesn't hurt megatec_usb, so I don't think it is a good
idea to add this to the trunk as is.

--
Carlos Rodrigues



-- 
Carlos Rodrigues
-------------- next part --------------
A non-text attachment was scrubbed...
Name: megatec-ioctl.diff
Type: application/octet-stream
Size: 996 bytes
Desc: not available
Url :
http://lists.alioth.debian.org/pipermail/nut-upsdev/attachments/20070618/b3b818d1/attachment.obj

Carlos Rodrigues wrote:
> Ok, I've tested it and it seems ok. However, this needs to be done in
> a way that doesn't hurt megatec_usb, so I don't think it is a good
> idea to add this to the trunk as is.
Of course. See what I came up with in the trunk. This should be flexible
enough.

Best regards, Arjen

On 6/19/07, Arjen de Korte <nut+devel at de-korte.org>
wrote:
> Trying to religiously avoid using #ifdef's is just as silly as the
blanket
> statement that goto's should be avoided (which is in our
> docs/developers.txt document and rightfully ignored by quite a few among
> us). If you intend to compile the same source file more than once with
> different defines, please be considerate with your fellow developers and
> let them at least know what your intentions are by placing the #ifdef's
> where they are used and don't hide them in a header file.
That's why I said "mostly".

I don't think there's a loss of readability there (nor gain), simply
because what matters is mostly seeing what's happening there and not
if the call is going to be ifdef'd out in the serial driver or just
implemented as an empty function. I (mostly) don't like ifdefs mixed
with code because I don't like to be distracted by compilation issues
when looking at functional issues.

But, like I said, I "mostly" don't like ifdefs. For the particular
case of those ioctl calls, I'm not seeing another way. I don't like it
much, but I guess that's pretty much the solution.
> Not surprisingly, all USB drivers have this same set of arguments which
> don't make sense for serial drivers. Likewise, there are arguments that
> are needed for serial drivers ('port' for instance), which are
irrelevant
> for USB drivers. I'm very much in favor of defining variables that are
> interface specific and not driver specific by default. So the set of
> regular expressions that are used for USB connected drivers are put in
> there, while the 'port' parameter would be used for serial
connected ones.
> By doing so, only the parameters that are really driver specific would be
> defined in the upsdrv_makevartable(). Would that be worthwile to
> investigate further?
I agree. However, megatec.c only defines driver specific variables,
and megatec_usb.c also has the need to specify at least one specific
variable besides the ones that could be considered USB-generic.

-- 
Carlos Rodrigues