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

[Arjen de Korte]

> Yep, it works, but it means the main driver is being "tweaked" by the
> subdrivers instead of just delegating stuff to them, which is
> something I wanted to avoid.

I agree that this solution isn't very elegant, but essentially is not that
different from the way how the megatec_subdrv_makevartable() and
megatec_subdrv_banner() functions are dealt with now. Had I realized this
requires recompilation of megatec.c anyway this morning (not enough coffee
at that time), we could also have done this by enclosing the manipulation
of the modem control lines with

#ifndef	MEGATEC_SUBDRV
	int	dtr_bit = TIOCM_DTR;
	int	rts_bit = TIOCM_RTS;

	ioctl(upsfd, TIOCMBIS, &dtr_bit);
	ioctl(upsfd, TIOCMBIC, &rts_bit);
#endif

I don't quite understand the reason behind the manipulation of the
megatec_subdrv_* functions by the way. Wouldn't it be a lot easier to
follow if you would just enclose these functions with

#ifdef	MEGATEC_SUBDRV
	megatec_subdrv_makevartable();
#endif

At least for me, it would have been instantly clear that building the
megatec_usb driver requires recompilation of megatec.c too. Until now, I
assumed this was dealt with at link time, so the decision to control the
modem lines should be made at run time too, rather than during
compilation. Effectively, this was hidden by putting this in the header
file. ;-)

If we would loose this dependency by the way and made megatec_usb.c truly
generic (and preferably rename it serial_usb.c), we might be able to reuse
it in other drivers that were originally made for serial ports. If you
link a driver with serial.o, it is for a serial interface, while if you
link it with serial_usb.o it uses a USB interface. Not that I know of any
driver now that could benefit from this, but you get the idea.

> This would be easier if NUT provided some "ser_" function to set the
> lines. Then the subdriver only had to implement this function as a
> no-op, just like it does for the rest of the serial interface. This
> would even ease a port for non unix platforms, if someone ever wanted
> to do it.
>
> What do you think?

No doubt, that would be much cleaner. Unfortunately, this was never in the
set of 'ser_' functions so everybody just uses the ioctl() function. I
hesitate to create a ser_control() function, when the only driver that
would use it was 'megatec'. Note that the ioctl() function itself is
portable if you limit yourself to POSIX compliant compilers. If you don't,
you have far bigger problems to worry about since the remainder of the
codebase also assumes POSIX compliance, so you may need to rewrite a lot
of low level I/O functions.

> Another alternative is to create a pair of functions, to set state and
> to put it back, and then the subdrivers either implemented alternate
> versions doing "whatever", or just provided no-ops. It should be any
> more complex than the current scheme.

I don't care much for storing the state of the modem control lines and
restoring them on exiting. If an application uses these lines, it must set
them to the correct state itself. You can never rely on the fact that the
OS has set them for you, because there simply is no default. The reason
why I clear the DTR bit in the drivers I maintain is rather trivial. The
RS232 breakout box I use on my development system indicates the state of
this line (and all others too) by means of a bicolor LED. If this LED is
red, I know a driver is still running and I need to terminate it before
firing up a new one. If it is green, the port is free and I can fire one
up without worries. :-)

Best regards, Arjen