From 186d626c40ccd268607b6b661d8d9a2d6c1589a8 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Fri, 29 Sep 2017 08:26:12 +1000 Subject: [PATCH] Remove RATBAG_LED_TYPE_UNKNOWN from the enum list A driver is supposed to know what type an LED is, unknown doesn't help anyone here. So replace this with a private #define and abort when a driver doesn't set the type. This also adds a requirement that any driver initializing a LED must have the read_led() function, otherwise we can't fill it in correctly. And bump the enum up by one, the first valid value should start at 1 (or some other non-boolean value). Related #365 Fixes #364 Signed-off-by: Peter Hutterer --- ratbagd/ratbagd-test.c | 9 ++++++--- src/driver-test.c | 6 +++++- src/libratbag-private.h | 2 ++ src/libratbag.c | 31 +++++++++++++++++++++++++++++-- src/libratbag.h | 3 +-- test/test-device.c | 8 ++++++-- tools/ratbagctl.in | 1 - tools/ratbagctl.test.in | 20 ++++++++++---------- tools/ratbagd.py | 11 +++++------ tools/shared.c | 8 ++++---- 10 files changed, 68 insertions(+), 31 deletions(-) diff --git a/ratbagd/ratbagd-test.c b/ratbagd/ratbagd-test.c index 1c1838e0..6db38bed 100644 --- a/ratbagd/ratbagd-test.c +++ b/ratbagd/ratbagd-test.c @@ -75,19 +75,22 @@ static const struct ratbag_test_device ratbagd_test_device_descr = { .mode = RATBAG_LED_OFF, .color = { .red = 255, .green = 0, .blue = 0 }, .hz = 1, - .brightness = 20 + .brightness = 20, + .type = RATBAG_LED_TYPE_LOGO, }, { .mode = RATBAG_LED_ON, .color = { .red = 255, .green = 0, .blue = 0 }, .hz = 1, - .brightness = 20 + .brightness = 20, + .type = RATBAG_LED_TYPE_SIDE, }, { .mode = RATBAG_LED_CYCLE, .color = { .red = 255, .green = 255, .blue = 0 }, .hz = 3, - .brightness = 40 + .brightness = 40, + .type = RATBAG_LED_TYPE_SIDE, } }, }, diff --git a/src/driver-test.c b/src/driver-test.c index 4d86d869..f0a6685c 100644 --- a/src/driver-test.c +++ b/src/driver-test.c @@ -171,7 +171,11 @@ test_read_led(struct ratbag_led *led) led->color.blue = t_led.color.blue; led->hz = t_led.hz; led->brightness = t_led.brightness; - led->type = RATBAG_LED_TYPE_LOGO; + + /* The led type has to be the same anyway so make writing test + * devices easier by always getting the first profile's LED type. + */ + led->type = d->profiles[0].leds[led->index].type; } static int diff --git a/src/libratbag-private.h b/src/libratbag-private.h index 611b78b0..57fe7404 100644 --- a/src/libratbag-private.h +++ b/src/libratbag-private.h @@ -32,6 +32,8 @@ #include "libratbag-util.h" #include "libratbag-hidraw.h" +#define RATBAG_LED_TYPE_UNKNOWN 0 + static inline void cleanup_device(struct ratbag_device **d) { diff --git a/src/libratbag.c b/src/libratbag.c index 15440376..34a0cbfb 100644 --- a/src/libratbag.c +++ b/src/libratbag.c @@ -283,6 +283,28 @@ ratbag_sanity_check_device(struct ratbag_device *device) goto out; } + /* Require LED to be the same type in each profile */ + for (i = 1; i < nprofiles; i++) { + _cleanup_profile_ struct ratbag_profile *p0 = ratbag_device_get_profile(device, 0); + _cleanup_profile_ struct ratbag_profile *p = ratbag_device_get_profile(device, i); + unsigned int nleds = ratbag_device_get_num_leds(device); + + for (unsigned int j = 0; j < nleds; j++) { + _cleanup_led_ struct ratbag_led *p0_led = ratbag_profile_get_led(p0, j); + _cleanup_led_ struct ratbag_led *led = ratbag_profile_get_led(p, j); + + if (p0_led->type != led->type) { + log_bug_libratbag(ratbag, + "%s: mismatching LED types (profile %d led %d)\n", + device->name, + p->index, + led->type); + goto out; + } + } + + } + rc = true; out: @@ -600,11 +622,16 @@ ratbag_create_led(struct ratbag_profile *profile, unsigned int index) led->profile = profile; led->index = index; led->colordepth = RATBAG_LED_COLORDEPTH_RGB_888; + led->type = RATBAG_LED_TYPE_UNKNOWN; list_insert(&profile->leds, &led->link); - if (device->driver->read_led) - device->driver->read_led(led); + /* If you init a LED, you need to provide this function */ + assert(device->driver->read_led); + device->driver->read_led(led); + + /* The driver must fill this in correctly */ + assert(led->type != RATBAG_LED_TYPE_UNKNOWN); return led; } diff --git a/src/libratbag.h b/src/libratbag.h index 2f3adb58..0497d4e7 100644 --- a/src/libratbag.h +++ b/src/libratbag.h @@ -1458,8 +1458,7 @@ enum ratbag_led_mode { * LED types, usually based on their physical location */ enum ratbag_led_type { - RATBAG_LED_TYPE_UNKNOWN = -1, - RATBAG_LED_TYPE_LOGO = 0, + RATBAG_LED_TYPE_LOGO = 1, RATBAG_LED_TYPE_SIDE, RATBAG_LED_TYPE_BATTERY, RATBAG_LED_TYPE_DPI, diff --git a/test/test-device.c b/test/test-device.c index be2494f4..828b5203 100644 --- a/test/test-device.c +++ b/test/test-device.c @@ -62,6 +62,10 @@ const struct ratbag_test_device sane_device = { { .xres = 200, .yres = 300, .hz = 1000 }, { .xres = 300, .yres = 400, .hz = 1000 }, }, + .leds = { + { .type = RATBAG_LED_TYPE_SIDE, }, + { .type = RATBAG_LED_TYPE_LOGO, }, + }, .active = true, .dflt = false, }, @@ -85,13 +89,13 @@ const struct ratbag_test_device sane_device = { .mode = RATBAG_LED_ON, .color = { .red = 255, .green = 0, .blue = 0 }, .hz = 1, - .brightness = 20 + .brightness = 20, }, { .mode = RATBAG_LED_CYCLE, .color = { .red = 255, .green = 255, .blue = 0 }, .hz = 3, - .brightness = 40 + .brightness = 40, } }, .active = false, diff --git a/tools/ratbagctl.in b/tools/ratbagctl.in index 0ea2946e..f58d7ef9 100755 --- a/tools/ratbagctl.in +++ b/tools/ratbagctl.in @@ -126,7 +126,6 @@ def print_led(d, p, l, level): RatbagdLed.MODE_ON: "on", } types = { - RatbagdLed.TYPE_UNKNOWN: "unknown", RatbagdLed.TYPE_LOGO: "logo", RatbagdLed.TYPE_SIDE: "side", } diff --git a/tools/ratbagctl.test.in b/tools/ratbagctl.test.in index 3831f470..f89825ba 100755 --- a/tools/ratbagctl.test.in +++ b/tools/ratbagctl.test.in @@ -464,9 +464,9 @@ class TestRatbagCtlLED(TestRatbagCtl): r = self.launch_good_test("led 0 get test_device") self.assertEqual(r, "LED: 0 type: logo, depth: rgb, mode: off") r = self.launch_good_test("led 1 get test_device") - self.assertEqual(r, "LED: 1 type: logo, depth: rgb, mode: on, color: ff0000") + self.assertEqual(r, "LED: 1 type: side, depth: rgb, mode: on, color: ff0000") r = self.launch_good_test("led 2 get test_device") - self.assertEqual(r, "LED: 2 type: logo, depth: rgb, mode: cycle, rate: 3, brightness: 40") + self.assertEqual(r, "LED: 2 type: side, depth: rgb, mode: cycle, rate: 3, brightness: 40") self.launch_fail_test("led 0 get 10 test_device") self.launch_fail_test("led 10 get test_device") self.launch_fail_test("led 0 get test_device X") @@ -487,15 +487,15 @@ class TestRatbagCtlLED(TestRatbagCtl): def test_led_n_set_color(self): self.setProfile(0) r = self.launch_good_test("led 1 get test_device") - self.assertEqual(r, "LED: 1 type: logo, depth: rgb, mode: on, color: ff0000") + self.assertEqual(r, "LED: 1 type: side, depth: rgb, mode: on, color: ff0000") r = self.launch_good_test("led 1 set color 00ff00 test_device") self.assertEqual(r, '') r = self.launch_good_test("led 1 get test_device") - self.assertEqual(r, "LED: 1 type: logo, depth: rgb, mode: on, color: 00ff00") + self.assertEqual(r, "LED: 1 type: side, depth: rgb, mode: on, color: 00ff00") r = self.launch_good_test("led 1 set color 0x0000ff test_device") self.assertEqual(r, '') r = self.launch_good_test("led 1 get test_device") - self.assertEqual(r, "LED: 1 type: logo, depth: rgb, mode: on, color: 0000ff") + self.assertEqual(r, "LED: 1 type: side, depth: rgb, mode: on, color: 0000ff") self.launch_fail_test("led 1 set color g0ff00 test_device") self.launch_fail_test("led 1 set color test_device") self.launch_fail_test("led 1 set color 00ff00 test_device X") @@ -504,11 +504,11 @@ class TestRatbagCtlLED(TestRatbagCtl): def test_led_n_set_rate(self): self.setProfile(0) r = self.launch_good_test("led 2 get test_device") - self.assertEqual(r, "LED: 2 type: logo, depth: rgb, mode: cycle, rate: 3, brightness: 40") + self.assertEqual(r, "LED: 2 type: side, depth: rgb, mode: cycle, rate: 3, brightness: 40") r = self.launch_good_test("led 2 set rate 100 test_device") self.assertEqual(r, '') r = self.launch_good_test("led 2 get test_device") - self.assertEqual(r, "LED: 2 type: logo, depth: rgb, mode: cycle, rate: 100, brightness: 40") + self.assertEqual(r, "LED: 2 type: side, depth: rgb, mode: cycle, rate: 100, brightness: 40") self.launch_fail_test("led 2 set rate this_is_not_an_int test_device") self.launch_fail_test("led 2 set rate test_device") self.launch_fail_test("led 2 set rate 100 test_device X") @@ -517,11 +517,11 @@ class TestRatbagCtlLED(TestRatbagCtl): def test_led_n_set_brightness(self): self.setProfile(2) r = self.launch_good_test("led 1 get test_device") - self.assertEqual(r, "LED: 1 type: logo, depth: rgb, mode: cycle, rate: 3, brightness: 40") + self.assertEqual(r, "LED: 1 type: side, depth: rgb, mode: cycle, rate: 3, brightness: 40") r = self.launch_good_test("led 1 set brightness 100 test_device") self.assertEqual(r, '') r = self.launch_good_test("led 1 get test_device") - self.assertEqual(r, "LED: 1 type: logo, depth: rgb, mode: cycle, rate: 3, brightness: 100") + self.assertEqual(r, "LED: 1 type: side, depth: rgb, mode: cycle, rate: 3, brightness: 100") self.launch_fail_test("led 1 set brightness this_is_not_an_int test_device") self.launch_fail_test("led 1 set brightness test_device") self.launch_fail_test("led 1 set brightness 100 test_device X") @@ -554,7 +554,7 @@ class TestRatbagCtlLED(TestRatbagCtl): def test_prefix_led(self): self.setProfile(2) r = self.launch_good_test("Profile 1 Led 1 get test_device") - self.assertEqual(r, "LED: 1 type: logo, depth: rgb, mode: off") + self.assertEqual(r, "LED: 1 type: side, depth: rgb, mode: off") self.launch_fail_test("Profile 1 Profile 2 Led 1 get test_device") self.launch_fail_test("Profile 1 Resolution 2 Led 1 get test_device") diff --git a/tools/ratbagd.py b/tools/ratbagd.py index bcbb4fef..dead3b96 100644 --- a/tools/ratbagd.py +++ b/tools/ratbagd.py @@ -880,11 +880,10 @@ class RatbagdMacro(GObject.Object): class RatbagdLed(_RatbagdDBus): """Represents a ratbagd led.""" - TYPE_UNKNOWN = -1 - TYPE_LOGO = 0 - TYPE_SIDE = 1 - TYPE_BATTERY = 2 - TYPE_DPI = 3 + TYPE_LOGO = 1 + TYPE_SIDE = 2 + TYPE_BATTERY = 3 + TYPE_DPI = 4 MODE_OFF = 0 MODE_ON = 1 @@ -932,7 +931,7 @@ class RatbagdLed(_RatbagdDBus): @GObject.Property def type(self): - """An enum describing this led's type, one of RatbagdLed.TYPE_UNKNOWN, + """An enum describing this led's type, RatbagdLed.TYPE_LOGO or RatbagdLed.TYPE_SIDE.""" return self._get_dbus_property("Type") diff --git a/tools/shared.c b/tools/shared.c index be627133..570140ae 100644 --- a/tools/shared.c +++ b/tools/shared.c @@ -51,12 +51,9 @@ udev_device_from_path(struct udev *udev, const char *path) const char* led_type_to_str(enum ratbag_led_type type) { - const char *str = "UNKNOWN"; + const char *str = NULL; switch(type) { - case RATBAG_LED_TYPE_UNKNOWN: - str = "unknown"; - break; case RATBAG_LED_TYPE_LOGO: str = "logo"; break; @@ -69,6 +66,9 @@ led_type_to_str(enum ratbag_led_type type) case RATBAG_LED_TYPE_DPI: str = "dpi"; break; + default: + assert(!"Invalid LED type"); + break; } return str;