unifying: Only call HID++2.0 features on version 2+ devices

Also, ignore HID++1.0 replies from HID++2.0 devices and use the correct
function ID for getDfuStatus to avoid an error.

plugins/unifying/lu-device-peripheral.c

@@ -110,58 +110,62 @@ lu_device_peripheral_fetch_firmware_info (LuDevice *device, GError **error)
 static gboolean
 lu_device_peripheral_fetch_battery_level (LuDevice *device, GError **error)
 {
-	guint8 idx;
-	g_autoptr(LuDeviceHidppMsg) msg = lu_device_hidpp_new ();
-
 	/* try using HID++2.0 */
-	idx = lu_device_hidpp_feature_get_idx (device, HIDPP_FEATURE_BATTERY_LEVEL_STATUS);
+	if (lu_device_get_hidpp_version (device) >= 2) {
-	if (idx != 0x00) {
+		guint8 idx;
-		msg->report_id = HIDPP_REPORT_ID_SHORT;
+		idx = lu_device_hidpp_feature_get_idx (device, HIDPP_FEATURE_BATTERY_LEVEL_STATUS);
-		msg->device_id = lu_device_get_hidpp_id (device);
+		if (idx != 0x00) {
-		msg->sub_id = idx;
+			g_autoptr(LuDeviceHidppMsg) msg = lu_device_hidpp_new ();
-		msg->function_id = 0x00; /* GetBatteryLevelStatus */
+			msg->report_id = HIDPP_REPORT_ID_SHORT;
-		if (!lu_device_hidpp_transfer (device, msg, error)) {
+			msg->device_id = lu_device_get_hidpp_id (device);
-			g_prefix_error (error, "failed to get battery info: ");
+			msg->sub_id = idx;
-			return FALSE;
+			msg->function_id = 0x00; /* GetBatteryLevelStatus */
+			if (!lu_device_hidpp_transfer (device, msg, error)) {
+				g_prefix_error (error, "failed to get battery info: ");
+				return FALSE;
+			}
+			if (msg->data[0] != 0x00)
+				lu_device_set_battery_level (device, msg->data[0]);
+			return TRUE;
 		}
-		if (msg->data[0] != 0x00)
-			lu_device_set_battery_level (device, msg->data[0]);
-		return TRUE;
 	}
 
 	/* try HID++1.0 battery mileage */
-	msg->report_id = HIDPP_REPORT_ID_SHORT;
+	if (lu_device_get_hidpp_version (device) == 1) {
-	msg->device_id = lu_device_get_hidpp_id (device);
+		g_autoptr(LuDeviceHidppMsg) msg = lu_device_hidpp_new ();
-	msg->sub_id = HIDPP_SUBID_GET_REGISTER;
+		msg->report_id = HIDPP_REPORT_ID_SHORT;
-	msg->function_id = HIDPP_REGISTER_BATTERY_MILEAGE;
+		msg->device_id = lu_device_get_hidpp_id (device);
-	if (lu_device_hidpp_transfer (device, msg, NULL)) {
+		msg->sub_id = HIDPP_SUBID_GET_REGISTER;
-		if (msg->data[0] != 0x00)
+		msg->function_id = HIDPP_REGISTER_BATTERY_MILEAGE;
-			lu_device_set_battery_level (device, msg->data[0]);
+		if (lu_device_hidpp_transfer (device, msg, NULL)) {
-		return TRUE;
+			if (msg->data[0] != 0x00)
-	}
+				lu_device_set_battery_level (device, msg->data[0]);
-
+			return TRUE;
-	/* try HID++1.0 battery status instead */
+		}
-	msg->function_id = HIDPP_REGISTER_BATTERY_STATUS;
+
-	if (lu_device_hidpp_transfer (device, msg, NULL)) {
+		/* try HID++1.0 battery status instead */
-		switch (msg->data[0]) {
+		msg->function_id = HIDPP_REGISTER_BATTERY_STATUS;
-		case 1: /* 0 - 10 */
+		if (lu_device_hidpp_transfer (device, msg, NULL)) {
-			lu_device_set_battery_level (device, 5);
+			switch (msg->data[0]) {
-			break;
+			case 1: /* 0 - 10 */
-		case 3: /* 11 - 30 */
+				lu_device_set_battery_level (device, 5);
-			lu_device_set_battery_level (device, 20);
+				break;
-			break;
+			case 3: /* 11 - 30 */
-		case 5: /* 31 - 80 */
+				lu_device_set_battery_level (device, 20);
-			lu_device_set_battery_level (device, 55);
+				break;
-			break;
+			case 5: /* 31 - 80 */
-		case 7: /* 81 - 100 */
+				lu_device_set_battery_level (device, 55);
-			lu_device_set_battery_level (device, 90);
+				break;
-			break;
+			case 7: /* 81 - 100 */
-		default:
+				lu_device_set_battery_level (device, 90);
-			g_warning ("unknown battery percentage: 0x%02x",
+				break;
-				   msg->data[0]);
+			default:
-			break;
+				g_warning ("unknown battery percentage: 0x%02x",
+					   msg->data[0]);
+				break;
+			}
+			return TRUE;
 		}
-		return TRUE;
 	}
 
 	/* not an error, the device just doesn't support any of the methods */
@@ -240,6 +244,12 @@ lu_device_peripheral_probe (LuDevice *device, GError **error)
 						     map_features[i],
 						     &error_local)) {
 			g_debug ("%s", error_local->message);
+			if (g_error_matches (error_local,
+					     G_IO_ERROR,
+					     G_IO_ERROR_TIMED_OUT)) {
+				/* timed out, so not trying any more */
+				break;
+			}
 		}
 	}
 
@@ -264,7 +274,7 @@ lu_device_peripheral_probe (LuDevice *device, GError **error)
 		msg->report_id = HIDPP_REPORT_ID_SHORT;
 		msg->device_id = lu_device_get_hidpp_id (device);
 		msg->sub_id = idx;
-		msg->function_id = 0x01 << 4; /* getDfuStatus */
+		msg->function_id = 0x00 << 4; /* getDfuStatus */
 		if (!lu_device_hidpp_transfer (device, msg, error)) {
 			g_prefix_error (error, "failed to get DFU status: ");
 			return FALSE;

plugins/unifying/lu-device.c

@@ -129,17 +129,85 @@ lu_device_hidpp_new (void)
 #define HIDPP_REPORT_04			0x04
 #define HIDPP_REPORT_20			0x20
 
-static void
+static gchar *
-hidpp_device_map_print (LuDevice *device)
+lu_device_flags_to_string (LuDeviceFlags flags)
+{
+	GString *str = g_string_new (NULL);
+	if (flags & LU_DEVICE_FLAG_REQUIRES_SIGNED_FIRMWARE)
+		g_string_append (str, "signed-firmware,");
+	if (flags & LU_DEVICE_FLAG_CAN_FLASH)
+		g_string_append (str, "can-flash,");
+	if (flags & LU_DEVICE_FLAG_REQUIRES_RESET)
+		g_string_append (str, "requires-reset,");
+	if (flags & LU_DEVICE_FLAG_ACTIVE)
+		g_string_append (str, "active,");
+	if (flags & LU_DEVICE_FLAG_IS_OPEN)
+		g_string_append (str, "is-open,");
+	if (flags & LU_DEVICE_FLAG_REQUIRES_ATTACH)
+		g_string_append (str, "requires-attach,");
+	if (flags & LU_DEVICE_FLAG_REQUIRES_DETACH)
+		g_string_append (str, "requires-detach,");
+	if (flags & LU_DEVICE_FLAG_DETACH_WILL_REPLUG)
+		g_string_append (str, "detach-will-replug,");
+	if (str->len == 0) {
+		g_string_append (str, "none");
+	} else {
+		g_string_truncate (str, str->len - 1);
+	}
+	return g_string_free (str, FALSE);
+}
+
+gchar *
+lu_device_to_string (LuDevice *device)
 {
 	LuDevicePrivate *priv = GET_PRIVATE (device);
+	GString *str = g_string_new (NULL);
+	g_autofree gchar *flags_str = NULL;
+
+	g_string_append_printf (str, "type:\t\t\t%s\n", lu_device_kind_to_string (priv->kind));
+	flags_str = lu_device_flags_to_string (priv->flags);
+	g_string_append_printf (str, "flags:\t\t\t%s\n", flags_str);
+	g_string_append_printf (str, "hidpp-version:\t\t%u\n", priv->hidpp_version);
+	if (priv->hidpp_id != HIDPP_DEVICE_ID_UNSET)
+		g_string_append_printf (str, "hidpp-id:\t\t0x%02x\n", (guint) priv->hidpp_id);
+	if (priv->udev_device_fd > 0)
+		g_string_append_printf (str, "udev-device:\t\t%i\n", priv->udev_device_fd);
+	if (priv->usb_device != NULL)
+		g_string_append_printf (str, "usb-device:\t\t%p\n", priv->usb_device);
+	if (priv->platform_id != NULL)
+		g_string_append_printf (str, "platform-id:\t\t%s\n", priv->platform_id);
+	if (priv->vendor != NULL)
+		g_string_append_printf (str, "vendor:\t\t\t%s\n", priv->vendor);
+	if (priv->product != NULL)
+		g_string_append_printf (str, "product:\t\t%s\n", priv->product);
+	if (priv->version_bl != NULL)
+		g_string_append_printf (str, "version-bootloader:\t%s\n", priv->version_bl);
+	if (priv->version_fw != NULL)
+		g_string_append_printf (str, "version-firmware:\t%s\n", priv->version_fw);
+	for (guint i = 0; i < priv->guids->len; i++) {
+		const gchar *guid = g_ptr_array_index (priv->guids, i);
+		g_string_append_printf (str, "guid:\t\t\t%s\n", guid);
+	}
+	if (priv->battery_level != 0)
+		g_string_append_printf (str, "battery-level:\t\t%u\n", priv->battery_level);
 	for (guint i = 0; i < priv->feature_index->len; i++) {
 		LuDeviceHidppMap *map = g_ptr_array_index (priv->feature_index, i);
-		g_debug ("%02x\t[%04x] %s",
+		g_string_append_printf (str, "feature%02x:\t\t%s [0x%04x]\n",
-			 map->idx,
+					map->idx,
-			 map->feature,
+					lu_hidpp_feature_to_string (map->feature),
-			 lu_hidpp_feature_to_string (map->feature));
+					map->feature);
 	}
+
+	/* fixme: superclass? */
+	if (LU_IS_DEVICE_BOOTLOADER (device)) {
+		g_string_append_printf (str, "flash-addr-high:\t0x%04x\n",
+					lu_device_bootloader_get_addr_hi (device));
+		g_string_append_printf (str, "flash-addr-low:\t0x%04x\n",
+					lu_device_bootloader_get_addr_lo (device));
+		g_string_append_printf (str, "flash-block-size:\t0x%04x\n",
+					lu_device_bootloader_get_blocksize (device));
+	}
+	return g_string_free (str, FALSE);
 }
 
 guint8
@@ -304,57 +372,47 @@ lu_device_hidpp_receive (LuDevice *device,
 		return FALSE;
 	}
 
-	/* this is likely an errata somewhere */
-	if (lu_device_get_kind (device) == LU_DEVICE_KIND_PERIPHERAL) {
-		const guint8 bytes[] = { 0x11, 0x02, 0xff, 0x11, 0x17 };
-		if (memcmp (msg, bytes, 5) == 0) {
-			guint8 *buf = (guint8 *) msg;
-			g_debug ("FIXME: using 0xff errata");
-			for (guint i = 2; i < read_size; i++)
-				buf[i] = buf[i + 1];
-			lu_device_hidpp_dump (device, "device->host", buf, read_size);
-		}
-	}
-
 	/* success */
 	return TRUE;
 }
 
-gboolean
+static gboolean
-lu_device_hidpp_transfer (LuDevice *device, LuDeviceHidppMsg *msg, GError **error)
+lu_device_hidpp_msg_is_hidpp_10 (LuDeviceHidppMsg *msg)
 {
-	LuDevicePrivate *priv = GET_PRIVATE (device);
+	/* filter HID++1.0 messages */
-	guint timeout = LU_DEVICE_TIMEOUT_MS;
+	if (msg->sub_id == 0x40 ||
-	g_autoptr(LuDeviceHidppMsg) msg_tmp = lu_device_hidpp_new ();
+	    msg->sub_id == 0x41 ||
+	    msg->sub_id == 0x49 ||
+	    msg->sub_id == 0x4b ||
+	    msg->sub_id == 0x8f) {
+		return TRUE;
+	}
+	return FALSE;
+}
 
-	/* increase timeout for some operations */
+static gboolean
-	if (msg->flags & LU_DEVICE_HIDPP_MSG_FLAG_LONGER_TIMEOUT)
+lu_device_hidpp_msg_check_swid (LuDeviceHidppMsg *msg, LuDeviceHidppMsg *msg_tmp)
-		timeout *= 5;
+{
-
+	if ((msg_tmp->function_id & 0x0f) != FU_DEVICE_UNIFYING_SW_ID)
-	/* send */
-	if (!lu_device_hidpp_send (device, msg, timeout, error))
 		return FALSE;
+	return TRUE;
+}
 
-	/* keep trying to receive until we get a valid reply */
+static gboolean
-	while (1) {
+lu_device_hidpp_msg_is_error (LuDevice *device,
-		if (!lu_device_hidpp_receive (device, msg_tmp, timeout, error))
+			      LuDeviceHidppMsg *msg,
-			return FALSE;
+			      LuDeviceHidppMsg *msg_tmp,
-
+			      GError **error)
-		/* not us */
+{
-		if (lu_device_get_hidpp_version (device) >= 2 &&
+	/* wrong device ID reported */
-		    (msg->flags & LU_DEVICE_HIDPP_MSG_FLAG_IGNORE_SWID) == 0) {
+	if (msg->device_id != HIDPP_DEVICE_ID_UNSET &&
-			if ((msg_tmp->function_id & 0x0f) != FU_DEVICE_UNIFYING_SW_ID) {
+	    msg->device_id != msg_tmp->device_id) {
-				g_debug ("ignoring reply with SwId 0x%02i, expected 0x%02i",
+		g_set_error_literal (error,
-					 msg_tmp->function_id & 0x0f,
+				     G_IO_ERROR,
-					 FU_DEVICE_UNIFYING_SW_ID);
+				     G_IO_ERROR_FAILED,
-				continue;
+				     "invalid device_id response");
-			}
+		return FALSE;
-		}
+	}
-
-		if (msg_tmp->report_id == 0x10 || msg_tmp->report_id == 0x11)
-			break;
-		g_debug ("ignoring message with report 0x%02x", msg_tmp->report_id);
-	};
 
 	/* kernel not doing it's thing */
 	if (msg_tmp->device_id == HIDPP_DEVICE_ID_UNSET) {
@@ -365,12 +423,6 @@ lu_device_hidpp_transfer (LuDevice *device, LuDeviceHidppMsg *msg, GError **erro
 		return FALSE;
 	}
 
-	/* if the HID++ ID is unset, grab it from the reply */
-	if (priv->hidpp_id == HIDPP_DEVICE_ID_UNSET) {
-		priv->hidpp_id = msg_tmp->device_id;
-		g_debug ("HID++ ID now %02x", priv->hidpp_id);
-	}
-
 	/* HID++ 1.0 error */
 	if (msg_tmp->sub_id == HIDPP_SUBID_ERROR_MSG) {
 		const gchar *tmp;
@@ -461,6 +513,63 @@ lu_device_hidpp_transfer (LuDevice *device, LuDeviceHidppMsg *msg, GError **erro
 		return FALSE;
 	}
 
+	/* HID++ 2.0 error */
+	if (msg_tmp->sub_id == HIDPP_SUBID_ERROR_MSG_20) {
+
+		switch (msg_tmp->data[1]) {
+		case HIDPP_ERROR_CODE_INVALID_ARGUMENT:
+			g_set_error (error,
+				     G_IO_ERROR,
+				     G_IO_ERROR_INVALID_ARGUMENT,
+				     "Invalid argument 0x%02x",
+				     msg_tmp->data[2]);
+			break;
+		case HIDPP_ERROR_CODE_OUT_OF_RANGE:
+			g_set_error (error,
+				     G_IO_ERROR,
+				     G_IO_ERROR_INVALID_DATA,
+				     "out of range");
+			break;
+		case HIDPP_ERROR_CODE_HW_ERROR:
+			g_set_error (error,
+				     G_IO_ERROR,
+				     G_IO_ERROR_BROKEN_PIPE,
+				     "hardware error");
+			break;
+		case HIDPP_ERROR_CODE_INVALID_FEATURE_INDEX:
+			g_set_error (error,
+				     G_IO_ERROR,
+				     G_IO_ERROR_INVALID_ARGUMENT,
+				     "invalid feature index");
+			break;
+		case HIDPP_ERROR_CODE_INVALID_FUNCTION_ID:
+			g_set_error (error,
+				     G_IO_ERROR,
+				     G_IO_ERROR_INVALID_ARGUMENT,
+				     "invalid function ID");
+			break;
+		case HIDPP_ERROR_CODE_BUSY:
+			g_set_error (error,
+				     G_IO_ERROR,
+				     G_IO_ERROR_BUSY,
+				     "busy");
+			break;
+		case HIDPP_ERROR_CODE_UNSUPPORTED:
+			g_set_error (error,
+				     G_IO_ERROR,
+				     G_IO_ERROR_NOT_SUPPORTED,
+				     "unsupported");
+			break;
+		default:
+			g_set_error_literal (error,
+					     G_IO_ERROR,
+					     G_IO_ERROR_FAILED,
+					     "generic failure");
+			break;
+		}
+		return FALSE;
+	}
+
 	/* check the response was valid */
 	if (0&&msg->report_id != msg_tmp->report_id) {
 		g_set_error_literal (error,
@@ -469,22 +578,78 @@ lu_device_hidpp_transfer (LuDevice *device, LuDeviceHidppMsg *msg, GError **erro
 				     "invalid report_id response");
 		return FALSE;
 	}
-	if (msg->device_id != HIDPP_DEVICE_ID_UNSET &&
-	    msg->device_id != msg_tmp->device_id) {
-		g_set_error_literal (error,
-				     G_IO_ERROR,
-				     G_IO_ERROR_FAILED,
-				     "invalid device_id response");
-		return FALSE;
-	}
 
-	/* copy over data */
+	return TRUE;
+}
+
+static void
+lu_device_hidpp_msg_copy (LuDeviceHidppMsg *msg, LuDeviceHidppMsg *msg_tmp)
+{
 	memset (msg->data, 0x00, sizeof(msg->data));
 	msg->device_id = msg_tmp->device_id;
 	msg->sub_id = msg_tmp->sub_id;
 	msg->function_id = msg_tmp->function_id;
 	memcpy (msg->data, msg_tmp->data, sizeof(msg->data));
+}
 
+gboolean
+lu_device_hidpp_transfer (LuDevice *device, LuDeviceHidppMsg *msg, GError **error)
+{
+	LuDevicePrivate *priv = GET_PRIVATE (device);
+	guint timeout = LU_DEVICE_TIMEOUT_MS;
+	g_autoptr(LuDeviceHidppMsg) msg_tmp = lu_device_hidpp_new ();
+
+	/* increase timeout for some operations */
+	if (msg->flags & LU_DEVICE_HIDPP_MSG_FLAG_LONGER_TIMEOUT)
+		timeout *= 5;
+
+	/* send request */
+	if (!lu_device_hidpp_send (device, msg, timeout, error))
+		return FALSE;
+
+	/* keep trying to receive until we get a valid reply */
+	while (1) {
+		if (!lu_device_hidpp_receive (device, msg_tmp, timeout, error))
+			return FALSE;
+
+		/* to ensure compatibility when an HID++ 2.0 device is
+		 * connected to an HID++ 1.0 receiver, any feature index
+		 * corresponding to an HID++ 1.0 sub-identifier which could be
+		 * sent by the receiver, must be assigned to a dummy feature */
+		if (lu_device_get_hidpp_version (device) >= 2 &&
+		    lu_device_hidpp_msg_is_hidpp_10 (msg_tmp)) {
+			g_debug ("ignoring HID++1.0 reply");
+			continue;
+		}
+
+		/* is error */
+		if (!lu_device_hidpp_msg_is_error (device, msg, msg_tmp, error))
+			return FALSE;
+
+		/* not us */
+		if (lu_device_get_hidpp_version (device) >= 2 &&
+		    (msg->flags & LU_DEVICE_HIDPP_MSG_FLAG_IGNORE_SWID) == 0) {
+			if (!lu_device_hidpp_msg_check_swid (msg, msg_tmp)) {
+				g_debug ("ignoring reply with SwId 0x%02i, expected 0x%02i",
+					 msg_tmp->function_id & 0x0f,
+					 FU_DEVICE_UNIFYING_SW_ID);
+				continue;
+			}
+		}
+
+		if (msg_tmp->report_id == 0x10 || msg_tmp->report_id == 0x11)
+			break;
+		g_debug ("ignoring message with report 0x%02x", msg_tmp->report_id);
+	};
+
+	/* if the HID++ ID is unset, grab it from the reply */
+	if (priv->hidpp_id == HIDPP_DEVICE_ID_UNSET) {
+		priv->hidpp_id = msg_tmp->device_id;
+		g_debug ("HID++ ID now %02x", priv->hidpp_id);
+	}
+
+	/* copy over data */
+	lu_device_hidpp_msg_copy (msg, msg_tmp);
 	return TRUE;
 }
 
@@ -732,6 +897,7 @@ lu_device_open (LuDevice *device, GError **error)
 {
 	LuDeviceClass *klass = LU_DEVICE_GET_CLASS (device);
 	LuDevicePrivate *priv = GET_PRIVATE (device);
+	g_autofree gchar *device_str = NULL;
 
 	g_return_val_if_fail (LU_IS_DEVICE (device), FALSE);
 	g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
@@ -797,8 +963,9 @@ lu_device_open (LuDevice *device, GError **error)
 		return FALSE;
 	}
 
-	/* show the HID++2.0 features we found */
+	/* show the device */
-	hidpp_device_map_print (device);
+	device_str = lu_device_to_string (device);
+	g_debug ("%s", device_str);
 
 	/* success */
 	return TRUE;

plugins/unifying/lu-device.h

@@ -119,6 +119,7 @@ LuDeviceHidppMsg *lu_device_hidpp_new		(void);
 LuDeviceKind	 lu_device_kind_from_string	(const gchar	*kind);
 const gchar	*lu_device_kind_to_string	(LuDeviceKind	 kind);
 
+gchar		*lu_device_to_string		(LuDevice		*device);
 LuDeviceKind	 lu_device_get_kind		(LuDevice		*device);
 guint8		 lu_device_get_hidpp_id		(LuDevice		*device);
 void		 lu_device_set_hidpp_id		(LuDevice		*device,

plugins/unifying/lu-hidpp.h

@@ -45,6 +45,7 @@ G_BEGIN_DECLS
 #define HIDPP_SUBID_SET_LONG_REGISTER		0x82
 #define HIDPP_SUBID_GET_LONG_REGISTER		0x83
 #define HIDPP_SUBID_ERROR_MSG			0x8F
+#define HIDPP_SUBID_ERROR_MSG_20		0xFF
 
 #define HIDPP_ERR_SUCCESS			0x00
 #define HIDPP_ERR_INVALID_SUBID			0x01

plugins/unifying/lu-tool.c

@@ -198,86 +198,11 @@ lu_get_default_device (FuLuToolPrivate *priv, GError **error)
 	return device;
 }
 
-static gchar *
-lu_device_flags_to_string (LuDeviceFlags flags)
-{
-	GString *str = g_string_new (NULL);
-	if (flags & LU_DEVICE_FLAG_REQUIRES_SIGNED_FIRMWARE)
-		g_string_append (str, "signed-firmware,");
-	if (flags & LU_DEVICE_FLAG_CAN_FLASH)
-		g_string_append (str, "can-flash,");
-	if (flags & LU_DEVICE_FLAG_REQUIRES_RESET)
-		g_string_append (str, "requires-reset,");
-	if (flags & LU_DEVICE_FLAG_ACTIVE)
-		g_string_append (str, "active,");
-	if (flags & LU_DEVICE_FLAG_IS_OPEN)
-		g_string_append (str, "is-open,");
-	if (flags & LU_DEVICE_FLAG_REQUIRES_ATTACH)
-		g_string_append (str, "requires-attach,");
-	if (flags & LU_DEVICE_FLAG_REQUIRES_DETACH)
-		g_string_append (str, "requires-detach,");
-	if (flags & LU_DEVICE_FLAG_DETACH_WILL_REPLUG)
-		g_string_append (str, "detach-will-replug,");
-	if (str->len == 0) {
-		g_string_append (str, "none");
-	} else {
-		g_string_truncate (str, str->len - 1);
-	}
-	return g_string_free (str, FALSE);
-}
-
 static gboolean
-lu_tool_info_device (FuLuToolPrivate *priv, LuDevice *device, GError **error)
+lu_tool_info_device (LuDevice *device)
 {
-	GPtrArray *guids;
+	g_autofree gchar *str = lu_device_to_string (device);
-	g_autoptr(GError) error_local = NULL;
+	g_print ("%s", str);
-
-	/* show on console */
-	g_print ("Type:           %s\n",
-		 lu_device_kind_to_string (lu_device_get_kind (device)));
-	g_print ("Flags:          %s\n",
-		 lu_device_flags_to_string (lu_device_get_flags (device)));
-	if (lu_device_get_hidpp_id (device) != HIDPP_DEVICE_ID_UNSET) {
-		g_print ("Hid++ ID:       0x%02x\n",
-			 lu_device_get_hidpp_id (device));
-	}
-	if (lu_device_get_hidpp_version (device) != 0) {
-		g_print ("Hid++ Version:  0x%02x\n",
-			 lu_device_get_hidpp_version (device));
-	}
-	g_print ("Platform ID:    %s\n",
-		 lu_device_get_platform_id (device));
-	g_print ("Vendor:         %s\n",
-		 lu_device_get_vendor (device));
-	g_print ("Product:        %s\n",
-		 lu_device_get_product (device));
-	if (lu_device_get_battery_level (device) != 0) {
-		g_print ("Battery Level:  %u%%\n",
-			 lu_device_get_battery_level (device));
-	}
-	if (lu_device_get_version_fw (device) != NULL) {
-		g_print ("Firmware Ver:   %s\n",
-			 lu_device_get_version_fw (device));
-	}
-	if (lu_device_get_version_bl (device) != NULL) {
-		g_print ("Bootloader Ver: %s\n",
-			 lu_device_get_version_bl (device));
-	}
-	if (LU_IS_DEVICE_BOOTLOADER (device)) {
-		g_print ("Flash Addr Hi:  0x%04x\n",
-			 lu_device_bootloader_get_addr_hi (device));
-		g_print ("Flash Addr Lo:  0x%04x\n",
-			 lu_device_bootloader_get_addr_lo (device));
-		g_print ("Flash Block Sz: 0x%04x\n",
-			 lu_device_bootloader_get_blocksize (device));
-	}
-	guids = lu_device_get_guids (device);
-	for (guint i = 0; i < guids->len; i++) {
-		const gchar *guid = g_ptr_array_index (guids, i);
-		g_print ("GUID:           %s\n", guid);
-	}
-
-	/* do not need to close device */
 	return TRUE;
 }
 
@@ -291,8 +216,7 @@ lu_tool_info (FuLuToolPrivate *priv, gchar **values, GError **error)
 	if (priv->emulation_kind != LU_DEVICE_KIND_UNKNOWN) {
 		g_autoptr(LuDevice) device = NULL;
 		device = lu_device_fake_new (priv->emulation_kind);
-		if (!lu_tool_info_device (priv, device, error))
+		lu_tool_info_device (device);
-			return FALSE;
 	}
 
 	/* get the devices */
@@ -304,8 +228,7 @@ lu_tool_info (FuLuToolPrivate *priv, gchar **values, GError **error)
 	devices = lu_context_get_devices (ctx);
 	for (guint i = 0; i < devices->len; i++) {
 		LuDevice *device = g_ptr_array_index (devices, i);
-		if (!lu_tool_info_device (priv, device, error))
+		lu_tool_info_device (device);
-			return FALSE;
 		if (i != devices->len - 1)
 			g_print ("\n");
 	}
@@ -467,8 +390,7 @@ lu_tool_device_added_cb (LuContext* ctx, LuDevice *device, FuLuToolPrivate *priv
 	g_print ("ADDED\tLogitech Unifying device %s {%p} [%s]\n",
 		 lu_device_kind_to_string (lu_device_get_kind (device)),
 		 device, lu_device_get_platform_id (device));
-	if (!lu_tool_info_device (priv, device, &error))
+	lu_tool_info_device (device);
-		g_print ("Failed to open: %s\n", error->message);
 }
 
 static void