Nut upsdev - Jun 2011 - [nut-commits] svn commit r3104 - branches/apcsmart-dev/drivers

Citeren Michal Soltys <msoltyspl-guest at alioth.debian.org>:
> @@ -417,10 +414,12 @@
>  	if (flags & SER_CC) {
>  		iset = IGN_CCCHARS;
>  		aset = "";
> +		sec = 6; usec = 0;
>  	}
>  	if (flags & SER_CS) {
>  		iset = IGN_CSCHARS;
>  		aset = "";
> +		sec = 6; usec = 0;
>  	}
>
>  	memset(buf, '\0', buflen);
Please see the following comment on upsdrv_updateinfo from the  
new-drivers documentation:

   "Don't spent more than a couple of seconds in this function.
Typically
    five (5) seconds is the maximum time allowed before you risk that the
    server declares the driver stale. If your UPS hardware requires a
    timeout period of several seconds before it answers, consider returning
    from this function after sending a command immediately and read the
    answer the next time it is called."

It looks like you're setting a timeout of 6 seconds here, which puts  
your driver being declared stale by the server. Drivers are single  
threaded (like all of NUT at present) so hanging around this long,  
will mean that it won't respond to polls from the server in time.

If these commands really take this long to process, you really should  
split this up in multiple invocations of upsdrv_updateinfo (so send  
the command, return and process the reply the next time you enter).

Best regards, Arjen
-- 
Please keep list traffic on the list (off-list replies will be rejected)

On 11-06-28 21:08, Arjen de Korte wrote:
> Citeren Michal Soltys <msoltyspl-guest at alioth.debian.org>:
>
>> @@ -417,10 +414,12 @@
>> if (flags & SER_CC) {
>> iset = IGN_CCCHARS;
>> aset = "";
>> + sec = 6; usec = 0;
>> }
>> if (flags & SER_CS) {
>> iset = IGN_CSCHARS;
>> aset = "";
>> + sec = 6; usec = 0;
>> }
>>
>> memset(buf, '\0', buflen);
>
> Please see the following comment on upsdrv_updateinfo from the
> new-drivers documentation:
>
> "Don't spent more than a couple of seconds in this function.
Typically
> five (5) seconds is the maximum time allowed before you risk that the
> server declares the driver stale. If your UPS hardware requires a
> timeout period of several seconds before it answers, consider returning
> from this function after sending a command immediately and read the
> answer the next time it is called."
>
> It looks like you're setting a timeout of 6 seconds here, which puts
> your driver being declared stale by the server. Drivers are single
> threaded (like all of NUT at present) so hanging around this long, will
> mean that it won't respond to polls from the server in time.
>
> If these commands really take this long to process, you really should
> split this up in multiple invocations of upsdrv_updateinfo (so send the
> command, return and process the reply the next time you enter).
>
Those two commands happen only once before the actual loop is in effect, 
during the initialization stage - in upsdrv_initinfo(). It's the only 
place where such large delay is used. Note this is only the max possible 
delay, the read will return as quickly as possible (each command ends 
with LF) - in case of one of my models it takes roughly 3 seconds 
("capability set" command, "command set" is faster) - so I
wanted to be
on the safe side.

I persume the upper limit it's not a problem when upsdrv_initinfo() is 
considered ?