Merge pull request #1557 from ispyisail/fix/spacemouse-scan-only-3d-mice

Fix the 3D mouse search asking every device for its name (#1553)

sources/spacemouse/hidbackend.cpp

@@ -82,16 +82,32 @@ void HidBackend::scan()
 		return;
 	}
 
-	hid_device_info *list = hid_enumerate(0, 0);
-	for (hid_device_info *info = list; info && !m_device; info = info->next)
+		//On Linux and macOS hidapi takes the name strings of the devices it
+		//lists from the system, and one call listing every device is the
+		//cheapest scan. Elsewhere, such as on Windows, it reads them from
+		//each device itself, so only 3D mice are asked for (see
+		//SpaceMouseHid::scanIds()).
+#if defined(Q_OS_LINUX) || defined(Q_OS_MACOS)
+	const QList<SpaceMouseHid::DeviceId> ids = {SpaceMouseHid::DeviceId()};
+#else
+	const QList<SpaceMouseHid::DeviceId> ids = SpaceMouseHid::scanIds();
+#endif
+	for (const SpaceMouseHid::DeviceId &id : ids)
 	{
-		if (!SpaceMouseHid::isSpaceMouse(info->vendor_id, info->product_id,
-						 info->usage_page, info->usage)) {
-			continue;
+		hid_device_info *list = hid_enumerate(id.vendor, id.product);
+		for (hid_device_info *info = list; info && !m_device; info = info->next)
+		{
+			if (!SpaceMouseHid::isSpaceMouse(info->vendor_id, info->product_id,
+							 info->usage_page, info->usage)) {
+				continue;
+			}
+			m_device = hid_open_path(info->path);
+		}
+		hid_free_enumeration(list);
+		if (m_device) {
+			break;
 		}
-		m_device = hid_open_path(info->path);
 	}
-	hid_free_enumeration(list);
 
 	if (!m_device) {
 		m_scan_timer->start();

sources/spacemouse/spacemousehid.cpp

@@ -42,7 +42,7 @@ namespace {
 	constexpr qint64 MAX_BITS = 1 << 20;
 
 		//3D mice sold under Logitech's vendor id, for the ones whose
-		//interface does not report a usage.
+		//interface does not report a usage, and for scanIds().
 	const QSet<unsigned short> LOGITECH_3D_MICE = {
 		0xc603, 0xc605, 0xc606, 0xc621, 0xc623, 0xc625,
 		0xc626, 0xc627, 0xc628, 0xc629, 0xc62b
@@ -95,6 +95,19 @@ bool SpaceMouseHid::isSpaceMouse(unsigned short vendor, unsigned short product,
 	return false;
 }
 
+QList<SpaceMouseHid::DeviceId> SpaceMouseHid::scanIds()
+{
+	QList<DeviceId> ids;
+	ids.append({VENDOR_3DCONNEXION, 0});
+
+	QList<unsigned short> logitech(LOGITECH_3D_MICE.cbegin(), LOGITECH_3D_MICE.cend());
+	std::sort(logitech.begin(), logitech.end());
+	for (const unsigned short product : std::as_const(logitech)) {
+		ids.append({VENDOR_LOGITECH, product});
+	}
+	return ids;
+}
+
 bool SpaceMouseHid::Layout::hasAxes() const
 {
 	for (const Field &f : axes) {

sources/spacemouse/spacemousehid.h

@@ -45,6 +45,21 @@ namespace SpaceMouseHid
 	bool isSpaceMouse(unsigned short vendor, unsigned short product,
 			  unsigned short usage_page, unsigned short usage);
 
+		/// A vendor and product id as hid_enumerate() takes them, where 0
+		/// matches every vendor or every product.
+	struct DeviceId
+	{
+		unsigned short vendor = 0;
+		unsigned short product = 0;
+	};
+
+		/// @return what a scan asks hidapi for where hidapi reads each
+		/// matching device's name strings from the device itself, as on
+		/// Windows (issue #1553): every 3Dconnexion device, and Logitech's
+		/// 3D mice by product id, since most other mice and keyboards carry
+		/// Logitech's id too.
+	QList<DeviceId> scanIds();
+
 		/// Where one value sits in the reports, per the report descriptor.
 	struct Field
 	{

tests/qttest/tst_spacemousehid.cpp

@@ -74,6 +74,8 @@ class TstSpaceMouseHid : public QObject
 	private slots:
 		void recognisesOnly3DMice_data();
 		void recognisesOnly3DMice();
+		void scanAsksOnlyFor3DMice_data();
+		void scanAsksOnlyFor3DMice();
 		void parsesTheClassicDescriptor();
 		void parsesADescriptorWithoutReportIds();
 		void survivesBrokenDescriptors_data();
@@ -115,6 +117,41 @@ void TstSpaceMouseHid::recognisesOnly3DMice()
 	QCOMPARE(isSpaceMouse(vendor, product, page, usage), match);
 }
 
+	// Where hidapi reads the name strings of every device a scan asks for
+	// from the device itself, as on Windows, the other devices on the PC
+	// must never be asked for at all (issue #1553).
+void TstSpaceMouseHid::scanAsksOnlyFor3DMice_data()
+{
+	QTest::addColumn<int>("vendor");
+	QTest::addColumn<int>("product");
+	QTest::addColumn<bool>("asked");
+	QTest::newRow("3Dconnexion SpaceMouse Compact") << 0x256f << 0xc635 << true;
+	QTest::newRow("3Dconnexion receiver") << 0x256f << 0xc652 << true;
+	QTest::newRow("Logitech SpaceNavigator") << 0x046d << 0xc626 << true;
+	QTest::newRow("Logitech SpacePilot Pro") << 0x046d << 0xc629 << true;
+	QTest::newRow("Logitech receiver") << 0x046d << 0xc52b << false;
+	QTest::newRow("Logitech mouse") << 0x046d << 0xc077 << false;
+	QTest::newRow("Microsoft keyboard") << 0x045e << 0x07f8 << false;
+	QTest::newRow("HP mouse") << 0x03f0 << 0x094a << false;
+}
+
+void TstSpaceMouseHid::scanAsksOnlyFor3DMice()
+{
+	QFETCH(int, vendor);
+	QFETCH(int, product);
+	QFETCH(bool, asked);
+
+		// hid_enumerate()'s own rule: an id of 0 matches every id.
+	bool matched = false;
+	for (const DeviceId &id : scanIds()) {
+		if ((id.vendor == 0 || id.vendor == vendor)
+				&& (id.product == 0 || id.product == product)) {
+			matched = true;
+		}
+	}
+	QCOMPARE(matched, asked);
+}
+
 void TstSpaceMouseHid::parsesTheClassicDescriptor()
 {
 	const Layout layout = parseDescriptor(classicDescriptor(true));