Skip to content

Commit

Permalink
Rework the match approach to prefer uniq over name
Browse files Browse the repository at this point in the history 
Previously we would match for name first, then uniq. But the name
matches are less reliable than uniq matches so where we can match for
uniq we should prefer that.

This avoids the case where where a name that applies to multiple
devices takes precedence over a more precise uniq match.
  • Loading branch information
@whot
whot committed May 8, 2024
1 parent 1b9692a commit 4a84563
Show file tree
Hide file tree
Showing 2 changed files with 186 additions and 14 deletions.
43 changes: 29 additions & 14 deletions libwacom/libwacom.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
}
Expand Down
157 changes: 157 additions & 0 deletions test/test_libwacom.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,64 @@
logger = logging.getLogger(__name__)


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


def test_database_init(db):
"""Just a test to make sure it doesn't crash"""
assert db is not None
Expand Down Expand Up @@ -226,3 +284,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

0 comments on commit 4a84563

Please sign in to comment.