[Nut-upsdev] [nut-commits] svn commit r1156 - in trunk: . drivers

[Arjen de Korte]

Carlos Rodrigues wrote:

>> Sure. As it used to be, the detection phase would try to get a status from
>> the UPS five times. If it got three replies (or more), it would consider
>> this a positive identification. After that, it would try to read a status
>> once more and if that failed, it would abort. This was suboptimal for two
>> reasons:
>>
>> 1) It is unreasonable to allow 2 out of 5 tries to fail in the detection
>> phase, but terminate unconditionally if the sixth try failed.
> 
> Not really, because the point isn't getting a positive response, is
> knowing if we always get a consistent response, and get an idea about
> how reliabile is the communication with the UPS. And to know this, we
> need a _fixed_ number of samples. If the idea were just to check if
> there's a supported UPS on the other side of the cable, a single
> success would be enough.

I might agree with you, if the driver didn't have the following message
which is printed if there were less than three successful detections:

	"The UPS is supported, but the connection is too unreliable.
	 Try checking the cable for defects."

The change in the detection sequence I made has *no* impact on the
outcome of the detection. That's my point. But basically, it is a side
effect of finding a way to guarantee that the last Q1 command that is
used in the detection, is successful so that we can use it later on.
Before I started hacking this driver, that could not be guaranteed and
it did lead to surprising behavior sometimes.

You seem to ignore the fact that the driver in its original state would
only retry the Q1 commands in upsdrv_initinfo() used in the detection
phase. I have seen it fail on the Q1 command that is used to read the
status (the driver has detected and agreed that a useable UPS is
connected at that time) and you can't explain to user that it fails
then. The same goes for the other commands.

It is a really bad idea not to retry commands if you suspect (or
sometimes even know) the connection is unreliable. Serial communication
is not error free and especially not on some devices that use this protocol.

> Having an idea of how reliable the communication is, in the least,
> would allow the user to know about it an not be surprised if his box
> suddenly shuts itself down one day because the driver lost
> communications with the UPS.

If the detection failed twice, most users would never know about that.
Startup messages are generally devnull'ed and we both know Joe User
doesn't read the logs unless explicitly told to do so. His box won't
shutdown suddenly, because that is not what's going to happen when the
driver loses communication. Instead, 'upsmon' will send out warning
messages at regular intervals. If Joe has configured his system to not
show any warnings on communication lost and doesn't read his logs, there
is nothing you and I can do about that.

>> 2) If the first three attempts to read the Q1 status are successful, the
>> outcome of the detection is already known, so reading two more is not
>> going to change the outcome anymore.
>>
>> If a device was detected previously, it will be detected in the same (or
>> less) attempts now. This hasn't changed. For devices that return a correct
>> answer every time (many do) this means that the number of Q1 queries is
>> cut in half.
> 
> Thats's an unnecessary optimization, and isn't fixing anything that's
> broken.

Optimizing might not be an issue, but the upsdrv_initinfo() *was* broken
 (otherwise I wouldn't have started messing with it anyway).

> Driver startup time isn't important, as long as it is kept
> reasonably short, which it already is (other drivers take much longer
> to initialize and even those never got any complaints from users).

The latter is not true. We have received numerous complaints about
startup of drivers (maybe not the 'megatec' driver), to the point where
we had to add code to 'upsd' to make it not fail on drivers taking too
long to startup.

Best regards, Arjen