diff --git a/libwacom/libwacom.c b/libwacom/libwacom.c index a617d20e..59d5d101 100644 --- a/libwacom/libwacom.c +++ b/libwacom/libwacom.c @@ -730,7 +730,8 @@ libwacom_new_from_builder(const WacomDeviceDatabase *db, const WacomBuilder *bui int vendor_id, product_id; char *name, *uniq; - const char *match_name, *match_uniq; + const char *used_match_name = NULL; + const char *used_match_uniq = NULL; WacomMatch *used_match; vendor_id = builder->vendor_id; @@ -743,20 +744,34 @@ libwacom_new_from_builder(const WacomDeviceDatabase *db, const WacomBuilder *bui bus = all_busses; while (*bus != WBUSTYPE_UNKNOWN) { - match_name = name; - match_uniq = uniq; - device = libwacom_new (db, match_name, match_uniq, vendor_id, product_id, *bus, error); - if (device == NULL) { - match_uniq = NULL; + /* Uniq (where it exists) is more reliable than the name which may be re-used + * across tablets. So try to find a uniq+name match first, then uniq-only, then + * name-only. + */ + struct match_approach { + const char *name; + const char *uniq; + } approaches[] = { + { name, uniq }, + { NULL, uniq }, + { name, NULL }, + { NULL, NULL }, + }; + struct match_approach *approach = approaches; + while (true) { + const char *match_name = approach->name; + const char *match_uniq = approach->uniq; device = libwacom_new (db, match_name, match_uniq, vendor_id, product_id, *bus, error); - if (device == NULL) { - match_name = NULL; - device = libwacom_new (db, match_name, match_uniq, vendor_id, product_id, *bus, error); - if (device == NULL) { - match_uniq = uniq; - device = libwacom_new (db, match_name, match_uniq, vendor_id, product_id, *bus, error); - } + if (device) { + used_match_name = match_name; + used_match_uniq = match_uniq; + break; } + + if (approach->name == NULL && approach->uniq == NULL) + break; + + approach++; } if (device) break; @@ -765,7 +780,7 @@ libwacom_new_from_builder(const WacomDeviceDatabase *db, const WacomBuilder *bui ret = fallback_or_device(db, device, builder->device_name, fallback); if (ret) { /* for multiple-match devices, set to the one we requested */ - used_match = libwacom_match_new(match_name, match_uniq, *bus, vendor_id, product_id); + used_match = libwacom_match_new(used_match_name, used_match_uniq, *bus, vendor_id, product_id); libwacom_set_default_match(ret, used_match); libwacom_match_unref(used_match); } diff --git a/test/test_libwacom.py b/test/test_libwacom.py index 2a5a7656..41c7286c 100644 --- a/test/test_libwacom.py +++ b/test/test_libwacom.py @@ -699,6 +699,64 @@ def new_from_builder( return WacomDevice(device) if device else None +def write_stylus_file(dir): + from configparser import ConfigParser + + config = ConfigParser() + config.optionxform = lambda option: option + config["0xfffff"] = { + "Name": "General Pen", + "Group": "generic-with-eraser", + "PairedStylusIds": "0xffffe;", + "Buttons": "2", + "Axes": "Tilt;Pressure;Distance;", + "Type": "General", + } + + config["0xffffe"] = { + "Name": "General Pen Eraser", + "Group": "generic-with-eraser", + "PairedStylusIds": "0xfffff;", + "EraserType": "Invert", + "Buttons": "2", + "Axes": "Tilt;Pressure;Distance;", + "Type": "General", + } + + with open(dir / "libwacom.stylus", "w") as fd: + config.write(fd, space_around_delimiters=False) + + +def write_tablet_file(filename, devicename, matches): + from configparser import ConfigParser + + config = ConfigParser() + config.optionxform = lambda option: option + config["Device"] = { + "Name": devicename, + "DeviceMatch": ";".join(matches), + "Width": 9, + "Height": 6, + "IntegratedIn": "", + "Class": "Bamboo", + "Layout": "", + } + config["Features"] = { + "Stylus": True, + "Reversible": False, + } + + with open(filename, "w") as fd: + config.write(fd, space_around_delimiters=False) + + +@pytest.fixture() +def custom_datadir(tmp_path): + write_stylus_file(tmp_path) + write_tablet_file(tmp_path / "generic.tablet", "Generic", ["generic"]) + return tmp_path + + @pytest.fixture() def libwacom(): try: @@ -933,3 +991,102 @@ def test_new_from_builder_uniq(db): builder.device_name = "GAOMON S620" device = db.new_from_builder(builder) assert device is None + + +def test_exact_matches(custom_datadir): + USBID = (0x1234, 0x5678) + UNIQ = "uniqval" + NAME = "nameval" + + # A device match with uniq but no name + matches = ["usb|1234|5678||uniqval"] + write_tablet_file(custom_datadir / "uniq.tablet", "UniqOnly", matches) + + # A device match with a name but no uniq + matches = ["usb|1234|5678|nameval"] + write_tablet_file(custom_datadir / "name.tablet", "NameOnly", matches) + + # A device match with both + matches = ["usb|1234|5678|nameval|uniqval"] + write_tablet_file(custom_datadir / "both.tablet", "Both", matches) + + db = WacomDatabase(path=custom_datadir) + + builder = WacomBuilder.create(usbid=USBID, uniq=UNIQ) + device = db.new_from_builder(builder) + assert device is not None + assert device.name == "UniqOnly" + + builder = WacomBuilder.create(usbid=USBID, match_name=NAME) + device = db.new_from_builder(builder) + assert device is not None + assert device.name == "NameOnly" + + builder = WacomBuilder.create(usbid=USBID, uniq=UNIQ, match_name=NAME) + device = db.new_from_builder(builder) + assert device is not None + assert device.name == "Both" + + +def test_prefer_uniq_over_name(custom_datadir): + USBID = (0x1234, 0x5678) + UNIQ = "uniqval" + NAME = "nameval" + + # A device match with uniq but no name + matches = ["usb|1234|5678||uniqval"] + write_tablet_file(custom_datadir / "uniq.tablet", "UniqOnly", matches) + + # A device match with a name but no uniq + matches = ["usb|1234|5678|nameval"] + write_tablet_file(custom_datadir / "name.tablet", "NameOnly", matches) + + db = WacomDatabase(path=custom_datadir) + + # name and uniq set in our match but we don't have a device with both. + # Prefer the uniq match over the name match + builder = WacomBuilder.create(usbid=USBID, uniq=UNIQ, match_name=NAME) + device = db.new_from_builder(builder) + assert device is not None + assert device.name == "UniqOnly" + + # If we have a uniq in our match but none of the DeviceMatches + # have that, fall back to name only + builder = WacomBuilder.create(usbid=USBID, uniq="whatever", match_name=NAME) + device = db.new_from_builder(builder) + assert device is not None + assert device.name == "NameOnly" + + # If we have a name in our match but none of the DeviceMatches + # have that, fall back to uniq only + builder = WacomBuilder.create(usbid=USBID, uniq=UNIQ, match_name="whatever") + device = db.new_from_builder(builder) + assert device is not None + assert device.name == "UniqOnly" + + +def test_dont_ignore_exact_matches(custom_datadir): + USBID = (0x1234, 0x5678) + UNIQ = "uniqval" + NAME = "nameval" + + # A device match with both + matches = ["usb|1234|5678|nameval|uniqval"] + write_tablet_file(custom_datadir / "both.tablet", "Both", matches) + + db = WacomDatabase(path=custom_datadir) + + builder = WacomBuilder.create(usbid=USBID, uniq=UNIQ, match_name=NAME) + device = db.new_from_builder(builder) + assert device is not None + assert device.name == "Both" + + # Our DeviceMatch has both uniq and name set, so only match + # when *both* match + builder = WacomBuilder.create(usbid=USBID, uniq=UNIQ, match_name="whatever") + device = db.new_from_builder(builder) + assert device is None + + builder = WacomBuilder.create(usbid=USBID, uniq="whatever", match_name=NAME) + device = db.new_from_builder(builder) + assert device is None