hidpp: improve HID++ version detection, fix uninit var

Do not assume HID++ 1.0 when device is unreachable. This allows
up_device_unifying_refresh() to be optimized to stop sending a ping
message at every refresh for HID++ 1.0 devices.

priv->version will now always contain 0 when the real HID++ version of
a device is not (yet) known, comments are updated to reflect this.

Also fix an uninitialised msg variable that might confuse the error
handler in hidpp_device_refresh.

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

2 files changed, 27 insertions, 11 deletions

diff --git a/src/linux/hidpp-device.c b/src/linux/hidpp-device.c
index 8e7e692..96c34ed 100644
--- a/src/linux/hidpp-device.c
+++ b/src/linux/hidpp-device.c
@@ -567,7 +567,7 @@ hidpp_device_refresh (HidppDevice *device,
 	const HidppDeviceMap *map;
 	gboolean ret = TRUE;
 	GString *name = NULL;
-	HidppMessage msg;
+	HidppMessage msg = { };
 	guint i;
 	guint len;
 	HidppDevicePrivate *priv = device->priv;
@@ -605,11 +605,23 @@ hidpp_device_refresh (HidppDevice *device,
 			if (hidpp_is_error(&msg, &error_code) &&
 				(error_code == HIDPP10_ERROR_CODE_INVALID_SUBID ||
 				/* if a device is unreachable, assume HID++ 1.0.
-				 * Otherwise, the device won't show up at
+				 * By doing so, we are still able to get the
+				 * device type (e.g. mouse or keyboard) at
 				 * enumeration time. */
 				error_code == HIDPP10_ERROR_CODE_RESOURCE_ERROR)) {
-				/* assume HID++ 1.0 ping response */
-				priv->version = 1;
+
+				/* assert HID++ 1.0 for the device only if we
+				 * are sure (i.e.  when the ping request
+				 * returned INVALID_SUBID) */
+				if (error_code == HIDPP10_ERROR_CODE_INVALID_SUBID) {
+					priv->version = 1;
+				} else {
+					g_debug("Cannot detect version, unreachable device");
+				}
+
+				/* do not execute the error handler at the end
+				 * of this function */
+				memset(&msg, 0, sizeof (msg));
 				g_error_free(*error);
 				*error = NULL;
 				ret = TRUE;
@@ -655,7 +667,9 @@ hidpp_device_refresh (HidppDevice *device,
 	/* get device kind */
 	if ((refresh_flags & HIDPP_REFRESH_FLAGS_KIND) > 0) {
 
-		if (priv->version == 1) {
+		/* the device type can always be queried using HID++ 1.0 on the
+		 * receiver, regardless of the device version. */
+		if (priv->version <= 1) {
 			msg.type = HIDPP_MSG_TYPE_SHORT;
 			msg.device_idx = HIDPP_RECEIVER_ADDRESS;
 			msg.feature_idx = HIDPP_READ_LONG_REGISTER;
@@ -728,7 +742,9 @@ hidpp_device_refresh (HidppDevice *device,
 
 	/* get device model string */
 	if ((refresh_flags & HIDPP_REFRESH_FLAGS_MODEL) > 0) {
-		if (priv->version == 1) {
+		/* the device name can always be queried using HID++ 1.0 on the
+		 * receiver, regardless of the device version. */
+		if (priv->version <= 1) {
 			msg.type = HIDPP_MSG_TYPE_SHORT;
 			msg.device_idx = HIDPP_RECEIVER_ADDRESS;
 			msg.feature_idx = HIDPP_READ_LONG_REGISTER;
diff --git a/src/linux/up-device-unifying.c b/src/linux/up-device-unifying.c
index b07d515..005a132 100644
--- a/src/linux/up-device-unifying.c
+++ b/src/linux/up-device-unifying.c
@@ -61,12 +61,12 @@ up_device_unifying_refresh (UpDevice *device)
 	refresh_flags = HIDPP_REFRESH_FLAGS_BATTERY;
 
 	/*
-	 * Device hid++ v2 when in unreachable mode seems to be able
-	 * to respond to hid++ v1 queries (but fails to respond to v2
-	 * queries). When it gets waken up it starts responding
-	 * to v2 queries, so always try to upgrade protocol to v2
+	 * When a device is initially unreachable, the HID++ version cannot be
+	 * determined.  Therefore try determining the HID++ version, otherwise
+	 * battery information cannot be retrieved. Assume that the HID++
+	 * version does not change once detected.
 	 */
-	if (hidpp_device_get_version (priv->hidpp_device) < 2)
+	if (hidpp_device_get_version (priv->hidpp_device) == 0)
 		refresh_flags |= HIDPP_REFRESH_FLAGS_VERSION;
 
 	ret = hidpp_device_refresh (priv->hidpp_device,