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

[Arjen de Korte]

> Author: aquette
> Date: Thu Jun 21 07:43:46 2007
> New Revision: 971
>
> Log:
> fix communication lost status handling
>
> Modified:
>    trunk/ChangeLog
>    trunk/drivers/usbhid-ups.c
>
> Modified: trunk/ChangeLog
> ==============================================================================
> --- trunk/ChangeLog	(original)
> +++ trunk/ChangeLog	Thu Jun 21 07:43:46 2007
> @@ -1,3 +1,8 @@
> +Thu Jun 21 07:07:25 UTC 2007 / Arnaud Quette <aquette.dev at gmail.com>
> +
> + - usbhid-ups: improve the handling and reporting of the communication
> +   lost status. Previously, these were not reported anymore.
> +
>  Web Jun 20 22:14:43 UTC 2007 / Carlos Rodrigues
> <carlos.efr at mail.telepac.pt>
>
>   - drivers/megatec.c, drivers/megatec_usb.c: reverted changes to
>
> Modified: trunk/drivers/usbhid-ups.c
> ==============================================================================
> --- trunk/drivers/usbhid-ups.c	(original)
> +++ trunk/drivers/usbhid-ups.c	Thu Jun 21 07:43:46 2007
> @@ -987,7 +987,7 @@
>
>  static void reconnect_ups(void)
>  {
> -	if (hd == NULL)
> +	while (hd == NULL)
        ^^^^^^^^^^^^^^^^^^
If the communication to the UPS is lost (and can't be established again),
it will stay in this loop forever and therefor never leave the
upsdrv_updateinfo() function. This is bad for two reasons:

#1 The driver won't respond to a termination signal anymore, since the
exit_flag (that is set by a SIGTERM for instance) is not evaluated in
upsdrv_updateinfo().

#2 The driver won't respond to the server either, so it will appear (and
be flagged) dead, while the actual problem is in the connection to the
UPS. People might try to restart it, which isn't going to work due to #1
mentioned above.

We warn for this behaviour in 'docs/new-drivers.txt':

---8<---

upsdrv_updateinfo
-----------------

Poll the hardware, and update any variables that you care about
monitoring.  Use dstate_setinfo() to store the new values.

Do at most one pass of the variables.  You MUST return from this
function or upsd will be unable to read data from your driver.  main
will call this function at regular intervals.

If your UPS hardware is slow or brain-damaged, you might use an
odd/even approach to this function.  Half the time, set a flag and
return.  The other half, clear the flag and actually do the poll.

--->8---

Best regards, Arjen

>  	{
>  		upsdebugx(2, "==================================================");
>  		upsdebugx(2, "= device has been disconnected, try to reconnect =");
> @@ -1001,8 +1001,12 @@
>  		udev = NULL;
>  #endif
>
> -	  if ((hd = HIDOpenDevice(&udev, &curDevice, reopen_matcher,
> MODE_REOPEN)) == NULL)
> -		dstate_datastale();
> +		if ((hd = HIDOpenDevice(&udev, &curDevice, reopen_matcher,
> MODE_REOPEN)) == NULL) {
> +			dstate_datastale();
> +
> +			/* Wait a bit before retrying... */
> +			sleep(5);
> +		}
>  	}
>  }
>
>
> _______________________________________________
> nut-commits mailing list
> nut-commits at lists.alioth.debian.org
> http://lists.alioth.debian.org/mailman/listinfo/nut-commits
>
>

-- 
Eindhoven - The Netherlands
Key fingerprint - 66 4E 03 2C 9D B5 CB 9B  7A FE 7E C1 EE 88 BC 57