(ckb-daemon) Don't constantly wake to poll /dev nodes

(ckb-daemon) Fixed LED timing on some devices
This significantly reduces CPU usage (down to ~0%) when the GUI is not running. There's a slight performance improvement when the GUI is active.

VERSION

@@ -1 +1 @@
-beta-v0.2.2+t06
+beta-v0.2.2+t07

src/ckb-daemon/command.c

@@ -173,13 +173,14 @@ int readcmd(usbdevice* kb, const char* line){
             continue;
         }
         case FPS: {
-            // USB command delay (1 - 10ms)
+            // USB command delay (2 - 10ms)
             uint framerate;
             if(sscanf(word, "%u", &framerate) == 1 && framerate > 0){
-                // Max messages per second: 5 RGB + 1 indicator + 1 wait
-                uint delay = 1000 / framerate / 7;
-                if(delay < 1)
-                    delay = 1;
+                // Not all devices require the same number of messages per frame; select delay appropriately
+                uint per_frame = IS_MOUSE_DEV(kb) ? 2 : IS_STRAFE(kb) ? 14 : 5;
+                uint delay = 1000 / framerate / per_frame;
+                if(delay < 2)
+                    delay = 2;
                 else if(delay > 10)
                     delay = 10;
                 kb->usbdelay = delay;

src/ckb-daemon/devnode.c

@@ -168,7 +168,9 @@ static int _mkdevpath(usbdevice* kb){
         // Create command FIFO
         char inpath[sizeof(path) + 4];
         snprintf(inpath, sizeof(inpath), "%s/cmd", path);
-        if(mkfifo(inpath, gid >= 0 ? S_CUSTOM : S_READWRITE) != 0 || (kb->infifo = open(inpath, O_RDONLY | O_NONBLOCK) + 1) == 0){
+        if(mkfifo(inpath, gid >= 0 ? S_CUSTOM : S_READWRITE) != 0
+                // Open the node in RDWR mode because RDONLY will lock the thread
+                || (kb->infifo = open(inpath, O_RDWR) + 1) == 0){
             // Add one to the FD because 0 is a valid descriptor, but ckb uses 0 for uninitialized devices
             ckb_err("Unable to create %s: %s\n", inpath, strerror(errno));
             rm_recursive(path);
@@ -256,9 +258,11 @@ int mkdevpath(usbdevice* kb){
 int rmdevpath(usbdevice* kb){
     euid_guard_start;
     int index = INDEX_OF(kb, keyboard);
-    if(kb->infifo != 0)
+    if(kb->infifo != 0){
+        write(kb->infifo - 1, "\n", 1); // hack to prevent the FIFO thread from perma-blocking
         close(kb->infifo - 1);
-    kb->infifo = 0;
+        kb->infifo = 0;
+    }
     for(int i = 0; i < OUTFIFO_MAX; i++)
         _rmnotifynode(kb, i);
     char path[strlen(devpath) + 2];

src/ckb-daemon/input.c

@@ -135,21 +135,32 @@ void inputupdate(usbdevice* kb){
 }
 
 void updateindicators_kb(usbdevice* kb, int force){
-    uchar old = kb->hw_ileds;
-    os_updateindicators(kb, force);
+    // Read current hardware indicator state (set externally)
+    uchar old = kb->ileds, hw_old = kb->hw_ileds_old;
+    uchar new = kb->hw_ileds, hw_new = new;
+    // Update them if needed
+    if(kb->active){
+        usbmode* mode = kb->profile->currentmode;
+        new = (new & ~mode->ioff) | mode->ion;
+    }
+    kb->ileds = new;
+    kb->hw_ileds_old = hw_new;
+    if(old != new || force){
+        DELAY_SHORT(kb);
+        os_sendindicators(kb);
+    }
     // Print notifications if desired
     if(!kb->active)
         return;
-    uchar new = kb->hw_ileds;
     usbmode* mode = kb->profile->currentmode;
     uchar indicators[] = { I_NUM, I_CAPS, I_SCROLL };
     for(unsigned i = 0; i < sizeof(indicators) / sizeof(uchar); i++){
         uchar mask = indicators[i];
-        if((old & mask) == (new & mask))
+        if((hw_old & mask) == (hw_new & mask))
             continue;
         for(int notify = 0; notify < OUTFIFO_MAX; notify++){
             if(mode->inotify[notify] & mask)
-                nprintind(kb, notify, mask, new & mask);
+                nprintind(kb, notify, mask, hw_new & mask);
         }
     }
 }

src/ckb-daemon/input.h

@@ -45,7 +45,9 @@ void os_keypress(usbdevice* kb, int scancode, int down);
 void os_mousemove(usbdevice* kb, int x, int y);
 // Synchronize input (called after sending key presses)
 void os_isync(usbdevice* kb);
-// Updates indicator state. Should read state, update ileds (applying mask for current mode as appropriate) and send control message to keyboard
-void os_updateindicators(usbdevice* kb, int force);
+
+// Perform OS-specific setup for indicator lights. Called when the device is created. Return 0 on success.
+int os_setupindicators(usbdevice* kb);
+
 
 #endif  // INPUT_H

src/ckb-daemon/input_linux.c

@@ -1,3 +1,4 @@
+#include "command.h"
 #include "device.h"
 #include "input.h"
 
@@ -6,10 +7,10 @@
 // Xorg has buggy handling of combined keyboard + mouse devices, so instead we should create two separate devices:
 // One for keyboard events, one for mouse.
 int uinputopen(struct uinput_user_dev* indev, int mouse){
-    int fd = open("/dev/uinput", O_RDWR | O_NONBLOCK);
+    int fd = open("/dev/uinput", O_RDWR);
     if(fd < 0){
         // If that didn't work, try /dev/input/uinput instead
-        fd = open("/dev/input/uinput", O_RDWR | O_NONBLOCK);
+        fd = open("/dev/input/uinput", O_RDWR);
         if(fd < 0){
             ckb_err("Failed to open uinput: %s\n", strerror(errno));
             return 0;
@@ -148,9 +149,10 @@ void os_isync(usbdevice* kb){
         ckb_warn("uinput write failed: %s\n", strerror(errno));
 }
 
-void os_updateindicators(usbdevice* kb, int force){
-    // Read LED events from the device
-    uchar ileds = kb->hw_ileds;
+void* _ledthread(void* ctx){
+    usbdevice* kb = ctx;
+    uchar ileds = 0;
+    // Read LED events from the uinput device
     struct input_event event;
     while(read(kb->uinput_kb - 1, &event, sizeof(event)) > 0){
         if(event.type == EV_LED && event.code < 8){
@@ -160,19 +162,27 @@ void os_updateindicators(usbdevice* kb, int force){
             else
                 ileds &= ~which;
         }
+        // Update them if needed
+        pthread_mutex_lock(dmutex(kb));
+        if(kb->hw_ileds != ileds){
+            kb->hw_ileds = ileds;
+            kb->vtable->updateindicators(kb, 0);
+        }
+        pthread_mutex_unlock(dmutex(kb));
     }
-    kb->hw_ileds = ileds;
-    // Update them if needed
-    if(kb->active){
-        usbmode* mode = kb->profile->currentmode;
-        ileds = (ileds & ~mode->ioff) | mode->ion;
-    }
-    if(force || ileds != kb->ileds){
-        kb->ileds = ileds;
-        DELAY_SHORT(kb);
-        struct usbdevfs_ctrltransfer transfer = { 0x21, 0x09, 0x0200, 0x00, 1, 5000, &kb->ileds };
-        ioctl(kb->handle - 1, USBDEVFS_CONTROL, &transfer);
-    }
+    return 0;
+}
+
+int os_setupindicators(usbdevice* kb){
+    // Initialize LEDs to all off
+    kb->hw_ileds = kb->hw_ileds_old = kb->ileds = 0;
+    // Create and detach thread to read LED events
+    pthread_t thread;
+    int err = pthread_create(&thread, 0, _ledthread, kb);
+    if(err != 0)
+        return err;
+    pthread_detach(thread);
+    return 0;
 }
 
 #endif

src/ckb-daemon/input_mac.c

@@ -1,3 +1,4 @@
+#include "command.h"
 #include "device.h"
 #include "input.h"
 
@@ -271,6 +272,27 @@ void keyretrigger(CFRunLoopTimerRef timer, void* info){
     setkrtimer(kb);
 }
 
+// Unlike Linux, OSX keyboards have independent caps lock states. This means they're set by the driver itself so we don't poll for external events.
+// However, updating indicator state requires locking dmutex and we never want to do that in the input thread.
+// Instead, we launch a single-shot thread to update the state.
+static void* indicator_update(void* context){
+    usbdevice* kb = context;
+    pthread_mutex_lock(dmutex(kb));
+    {
+        pthread_mutex_lock(imutex(kb));
+        IOOptionBits modifiers = kb->modifiers;
+        // Allow the thread to be spawned again
+        kb->indicthread = 0;
+        pthread_mutex_unlock(imutex(kb));
+        // Num lock on, Caps dependent on modifier state
+        uchar ileds = 1 | !!(modifiers & NX_ALPHASHIFTMASK) << 1;
+        kb->hw_ileds = ileds;
+        kb->vtable->updateindicators(kb, 0);
+    }
+    pthread_mutex_unlock(dmutex(kb));
+    return 0;
+}
+
 void os_keypress(usbdevice* kb, int scancode, int down){
     // Trigger any pending repeats first
     keyretrigger(NULL, kb);
@@ -325,6 +347,14 @@ void os_keypress(usbdevice* kb, int scancode, int down){
         if(down)
             kb->modifiers ^= NX_ALPHASHIFTMASK;
         isMod = 1;
+        // Detach a thread to update the indicator state
+        if(!kb->indicthread){
+            // The thread is only spawned if kb->indicthread is null.
+            // Due to the logic inside the thread, this means that it could theoretically be spawned twice, but never a third time.
+            // Moreover, if it is spawned more than once, the indicator state will remain correct due to dmutex staying locked.
+            if(!pthread_create(&kb->indicthread, 0, indicator_update, kb))
+                pthread_detach(kb->inputthread);
+        }
     }
     else if(scancode == KEY_LEFTSHIFT) mod = NX_DEVICELSHIFTKEYMASK;
     else if(scancode == KEY_RIGHTSHIFT) mod = NX_DEVICERSHIFTKEYMASK;
@@ -373,43 +403,10 @@ void os_isync(usbdevice* kb){
     // OSX doesn't have any equivalent to the SYN_ events
 }
 
-void os_updateindicators(usbdevice* kb, int force){
+int os_setupindicators(usbdevice* kb){
     // Set NumLock on permanently
-    char ileds = 1;
-    // Set Caps Lock if enabled. Unlike Linux, OSX keyboards have independent caps lock states, so
-    // we use the last-assigned value rather than fetching it from the system
-    if(kb->modifiers & kCGEventFlagMaskAlphaShift)
-        ileds |= 2;
-    kb->hw_ileds = ileds;
-    if(kb->active){
-        usbmode* mode = kb->profile->currentmode;
-        ileds = (ileds & ~mode->ioff) | mode->ion;
-    }
-    if(force || ileds != kb->ileds){
-        kb->ileds = ileds;
-        // Get a list of LED elements from handle 0
-        long ledpage = kHIDPage_LEDs;
-        const void* keys[] = { CFSTR(kIOHIDElementUsagePageKey) };
-        const void* values[] = { CFNumberCreate(kCFAllocatorDefault, kCFNumberLongType, &ledpage) };
-        CFDictionaryRef matching = CFDictionaryCreate(kCFAllocatorDefault, keys, values, 1, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks);
-        CFRelease(values[0]);
-        CFArrayRef leds;
-        kern_return_t res = (*kb->handles[0])->copyMatchingElements(kb->handles[0], matching, &leds, 0);
-        CFRelease(matching);
-        if(res != kIOReturnSuccess)
-            return;
-        // Iterate through them and update the LEDs which have changed
-        DELAY_SHORT(kb);
-        CFIndex count = CFArrayGetCount(leds);
-        for(CFIndex i = 0; i < count; i++){
-            IOHIDElementRef led = (void*)CFArrayGetValueAtIndex(leds, i);
-            uint32_t usage = IOHIDElementGetUsage(led);
-            IOHIDValueRef value = IOHIDValueCreateWithIntegerValue(kCFAllocatorDefault, led, 0, !!(ileds & (1 << (usage - 1))));
-            (*kb->handles[0])->setValue(kb->handles[0], led, value, 5000, 0, 0, 0);
-            CFRelease(value);
-        }
-        CFRelease(leds);
-    }
+    kb->hw_ileds = kb->hw_ileds_old = kb->ileds = 1;
+    return 0;
 }
 
 #endif

src/ckb-daemon/led.c

@@ -1,3 +1,4 @@
+#include "command.h"
 #include "led.h"
 #include "profile.h"
 #include "usb.h"
@@ -39,20 +40,23 @@ void cmd_ioff(usbdevice* kb, usbmode* mode, int dummy1, int dummy2, const char*
     // Add the bits to ioff, remove them from ion
     mode->ioff |= bits;
     mode->ion &= ~bits;
+    kb->vtable->updateindicators(kb, 0);
 }
 
 void cmd_ion(usbdevice* kb, usbmode* mode, int dummy1, int dummy2, const char* led){
     uchar bits = iselect(led);
     // Remove the bits from ioff, add them to ion
     mode->ioff &= ~bits;
     mode->ion |= bits;
+    kb->vtable->updateindicators(kb, 0);
 }
 
 void cmd_iauto(usbdevice* kb, usbmode* mode, int dummy1, int dummy2, const char* led){
     uchar bits = iselect(led);
     // Remove the bits from both ioff and ion
     mode->ioff &= ~bits;
     mode->ion &= ~bits;
+    kb->vtable->updateindicators(kb, 0);
 }
 
 void cmd_inotify(usbdevice* kb, usbmode* mode, int nnumber, int dummy, const char* led){

src/ckb-daemon/led_keyboard.c

@@ -85,9 +85,8 @@ int updatergb_kb(usbdevice* kb, int force){
         return 0;
     lastlight->forceupdate = newlight->forceupdate = 0;
 
-    /*if(kb->fwversion >= 0x0120){ */
-    if (IS_STRAFE(kb)){
-        // update strafe sidelights if necessary
+    if(IS_STRAFE(kb)){
+        // Update strafe sidelights if necessary
         if(lastlight->sidelight != newlight->sidelight) {
             uchar data_pkt[2][MSG_SIZE] = {
                  { 0x07, 0x05, 0x08, 0x00, 0x00 },
@@ -120,8 +119,7 @@ int updatergb_kb(usbdevice* kb, int force){
         if(!usbsend(kb, data_pkt[0], 12))
             return -1;
     } else {
-    // 16.8M color lighting causes flickering and color glitches. Don't use it for this.
-    // Maybe in a future version this can be re-added as an advanced feature.
+        // On older keyboards it looks flickery and causes lighting glitches, so we don't use it.
         uchar data_pkt[5][MSG_SIZE] = {
             { 0x7f, 0x01, 60, 0 },
             { 0x7f, 0x02, 60, 0 },

src/ckb-daemon/structures.h

@@ -197,6 +197,7 @@ typedef struct {
     short lastkeypress;
     uchar mousestate;
     char scroll_rate;
+    pthread_t indicthread;
 #endif
     // Thread used for USB/devnode communication. To close: lock mutexes, set handle to zero, unlock, then wait for thread to stop
     pthread_t thread;
@@ -229,7 +230,7 @@ typedef struct {
     // Current input state
     usbinput input;
     // Indicator LED state
-    uchar hw_ileds, ileds;
+    uchar hw_ileds, hw_ileds_old, ileds;
     // Color dithering in use
     char dither;
 } usbdevice;