hidpp: fix properties for unreachable devices (v2)

This includes "is-present" and "state" (which will be marked "unknown").
"percentage" is not touched since it is still an indication of the
battery level, changing it to zero is not helpful.

Previously, properties were never updated because the refresh would
fail when the battery refresh request failed.

 - v2: fix invalid return value when an invalid parameter is passed to
   hidpp_device_is_reachable.

Signed-off-by: Peter Wu <lekensteyn@gmail.com>

3 files changed, 45 insertions, 12 deletions

diff --git a/src/linux/hidpp-device.c b/src/linux/hidpp-device.c
index 96c34ed..84e8375 100644
--- a/src/linux/hidpp-device.c
+++ b/src/linux/hidpp-device.c
@@ -152,6 +152,7 @@ struct HidppDevicePrivate
 	gboolean		 batt_is_approx;
 	HidppDeviceKind		 kind;
 	int			 fd;
+	gboolean		 is_present;
 };
 
 typedef struct {
@@ -524,6 +525,16 @@ hidpp_device_get_kind (HidppDevice *device)
 }
 
 /**
+ * hidpp_device_is_reachable:
+ **/
+gboolean
+hidpp_device_is_reachable (HidppDevice *device)
+{
+	g_return_val_if_fail (HIDPP_IS_DEVICE (device), FALSE);
+	return device->priv->is_present;
+}
+
+/**
  * hidpp_device_set_hidraw_device:
  **/
 void
@@ -615,8 +626,10 @@ hidpp_device_refresh (HidppDevice *device,
 				 * returned INVALID_SUBID) */
 				if (error_code == HIDPP10_ERROR_CODE_INVALID_SUBID) {
 					priv->version = 1;
+					priv->is_present = TRUE;
 				} else {
 					g_debug("Cannot detect version, unreachable device");
+					priv->is_present = FALSE;
 				}
 
 				/* do not execute the error handler at the end
@@ -626,8 +639,10 @@ hidpp_device_refresh (HidppDevice *device,
 				*error = NULL;
 				ret = TRUE;
 			}
-		} else
+		} else {
 			priv->version = msg.s.params[0];
+			priv->is_present = TRUE;
+		}
 
 		if (!ret)
 			goto out;
@@ -930,6 +945,15 @@ hidpp_device_refresh (HidppDevice *device,
 			}
 		}
 	}
+
+	/* when no error occured for the requests done by the following refresh
+	 * flags, assume the device present. Note that the is_present flag is
+	 * always set when using HIDPP_REFRESH_FLAGS_VERSION */
+	if (priv->version > 0 && refresh_flags &
+			(HIDPP_REFRESH_FLAGS_MODEL |
+			 HIDPP_REFRESH_FLAGS_BATTERY)) {
+		priv->is_present = TRUE;
+	}
 out:
 	/* do not spam when device is unreachable */
 	if (hidpp_is_error(&msg, &error_code) &&
@@ -937,6 +961,11 @@ out:
 		g_debug("HID++ error: %s", (*error)->message);
 		g_error_free(*error);
 		*error = NULL;
+		/* the device is unreachable but paired, consider the refresh
+		 * successful. Use is_present to determine if battery
+		 * information is actually updated */
+		ret = TRUE;
+		priv->is_present = FALSE;
 	}
 	if (name != NULL)
 		g_string_free (name, TRUE);
@@ -952,6 +981,7 @@ hidpp_device_init (HidppDevice *device)
 	HidppDeviceMap *map;
 
 	device->priv = HIDPP_DEVICE_GET_PRIVATE (device);
+	device->priv->is_present = FALSE;
 	device->priv->fd = -1;
 	device->priv->feature_index = g_ptr_array_new_with_free_func (g_free);
 	device->priv->batt_status = HIDPP_DEVICE_BATT_STATUS_UNKNOWN;
diff --git a/src/linux/hidpp-device.h b/src/linux/hidpp-device.h
index 3f249a8..2934606 100644
--- a/src/linux/hidpp-device.h
+++ b/src/linux/hidpp-device.h
@@ -84,6 +84,7 @@ void			 hidpp_device_set_index			(HidppDevice	*device,
 								 guint		 device_idx);
 void			 hidpp_device_set_enable_debug		(HidppDevice	*device,
 								 gboolean	 enable_debug);
+gboolean		 hidpp_device_is_reachable		(HidppDevice	*device);
 gboolean		 hidpp_device_refresh			(HidppDevice	*device,
 								 HidppRefreshFlags refresh_flags,
 								 GError		**error);
diff --git a/src/linux/up-device-unifying.c b/src/linux/up-device-unifying.c
index 005a132..989ac48 100644
--- a/src/linux/up-device-unifying.c
+++ b/src/linux/up-device-unifying.c
@@ -73,11 +73,9 @@ up_device_unifying_refresh (UpDevice *device)
 				    refresh_flags,
 				    &error);
 	if (!ret) {
-		if (error) {
-			g_warning ("failed to coldplug unifying device: %s",
-				   error->message);
-			g_error_free (error);
-		}
+		g_warning ("failed to coldplug unifying device: %s",
+			   error->message);
+		g_error_free (error);
 		goto out;
 	}
 	switch (hidpp_device_get_batt_status (priv->hidpp_device)) {
@@ -93,9 +91,15 @@ up_device_unifying_refresh (UpDevice *device)
 	default:
 		break;
 	}
+
+	/* if a device is unreachable, some known values do not make sense */
+	if (!hidpp_device_is_reachable (priv->hidpp_device)) {
+		state = UP_DEVICE_STATE_UNKNOWN;
+	}
+
 	g_get_current_time (&timeval);
 	g_object_set (device,
-		      "is-present", hidpp_device_get_version (priv->hidpp_device) > 0,
+		      "is-present", hidpp_device_is_reachable (priv->hidpp_device),
 		      "percentage", (gdouble) hidpp_device_get_batt_percentage (priv->hidpp_device),
 		      "state", state,
 		      "update-time", (guint64) timeval.tv_sec,
@@ -245,11 +249,9 @@ up_device_unifying_coldplug (UpDevice *device)
 				    HIDPP_REFRESH_FLAGS_MODEL,
 				    &error);
 	if (!ret) {
-		if (error) {
-			g_warning ("failed to coldplug unifying device: %s",
-				   error->message);
-			g_error_free (error);
-		}
+		g_warning ("failed to coldplug unifying device: %s",
+			   error->message);
+		g_error_free (error);
 		goto out;
 	}