From 2f1c28598ca5c2d6b6164f4b03e371afb1d21308 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 10 Feb 2016 14:23:50 +0100 Subject: [PATCH 1/5] hidpp20drv: extract hidpp20drv_read_resolution_dpi_2201() Dissociate the reading of the data from the device of the actual storing of those data in the profiles. This will allow to call hidpp20drv_read_resolution_dpi_2201() without having profiles to know how many sensors are in the device. Signed-off-by: Benjamin Tissoires --- src/driver-hidpp20.c | 55 ++++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/src/driver-hidpp20.c b/src/driver-hidpp20.c index f2d8db5a..224f8e12 100644 --- a/src/driver-hidpp20.c +++ b/src/driver-hidpp20.c @@ -397,6 +397,39 @@ hidpp20drv_set_default_profile(struct ratbag_device *device, unsigned int index) return -ENOTSUP; } +static int +hidpp20drv_read_resolution_dpi_2201(struct ratbag_device *device) +{ + struct hidpp20drv_data *drv_data = ratbag_get_drv_data(device); + struct ratbag *ratbag = device->ratbag; + int rc; + + free(drv_data->sensors); + drv_data->sensors = NULL; + drv_data->num_sensors = 0; + rc = hidpp20_adjustable_dpi_get_sensors(drv_data->dev, &drv_data->sensors); + if (rc < 0) { + log_error(ratbag, + "Error while requesting resolution: %s (%d)\n", + strerror(-rc), rc); + return rc; + } else if (rc == 0) { + log_error(ratbag, "Error, no compatible sensors found.\n"); + return -ENODEV; + } + log_debug(ratbag, + "device is at %d dpi (variable between %d and %d).\n", + drv_data->sensors[0].dpi, + drv_data->sensors[0].dpi_min, + drv_data->sensors[0].dpi_max); + + drv_data->num_sensors = rc; + if (drv_data->num_sensors > MAX_RESOLUTIONS) + drv_data->num_sensors = MAX_RESOLUTIONS; + + return 0; +} + static int hidpp20drv_read_resolution_dpi(struct ratbag_profile *profile) { @@ -424,27 +457,9 @@ hidpp20drv_read_resolution_dpi(struct ratbag_profile *profile) } if (drv_data->capabilities & HIDPP_CAP_SWITCHABLE_RESOLUTION_2201) { - free(drv_data->sensors); - drv_data->sensors = NULL; - drv_data->num_sensors = 0; - rc = hidpp20_adjustable_dpi_get_sensors(drv_data->dev, &drv_data->sensors); - if (rc < 0) { - log_error(ratbag, - "Error while requesting resolution: %s (%d)\n", - strerror(-rc), rc); + rc = hidpp20drv_read_resolution_dpi_2201(device); + if (rc < 0) return rc; - } else if (rc == 0) { - log_error(ratbag, "Error, no compatible sensors found.\n"); - return -ENODEV; - } - log_debug(ratbag, - "device is at %d dpi (variable between %d and %d).\n", - drv_data->sensors[0].dpi, - drv_data->sensors[0].dpi_min, - drv_data->sensors[0].dpi_max); - drv_data->num_sensors = rc; - if (drv_data->num_sensors > MAX_RESOLUTIONS) - drv_data->num_sensors = MAX_RESOLUTIONS; profile->resolution.num_modes = drv_data->num_sensors; for (i = 0; i < profile->resolution.num_modes; i++) { int dpi = drv_data->sensors[i].dpi; From 3c3277a403d9fa0e88366d1a3d6fa76c5135f159 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 10 Feb 2016 14:27:48 +0100 Subject: [PATCH 2/5] hidpp20drv: rework hidpp20drv_read_profile() The current code might call some other function when the device supports onboard profiles. We should not query those extra functions given that the profiles should give us all we need. Signed-off-by: Benjamin Tissoires --- src/driver-hidpp20.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/driver-hidpp20.c b/src/driver-hidpp20.c index 224f8e12..c1fd9d42 100644 --- a/src/driver-hidpp20.c +++ b/src/driver-hidpp20.c @@ -612,30 +612,26 @@ hidpp20drv_read_onboard_profile(struct ratbag_device *device, unsigned index) } static void -hidpp20drv_read_profile(struct ratbag_profile *profile, unsigned int index) +hidpp20drv_read_profile_8100(struct ratbag_profile *profile, unsigned int index) { struct ratbag_device *device = profile->device; struct hidpp20drv_data *drv_data = ratbag_get_drv_data(device); struct ratbag_resolution *res; + struct hidpp20_profile *p; unsigned i, dpi; - hidpp20drv_read_resolution_dpi(profile); - hidpp20drv_read_special_key_mouse(device); hidpp20drv_read_onboard_profile(device, profile->index); profile->is_active = false; if ((int)index == hidpp20drv_current_profile(device)) profile->is_active = true; - if (!(drv_data->capabilities & HIDPP_CAP_ONBOARD_PROFILES_8100)) - return; - dpi = ratbag_resolution_get_dpi(profile->resolution.modes); profile->resolution.num_modes = drv_data->profiles->num_modes; - for (i = 0; i < drv_data->profiles->num_modes; i++) { - struct hidpp20_profile *p = &drv_data->profiles->profiles[index]; + p = &drv_data->profiles->profiles[index]; + for (i = 0; i < profile->resolution.num_modes; i++) { res = ratbag_resolution_init(profile, i, p->dpi[i], p->dpi[i], @@ -649,6 +645,21 @@ hidpp20drv_read_profile(struct ratbag_profile *profile, unsigned int index) } } +static void +hidpp20drv_read_profile(struct ratbag_profile *profile, unsigned int index) +{ + struct ratbag_device *device = profile->device; + struct hidpp20drv_data *drv_data = ratbag_get_drv_data(device); + + if (drv_data->capabilities & HIDPP_CAP_ONBOARD_PROFILES_8100) + return hidpp20drv_read_profile_8100(profile, index); + + hidpp20drv_read_resolution_dpi(profile); + hidpp20drv_read_special_key_mouse(device); + + profile->is_active = (index == 0); +} + static int hidpp20drv_write_profile(struct ratbag_profile *profile) { From df088466ae57135b58255d3d1171187342aef2e8 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 10 Feb 2016 14:32:59 +0100 Subject: [PATCH 3/5] hidpp20drv: rework the initialisation process Do not interleave some internal ratbag initialisation with the hidpp20 initial reading. During early init, we are just interested in knowing how many profiles/resolutions/buttons the device supports, so we should not read more than required. Signed-off-by: Benjamin Tissoires --- src/driver-hidpp20.c | 48 ++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/src/driver-hidpp20.c b/src/driver-hidpp20.c index c1fd9d42..f463ae96 100644 --- a/src/driver-hidpp20.c +++ b/src/driver-hidpp20.c @@ -62,6 +62,10 @@ struct hidpp20drv_data { unsigned num_controls; struct hidpp20_control_id *controls; struct hidpp20_profiles *profiles; + + unsigned int num_profiles; + unsigned int num_resolutions; + unsigned int num_buttons; }; static void @@ -378,7 +382,7 @@ hidpp20drv_set_current_profile(struct ratbag_device *device, unsigned int index) if (!(drv_data->capabilities & HIDPP_CAP_ONBOARD_PROFILES_8100)) return 0; - if (index >= drv_data->profiles->num_profiles) + if (index >= drv_data->num_profiles) return -EINVAL; h_profile = &drv_data->profiles->profiles[index]; @@ -427,6 +431,8 @@ hidpp20drv_read_resolution_dpi_2201(struct ratbag_device *device) if (drv_data->num_sensors > MAX_RESOLUTIONS) drv_data->num_sensors = MAX_RESOLUTIONS; + drv_data->num_resolutions = drv_data->num_sensors; + return 0; } @@ -444,7 +450,6 @@ hidpp20drv_read_resolution_dpi(struct ratbag_profile *profile) uint16_t resolution; uint8_t flags; - profile->resolution.num_modes = 1; rc = hidpp20_mousepointer_get_mousepointer_info(drv_data->dev, &resolution, &flags); if (rc) { log_error(ratbag, @@ -460,7 +465,7 @@ hidpp20drv_read_resolution_dpi(struct ratbag_profile *profile) rc = hidpp20drv_read_resolution_dpi_2201(device); if (rc < 0) return rc; - profile->resolution.num_modes = drv_data->num_sensors; + profile->resolution.num_modes = drv_data->num_resolutions; for (i = 0; i < profile->resolution.num_modes; i++) { int dpi = drv_data->sensors[i].dpi; /* FIXME: retrieve the refresh rate */ @@ -596,14 +601,6 @@ hidpp20drv_read_onboard_profile(struct ratbag_device *device, unsigned index) if (!(drv_data->capabilities & HIDPP_CAP_ONBOARD_PROFILES_8100)) return 0; - if (!drv_data->profiles) { - rc = hidpp20_onboard_profiles_allocate(drv_data->dev, &drv_data->profiles); - if (rc < 0) - return rc; - - device->num_buttons = drv_data->profiles->num_buttons; - } - rc = hidpp20_onboard_profiles_read(drv_data->dev, index, drv_data->profiles); if (rc < 0) return rc; @@ -628,9 +625,9 @@ hidpp20drv_read_profile_8100(struct ratbag_profile *profile, unsigned int index) dpi = ratbag_resolution_get_dpi(profile->resolution.modes); - profile->resolution.num_modes = drv_data->profiles->num_modes; p = &drv_data->profiles->profiles[index]; + profile->resolution.num_modes = drv_data->num_resolutions; for (i = 0; i < profile->resolution.num_modes; i++) { res = ratbag_resolution_init(profile, i, p->dpi[i], @@ -693,6 +690,11 @@ hidpp20drv_init_feature(struct ratbag_device *device, uint16_t feature) log_debug(ratbag, "device has adjustable dpi\n"); hidpp20drv_set_exported_capability(device, RATBAG_DEVICE_CAP_SWITCHABLE_RESOLUTION); drv_data->capabilities |= HIDPP_CAP_SWITCHABLE_RESOLUTION_2201; + /* we read the profile once to get the correct number of + * supported resolutions. */ + rc = hidpp20drv_read_resolution_dpi_2201(device); + if (rc < 0) + return rc; break; } case HIDPP_PAGE_SPECIAL_KEYS_BUTTONS: { @@ -744,9 +746,15 @@ hidpp20drv_init_feature(struct ratbag_device *device, uint16_t feature) hidpp20drv_set_exported_capability(device, RATBAG_DEVICE_CAP_BUTTON_KEY); hidpp20drv_set_exported_capability(device, RATBAG_DEVICE_CAP_SWITCHABLE_RESOLUTION); hidpp20drv_set_exported_capability(device, RATBAG_DEVICE_CAP_SWITCHABLE_PROFILE); - /* we read the profiles once with an incorect profile index - * to get the correct number of supported profiles. */ - hidpp20drv_read_onboard_profile(device, 0xffffff); + + rc = hidpp20_onboard_profiles_allocate(drv_data->dev, &drv_data->profiles); + if (rc < 0) + return rc; + + drv_data->num_profiles = drv_data->profiles->num_profiles; + drv_data->num_resolutions = drv_data->profiles->num_modes; + drv_data->num_buttons = drv_data->profiles->num_buttons; + break; } case HIDPP_PAGE_MOUSE_BUTTON_SPY: { @@ -833,14 +841,20 @@ hidpp20drv_probe(struct ratbag_device *device) log_debug(device->ratbag, "'%s' is using protocol v%d.%d\n", ratbag_device_get_name(device), dev->proto_major, dev->proto_minor); + /* add some defaults that will be overwritten by the device */ + drv_data->num_profiles = 1; + drv_data->num_resolutions = 1; + drv_data->num_buttons = 8; + if (dev->proto_major >= 2) { rc = hidpp20drv_20_probe(device); if (rc) goto err; } - ratbag_device_init_profiles(device, drv_data->profiles ? drv_data->profiles->num_profiles : 1, - device->num_buttons ? device->num_buttons : 8); + ratbag_device_init_profiles(device, + drv_data->num_profiles, + drv_data->num_buttons); return rc; err: From 80fd187c3cf6ab07101c90b43c628de86a1b6089 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 10 Feb 2016 14:39:38 +0100 Subject: [PATCH 4/5] Dynamically allocate the resolutions We were limited to MAX_RESOLUTIONS (10). Also, this allows to know during ratbag_device_init_profiles() if the device supports switchable resolutions. Signed-off-by: Benjamin Tissoires --- src/driver-etekcity.c | 9 +++++---- src/driver-hidpp10.c | 10 ++++++---- src/driver-hidpp20.c | 6 +----- src/driver-roccat.c | 9 +++++---- src/driver-test.c | 4 ++-- src/libratbag-private.h | 4 ++-- src/libratbag-test.h | 2 +- src/libratbag.c | 12 ++++++++---- test/test-device.c | 10 +++------- 9 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/driver-etekcity.c b/src/driver-etekcity.c index 877ac73b..294dfaa3 100644 --- a/src/driver-etekcity.c +++ b/src/driver-etekcity.c @@ -387,9 +387,7 @@ etekcity_read_profile(struct ratbag_profile *profile, unsigned int index) report_rate = 0; } - profile->resolution.num_modes = ETEKCITY_NUM_DPI; - - for (i = 0; i < ETEKCITY_NUM_DPI; i++) { + for (i = 0; i < profile->resolution.num_modes; i++) { dpi_x = setting_report->xres[i] * 50; dpi_y = setting_report->yres[i] * 50; hz = report_rate; @@ -682,7 +680,10 @@ etekcity_probe(struct ratbag_device *device) log_debug(device->ratbag, "device is at %d ms of latency\n", drv_data->speed_setting[2]); /* profiles are 0-indexed */ - ratbag_device_init_profiles(device, ETEKCITY_PROFILE_MAX + 1, ETEKCITY_BUTTON_MAX + 1); + ratbag_device_init_profiles(device, + ETEKCITY_PROFILE_MAX + 1, + ETEKCITY_NUM_DPI, + ETEKCITY_BUTTON_MAX + 1); active_idx = etekcity_current_profile(device); if (active_idx < 0) { diff --git a/src/driver-hidpp10.c b/src/driver-hidpp10.c index 0454304c..0cd6082b 100644 --- a/src/driver-hidpp10.c +++ b/src/driver-hidpp10.c @@ -381,8 +381,7 @@ hidpp10drv_read_profile(struct ratbag_profile *profile, unsigned int index) if (rc) xres = 0xffff; - profile->resolution.num_modes = p.num_dpi_modes; - for (i = 0; i < p.num_dpi_modes; i++) { + for (i = 0; i < profile->resolution.num_modes; i++) { res = ratbag_resolution_init(profile, i, p.dpi_modes[i].xres, p.dpi_modes[i].yres, @@ -467,7 +466,10 @@ hidpp10drv_fill_from_profile(struct ratbag_device *device, struct hidpp10_device if (rc) return rc; - ratbag_device_init_profiles(device, count, profile.num_buttons); + ratbag_device_init_profiles(device, + count, + profile.num_dpi_modes, + profile.num_buttons); return 0; } @@ -588,7 +590,7 @@ hidpp10drv_probe(struct ratbag_device *device) /* Fall back to something that every mouse has */ struct ratbag_profile *profile; - ratbag_device_init_profiles(device, 1, 3); + ratbag_device_init_profiles(device, 1, 1, 3); profile = ratbag_device_get_profile(device, 0); profile->is_active = true; profile->is_default = true; diff --git a/src/driver-hidpp20.c b/src/driver-hidpp20.c index f463ae96..8f0b1f41 100644 --- a/src/driver-hidpp20.c +++ b/src/driver-hidpp20.c @@ -428,9 +428,6 @@ hidpp20drv_read_resolution_dpi_2201(struct ratbag_device *device) drv_data->sensors[0].dpi_max); drv_data->num_sensors = rc; - if (drv_data->num_sensors > MAX_RESOLUTIONS) - drv_data->num_sensors = MAX_RESOLUTIONS; - drv_data->num_resolutions = drv_data->num_sensors; return 0; @@ -465,7 +462,6 @@ hidpp20drv_read_resolution_dpi(struct ratbag_profile *profile) rc = hidpp20drv_read_resolution_dpi_2201(device); if (rc < 0) return rc; - profile->resolution.num_modes = drv_data->num_resolutions; for (i = 0; i < profile->resolution.num_modes; i++) { int dpi = drv_data->sensors[i].dpi; /* FIXME: retrieve the refresh rate */ @@ -627,7 +623,6 @@ hidpp20drv_read_profile_8100(struct ratbag_profile *profile, unsigned int index) p = &drv_data->profiles->profiles[index]; - profile->resolution.num_modes = drv_data->num_resolutions; for (i = 0; i < profile->resolution.num_modes; i++) { res = ratbag_resolution_init(profile, i, p->dpi[i], @@ -854,6 +849,7 @@ hidpp20drv_probe(struct ratbag_device *device) ratbag_device_init_profiles(device, drv_data->num_profiles, + drv_data->num_resolutions, drv_data->num_buttons); return rc; diff --git a/src/driver-roccat.c b/src/driver-roccat.c index 2a00c864..97b65c41 100644 --- a/src/driver-roccat.c +++ b/src/driver-roccat.c @@ -446,9 +446,7 @@ roccat_read_profile(struct ratbag_profile *profile, unsigned int index) report_rate = 0; } - profile->resolution.num_modes = ROCCAT_NUM_DPI; - - for (i = 0; i < ROCCAT_NUM_DPI; i++) { + for (i = 0; i < profile->resolution.num_modes; i++) { dpi_x = setting_report->xres[i] * 50; dpi_y = setting_report->yres[i] * 50; hz = report_rate; @@ -813,7 +811,10 @@ roccat_probe(struct ratbag_device *device) ratbag_set_drv_data(device, drv_data); /* profiles are 0-indexed */ - ratbag_device_init_profiles(device, ROCCAT_PROFILE_MAX + 1, ROCCAT_BUTTON_MAX + 1); + ratbag_device_init_profiles(device, + ROCCAT_PROFILE_MAX + 1, + ROCCAT_NUM_DPI, + ROCCAT_BUTTON_MAX + 1); active_idx = roccat_current_profile(device); if (active_idx < 0) { diff --git a/src/driver-test.c b/src/driver-test.c index 91a30e47..fb52d520 100644 --- a/src/driver-test.c +++ b/src/driver-test.c @@ -57,8 +57,7 @@ test_read_profile(struct ratbag_profile *profile, unsigned int index) assert(index < d->num_profiles); p = &d->profiles[index]; - profile->resolution.num_modes = p->num_resolutions; - for (i = 0; i < p->num_resolutions; i++) { + for (i = 0; i < d->num_resolutions; i++) { struct ratbag_resolution *res; r = &p->resolutions[i]; @@ -96,6 +95,7 @@ test_probe(struct ratbag_device *device, void *data) ratbag_set_drv_data(device, test_device); ratbag_device_init_profiles(device, test_device->num_profiles, + test_device->num_resolutions, test_device->num_buttons); return 0; } diff --git a/src/libratbag-private.h b/src/libratbag-private.h index 2eff7321..74794128 100644 --- a/src/libratbag-private.h +++ b/src/libratbag-private.h @@ -208,7 +208,6 @@ struct ratbag_driver { struct list link; }; -#define MAX_RESOLUTIONS 10 struct ratbag_resolution{ struct ratbag_profile *profile; int refcount; @@ -232,7 +231,7 @@ struct ratbag_profile { void *drv_data; void *user_data; struct { - struct ratbag_resolution modes[MAX_RESOLUTIONS]; + struct ratbag_resolution *modes; unsigned int num_modes; } resolution; @@ -332,6 +331,7 @@ ratbag_get_drv_data(struct ratbag_device *device) int ratbag_device_init_profiles(struct ratbag_device *device, unsigned int num_profiles, + unsigned int num_resolutions, unsigned int num_buttons); static inline void diff --git a/src/libratbag-test.h b/src/libratbag-test.h index a5573917..3dbd1ffd 100644 --- a/src/libratbag-test.h +++ b/src/libratbag-test.h @@ -43,7 +43,6 @@ struct ratbag_test_resolution { }; struct ratbag_test_profile { - unsigned int num_resolutions; struct ratbag_test_button buttons[RATBAG_TEST_MAX_BUTTONS]; struct ratbag_test_resolution resolutions[RATBAG_TEST_MAX_RESOLUTIONS]; bool active; @@ -52,6 +51,7 @@ struct ratbag_test_profile { struct ratbag_test_device { unsigned int num_profiles; + unsigned int num_resolutions; unsigned int num_buttons; struct ratbag_test_profile profiles[RATBAG_TEST_MAX_PROFILES]; void (*destroyed)(struct ratbag_device *device, void *data); diff --git a/src/libratbag.c b/src/libratbag.c index c4c27f2c..c6cedde9 100644 --- a/src/libratbag.c +++ b/src/libratbag.c @@ -564,6 +564,7 @@ ratbag_profile_init_buttons(struct ratbag_profile *profile, unsigned int count) static struct ratbag_profile * ratbag_create_profile(struct ratbag_device *device, unsigned int index, + unsigned int num_resolutions, unsigned int num_buttons) { struct ratbag_profile *profile; @@ -573,13 +574,15 @@ ratbag_create_profile(struct ratbag_device *device, profile->refcount = 0; profile->device = device; profile->index = index; + profile->resolution.modes = zalloc(num_resolutions * + sizeof(*profile->resolution.modes)); + profile->resolution.num_modes = num_resolutions; list_insert(&device->profiles, &profile->link); list_init(&profile->buttons); - for (i = 0; i < MAX_RESOLUTIONS; i++) + for (i = 0; i < num_resolutions; i++) ratbag_resolution_init(profile, i, 0, 0, 0); - profile->resolution.num_modes = 1; assert(device->driver->read_profile); device->driver->read_profile(profile, index); @@ -592,12 +595,13 @@ ratbag_create_profile(struct ratbag_device *device, int ratbag_device_init_profiles(struct ratbag_device *device, unsigned int num_profiles, + unsigned int num_resolutions, unsigned int num_buttons) { unsigned int i; for (i = 0; i < num_profiles; i++) { - ratbag_create_profile(device, i, num_buttons); + ratbag_create_profile(device, i, num_resolutions, num_buttons); } device->num_profiles = num_profiles; @@ -626,7 +630,7 @@ ratbag_profile_destroy(struct ratbag_profile *profile) list_for_each_safe(button, next, &profile->buttons, link) ratbag_button_destroy(button); - /* Resolution is a fixed list of structs, no freeing required */ + free(profile->resolution.modes); list_remove(&profile->link); free(profile); diff --git a/test/test-device.c b/test/test-device.c index 455e8906..776dc15e 100644 --- a/test/test-device.c +++ b/test/test-device.c @@ -51,10 +51,10 @@ device_destroyed(struct ratbag_device *device, void *data) */ const struct ratbag_test_device sane_device = { .num_profiles = 3, + .num_resolutions = 3, .num_buttons = 1, .profiles = { { - .num_resolutions = 3, .resolutions = { { .xres = 100, .yres = 200, .hz = 1000 }, { .xres = 200, .yres = 300, .hz = 1000 }, @@ -64,7 +64,6 @@ const struct ratbag_test_device sane_device = { .dflt = false, }, { - .num_resolutions = 3, .resolutions = { { .xres = 1100, .yres = 1200, .hz = 2000 }, { .xres = 1200, .yres = 1300, .hz = 2000 }, @@ -74,7 +73,6 @@ const struct ratbag_test_device sane_device = { .dflt = true, }, { - .num_resolutions = 3, .resolutions = { { .xres = 2100, .yres = 2200, .hz = 3000 }, { .xres = 2200, .yres = 2300, .hz = 3000 }, @@ -330,10 +328,10 @@ START_TEST(device_resolutions) struct ratbag_test_device td = { .num_profiles = 3, + .num_resolutions = 3, .num_buttons = 1, .profiles = { { - .num_resolutions = 3, .resolutions = { { .xres = 100, .yres = 200, .hz = 1000 }, { .xres = 200, .yres = 300, .hz = 1000, .active = true }, @@ -342,7 +340,6 @@ START_TEST(device_resolutions) .active = true, }, { - .num_resolutions = 3, .resolutions = { { .xres = 1100, .yres = 1200, .hz = 2000 }, { .xres = 1200, .yres = 1300, .hz = 2000, .active = true }, @@ -350,7 +347,6 @@ START_TEST(device_resolutions) }, }, { - .num_resolutions = 3, .resolutions = { { .xres = 2100, .yres = 2200, .hz = 3000 }, { .xres = 2200, .yres = 2300, .hz = 3000, .active = true }, @@ -429,9 +425,9 @@ START_TEST(device_resolutions_num_0) struct ratbag_test_device td = { .num_profiles = 1, .num_buttons = 1, + .num_resolutions = 0, /* failure trigger */ .profiles = { { - .num_resolutions = 0, /* failure trigger */ .active = true, }, }, From 960ee73da91420884b1823d4bbae1ed5b5135cd2 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Wed, 10 Feb 2016 15:08:35 +0100 Subject: [PATCH 5/5] remove .has_capability() driver hook Most of the capabilities can be inferred from ratbag_device_init_profiles(): - if a device has more than one profile, we should be able to switch between its profiles - likewise, more than one profile means that the buttons can be remapped - more than one resolution means that we can change the resolution. We need to add a stub in driver-test for .set_active_profile() now that RATBAG_DEVICE_CAP_SWITCHABLE_PROFILES is correctly set. We manually add the macro capability for all drivers that present it. Signed-off-by: Benjamin Tissoires --- src/driver-etekcity.c | 21 ++------------------- src/driver-hidpp10.c | 26 +++----------------------- src/driver-hidpp20.c | 34 ++-------------------------------- src/driver-roccat.c | 20 ++------------------ src/driver-test.c | 35 +++++++++++++++-------------------- src/libratbag-private.h | 16 +++++----------- src/libratbag.c | 24 ++++++++++++++++++++++-- 7 files changed, 51 insertions(+), 125 deletions(-) diff --git a/src/driver-etekcity.c b/src/driver-etekcity.c index 294dfaa3..e9af6fbd 100644 --- a/src/driver-etekcity.c +++ b/src/driver-etekcity.c @@ -250,24 +250,6 @@ etekcity_button_action_to_raw(const struct ratbag_button_action *action) return 0; } -static int -etekcity_has_capability(const struct ratbag_device *device, - enum ratbag_device_capability cap) -{ - switch (cap) { - case RATBAG_DEVICE_CAP_NONE: - return 0; - case RATBAG_DEVICE_CAP_SWITCHABLE_RESOLUTION: - case RATBAG_DEVICE_CAP_SWITCHABLE_PROFILE: - case RATBAG_DEVICE_CAP_BUTTON_KEY: - case RATBAG_DEVICE_CAP_BUTTON_MACROS: - return 1; - case RATBAG_DEVICE_CAP_DEFAULT_PROFILE: - return 0; - } - return 0; -} - static int etekcity_current_profile(struct ratbag_device *device) { @@ -685,6 +667,8 @@ etekcity_probe(struct ratbag_device *device) ETEKCITY_NUM_DPI, ETEKCITY_BUTTON_MAX + 1); + ratbag_device_set_capability(device, RATBAG_DEVICE_CAP_BUTTON_MACROS); + active_idx = etekcity_current_profile(device); if (active_idx < 0) { log_error(device->ratbag, @@ -731,7 +715,6 @@ struct ratbag_driver etekcity_driver = { .write_profile = etekcity_write_profile, .set_active_profile = etekcity_set_current_profile, .set_default_profile = etekcity_set_default_profile, - .has_capability = etekcity_has_capability, .read_button = etekcity_read_button, .write_button = etekcity_write_button, .write_resolution_dpi = etekcity_write_resolution_dpi, diff --git a/src/driver-hidpp10.c b/src/driver-hidpp10.c index 0cd6082b..5fa93dc8 100644 --- a/src/driver-hidpp10.c +++ b/src/driver-hidpp10.c @@ -317,28 +317,6 @@ hidpp10drv_write_button(struct ratbag_button *button, return hidpp10_set_profile(drv_data->dev, button->profile->index, &profile); } -static int -hidpp10drv_has_capability(const struct ratbag_device *device, - enum ratbag_device_capability cap) -{ - /* we force a non const device to shut up a warning, but we won't change anything */ - struct hidpp10drv_data *drv_data = ratbag_get_drv_data((struct ratbag_device *)device); - struct hidpp10_device *hidpp10 = drv_data->dev; - - switch (cap) { - case RATBAG_DEVICE_CAP_NONE: - abort(); - break; - case RATBAG_DEVICE_CAP_SWITCHABLE_PROFILE: - case RATBAG_DEVICE_CAP_SWITCHABLE_RESOLUTION: - case RATBAG_DEVICE_CAP_BUTTON_KEY: - case RATBAG_DEVICE_CAP_BUTTON_MACROS: - case RATBAG_DEVICE_CAP_DEFAULT_PROFILE: - return (hidpp10->profile_type != HIDPP10_PROFILE_UNKNOWN); - } - return 0; -} - static int hidpp10drv_set_current_profile(struct ratbag_device *device, unsigned int index) { @@ -471,6 +449,9 @@ hidpp10drv_fill_from_profile(struct ratbag_device *device, struct hidpp10_device profile.num_dpi_modes, profile.num_buttons); + if (dev->profile_type != HIDPP10_PROFILE_UNKNOWN) + ratbag_device_set_capability(device, RATBAG_DEVICE_CAP_BUTTON_MACROS); + return 0; } @@ -632,7 +613,6 @@ struct ratbag_driver hidpp10_driver = { .write_profile = hidpp10drv_write_profile, .set_active_profile = hidpp10drv_set_current_profile, .set_default_profile = hidpp10drv_set_default_profile, - .has_capability = hidpp10drv_has_capability, .read_button = hidpp10drv_read_button, .write_button = hidpp10drv_write_button, .write_resolution_dpi = hidpp10drv_write_resolution_dpi, diff --git a/src/driver-hidpp20.c b/src/driver-hidpp20.c index 8f0b1f41..1ac3ca4f 100644 --- a/src/driver-hidpp20.c +++ b/src/driver-hidpp20.c @@ -56,7 +56,6 @@ struct hidpp20drv_data { struct hidpp20_device *dev; unsigned long capabilities; - unsigned long exported_capabilities; unsigned num_sensors; struct hidpp20_sensor *sensors; unsigned num_controls; @@ -331,31 +330,6 @@ hidpp20drv_write_button(struct ratbag_button *button, return -ENOTSUP; } -static int -hidpp20drv_has_capability(const struct ratbag_device *device, - enum ratbag_device_capability cap) -{ - /* - * We need to force the non const, but we will not change anything, - * promise! - */ - struct hidpp20drv_data *drv_data = ratbag_get_drv_data((struct ratbag_device *)device); - - if (cap == RATBAG_DEVICE_CAP_NONE) - abort(); - - return !!(drv_data->exported_capabilities & (1UL << cap)); -} - -static inline void -hidpp20drv_set_exported_capability(const struct ratbag_device *device, - enum ratbag_device_capability cap) -{ - struct hidpp20drv_data *drv_data = ratbag_get_drv_data((struct ratbag_device *)device); - - drv_data->exported_capabilities |= (1UL << cap); -} - static int hidpp20drv_current_profile(struct ratbag_device *device) { @@ -683,7 +657,7 @@ hidpp20drv_init_feature(struct ratbag_device *device, uint16_t feature) } case HIDPP_PAGE_ADJUSTABLE_DPI: { log_debug(ratbag, "device has adjustable dpi\n"); - hidpp20drv_set_exported_capability(device, RATBAG_DEVICE_CAP_SWITCHABLE_RESOLUTION); + ratbag_device_set_capability(device, RATBAG_DEVICE_CAP_SWITCHABLE_RESOLUTION); drv_data->capabilities |= HIDPP_CAP_SWITCHABLE_RESOLUTION_2201; /* we read the profile once to get the correct number of * supported resolutions. */ @@ -695,7 +669,7 @@ hidpp20drv_init_feature(struct ratbag_device *device, uint16_t feature) case HIDPP_PAGE_SPECIAL_KEYS_BUTTONS: { log_debug(ratbag, "device has programmable keys/buttons\n"); drv_data->capabilities |= HIDPP_CAP_BUTTON_KEY_1b04; - hidpp20drv_set_exported_capability(device, RATBAG_DEVICE_CAP_BUTTON_KEY); + ratbag_device_set_capability(device, RATBAG_DEVICE_CAP_BUTTON_KEY); /* we read the profile once to get the correct number of * supported buttons. */ if (!hidpp20drv_read_special_key_mouse(device)) @@ -738,9 +712,6 @@ hidpp20drv_init_feature(struct ratbag_device *device, uint16_t feature) case HIDPP_PAGE_ONBOARD_PROFILES: { log_debug(ratbag, "device has onboard profiles\n"); drv_data->capabilities |= HIDPP_CAP_ONBOARD_PROFILES_8100; - hidpp20drv_set_exported_capability(device, RATBAG_DEVICE_CAP_BUTTON_KEY); - hidpp20drv_set_exported_capability(device, RATBAG_DEVICE_CAP_SWITCHABLE_RESOLUTION); - hidpp20drv_set_exported_capability(device, RATBAG_DEVICE_CAP_SWITCHABLE_PROFILE); rc = hidpp20_onboard_profiles_allocate(drv_data->dev, &drv_data->profiles); if (rc < 0) @@ -884,7 +855,6 @@ struct ratbag_driver hidpp20_driver = { .write_profile = hidpp20drv_write_profile, .set_active_profile = hidpp20drv_set_current_profile, .set_default_profile = hidpp20drv_set_default_profile, - .has_capability = hidpp20drv_has_capability, .read_button = hidpp20drv_read_button, .write_button = hidpp20drv_write_button, .write_resolution_dpi = hidpp20drv_write_resolution_dpi, diff --git a/src/driver-roccat.c b/src/driver-roccat.c index 97b65c41..ef563265 100644 --- a/src/driver-roccat.c +++ b/src/driver-roccat.c @@ -307,23 +307,6 @@ roccat_wait_ready(struct ratbag_device *device) return -ETIMEDOUT; } -static int -roccat_has_capability(const struct ratbag_device *device, - enum ratbag_device_capability cap) -{ - switch (cap) { - case RATBAG_DEVICE_CAP_NONE: - case RATBAG_DEVICE_CAP_DEFAULT_PROFILE: - return 0; - case RATBAG_DEVICE_CAP_SWITCHABLE_RESOLUTION: - case RATBAG_DEVICE_CAP_SWITCHABLE_PROFILE: - case RATBAG_DEVICE_CAP_BUTTON_KEY: - case RATBAG_DEVICE_CAP_BUTTON_MACROS: - return 1; - } - return 0; -} - static int roccat_current_profile(struct ratbag_device *device) { @@ -816,6 +799,8 @@ roccat_probe(struct ratbag_device *device) ROCCAT_NUM_DPI, ROCCAT_BUTTON_MAX + 1); + ratbag_device_set_capability(device, RATBAG_DEVICE_CAP_BUTTON_MACROS); + active_idx = roccat_current_profile(device); if (active_idx < 0) { log_error(device->ratbag, @@ -861,7 +846,6 @@ struct ratbag_driver roccat_driver = { .read_profile = roccat_read_profile, .write_profile = roccat_write_profile, .set_active_profile = roccat_set_current_profile, - .has_capability = roccat_has_capability, .read_button = roccat_read_button, .write_button = roccat_write_button, .write_resolution_dpi = roccat_write_resolution_dpi, diff --git a/src/driver-test.c b/src/driver-test.c index fb52d520..7447e8a5 100644 --- a/src/driver-test.c +++ b/src/driver-test.c @@ -28,24 +28,6 @@ #include "libratbag-private.h" #include "libratbag-test.h" -static int -test_has_capability(const struct ratbag_device *device, - enum ratbag_device_capability cap) -{ - switch (cap) { - case RATBAG_DEVICE_CAP_NONE: - abort(); - break; - case RATBAG_DEVICE_CAP_SWITCHABLE_RESOLUTION: - case RATBAG_DEVICE_CAP_BUTTON_KEY: - case RATBAG_DEVICE_CAP_BUTTON_MACROS: - return (device->num_profiles > 1); - default: - return 0; - } - return 0; -} - static void test_read_profile(struct ratbag_profile *profile, unsigned int index) { @@ -81,6 +63,17 @@ test_write_profile(struct ratbag_profile *profile) return 0; } +static int +test_set_active_profile(struct ratbag_device *device, unsigned int index) +{ + struct ratbag_test_device *d = ratbag_get_drv_data(device); + + /* check if the device is still valid */ + assert(d != NULL); + assert(index < d->num_profiles); + return 0; +} + static int test_fake_probe(struct ratbag_device *device) { @@ -97,6 +90,9 @@ test_probe(struct ratbag_device *device, void *data) test_device->num_profiles, test_device->num_resolutions, test_device->num_buttons); + if (test_device->num_profiles > 1) + ratbag_device_set_capability(device, RATBAG_DEVICE_CAP_BUTTON_MACROS); + return 0; } @@ -120,8 +116,7 @@ struct ratbag_driver test_driver = { .remove = test_remove, .read_profile = test_read_profile, .write_profile = test_write_profile, - .set_active_profile = NULL, - .has_capability = test_has_capability, + .set_active_profile = test_set_active_profile, .read_button = NULL, .write_button = NULL, .write_resolution_dpi = NULL, diff --git a/src/libratbag-private.h b/src/libratbag-private.h index 74794128..c6e5b9ca 100644 --- a/src/libratbag-private.h +++ b/src/libratbag-private.h @@ -89,6 +89,7 @@ struct ratbag_device { struct input_id ids; struct ratbag_driver *driver; struct ratbag *ratbag; + unsigned long capabilities; unsigned num_profiles; struct list profiles; @@ -162,17 +163,6 @@ struct ratbag_driver { */ int (*set_default_profile)(struct ratbag_device *device, unsigned int index); - /** - * This should return a boolean whether or not the device - * supports the given capability. - * - * In most cases, the .probe() should store a list of capabilities - * for each device, but most of the time, it can be statically - * stored. - */ - int (*has_capability)(const struct ratbag_device *device, - enum ratbag_device_capability cap); - /** * For the given button, fill in the struct ratbag_button * with the available information. @@ -334,6 +324,10 @@ ratbag_device_init_profiles(struct ratbag_device *device, unsigned int num_resolutions, unsigned int num_buttons); +void +ratbag_device_set_capability(struct ratbag_device *device, + enum ratbag_device_capability cap); + static inline void ratbag_profile_set_drv_data(struct ratbag_profile *profile, void *drv_data) { diff --git a/src/libratbag.c b/src/libratbag.c index c6cedde9..bfd7068b 100644 --- a/src/libratbag.c +++ b/src/libratbag.c @@ -606,6 +606,17 @@ ratbag_device_init_profiles(struct ratbag_device *device, device->num_profiles = num_profiles; + if (num_profiles > 1) { + ratbag_device_set_capability(device, RATBAG_DEVICE_CAP_SWITCHABLE_PROFILE); + + /* having more than one profile means we can remap the buttons + * at least */ + ratbag_device_set_capability(device, RATBAG_DEVICE_CAP_BUTTON_KEY); + } + + if (num_resolutions > 1) + ratbag_device_set_capability(device, RATBAG_DEVICE_CAP_SWITCHABLE_RESOLUTION); + return 0; } @@ -694,12 +705,21 @@ ratbag_device_get_num_buttons(struct ratbag_device *device) return device->num_buttons; } +void +ratbag_device_set_capability(struct ratbag_device *device, + enum ratbag_device_capability cap) +{ + device->capabilities |= (1UL << cap); +} + LIBRATBAG_EXPORT int ratbag_device_has_capability(const struct ratbag_device *device, enum ratbag_device_capability cap) { - assert(device->driver->has_capability); - return device->driver->has_capability(device, cap); + if (cap == RATBAG_DEVICE_CAP_NONE) + abort(); + + return !!(device->capabilities & (1UL << cap)); } LIBRATBAG_EXPORT int