Don't make led_driver_state_t members volatile. Uncomment the body of LedSlaveDriver_EnableLeds()

Extend the behavior of keys upon layer changes to keymap changes

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is loosely based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to the [UHK Versioning](VERSIONING.md) conventions.
 
+## [8.5.3] - 2018-10-20
+
+Device Protocol: 4.5.0 | Module Protocol: 4.0.0 | User Config: 4.1.0 | Hardware Config: 1.0.0
+
+- Re-enable the I2C watchdog of the left keyboard half which was accidentally disabled starting from firmware 8.4.3. This should fix the freezes of the left keyboard half.
+
 ## [8.5.2] - 2018-10-06
 
 Device Protocol: 4.5.0 | Module Protocol: 4.0.0 | User Config: 4.1.0 | Hardware Config: 1.0.0

ISSUE_TEMPLATE

@@ -1 +1,8 @@
-If you're using Karabiner Elements on your Mac, then stop here! Make sure to close Karabiner Elements, then try to reproduce the issue again, even if you think that Karabiner Elements shouldn't be the cause. Karabiner Elements is the source of numerous problems, and we don't want to receive any more reports it causes.
+Before submitting a new issue, make sure to do the following:
+
+1. If you're using Karabiner Elements on your Mac, close it!
+2. Install the latest Agent:
+   https://github.com/UltimateHackingKeyboard/agent/releases/latest
+3. Use Agent to update to the latest firmware:
+   https://github.com/UltimateHackingKeyboard/firmware/releases/latest
+4. Try to reproduce the issue, and only report it if it still persists.

left/src/key_scanner.c

@@ -5,9 +5,7 @@
 void KEY_SCANNER_HANDLER(void)
 {
     KeyMatrix_ScanRow(&keyMatrix);
-#ifdef I2C_WATCHDOG
     RunWatchdog();
-#endif
     LPTMR_ClearStatusFlags(KEY_SCANNER_LPTMR_BASEADDR, kLPTMR_TimerCompareFlag);
 }
 

right/src/config_parser/parse_config.c

@@ -146,8 +146,6 @@ parser_error_t ParseConfig(config_buffer_t *buffer)
         AlphanumericSegmentsBrightness = alphanumericSegmentsBrightness;
         KeyBacklightBrightness = keyBacklightBrightness;
 
-        LedSlaveDriver_UpdateLeds();
-
         // Update mouse key speeds
 
         MouseMoveState.initialSpeed = mouseMoveInitialSpeed;

right/src/led_display.c

@@ -84,6 +84,7 @@ void LedDisplay_SetText(uint8_t length, const char* text)
         }
         allSegmentSets >>= 5;
     }
+    LedSlaveDriver_UpdateLeds(LedDriverId_Left);
 }
 
 void LedDisplay_SetLayer(layer_id_t layerId)
@@ -95,6 +96,7 @@ void LedDisplay_SetLayer(layer_id_t layerId)
     if (layerId >= LayerId_Mod && layerId <= LayerId_Mouse) {
         LedDriverValues[LedDriverId_Left][16 * layerId - 3] = IconsAndLayerTextsBrightness;
     }
+    LedSlaveDriver_UpdateLeds(LedDriverId_Left);
 }
 
 bool LedDisplay_GetIcon(led_display_icon_t icon)
@@ -106,6 +108,7 @@ void LedDisplay_SetIcon(led_display_icon_t icon, bool isEnabled)
 {
     ledIconStates[icon] = isEnabled;
     LedDriverValues[LedDriverId_Left][icon + 8] = isEnabled ? IconsAndLayerTextsBrightness : 0;
+    LedSlaveDriver_UpdateLeds(LedDriverId_Left);
 }
 
 void LedDisplay_UpdateIcons(void)

right/src/peripherals/led_driver.h

@@ -18,6 +18,11 @@
 
     #define LED_DRIVER_REGISTER_SHUTDOWN            0x0A
     #define LED_DRIVER_REGISTER_FRAME               0xFD
+    #define LED_DRIVER_REGISTER_CONFIGURATION       0x00
+    #define LED_DRIVER_REGISTER_AUTO_PLAY_CONTROL_1 0x02
+    #define LED_DRIVER_REGISTER_AUTO_PLAY_CONTROL_2 0x03
+    #define LED_DRIVER_REGISTER_BREATH_CONTROL_1    0x08
+    #define LED_DRIVER_REGISTER_BREATH_CONTROL_2    0x09
 
     #define LED_DRIVER_FRAME_1 0
     #define LED_DRIVER_FRAME_2 1
@@ -42,6 +47,25 @@
     #define SHUTDOWN_MODE_SHUTDOWN 0
     #define SHUTDOWN_MODE_NORMAL   1
 
+    #define DISPLAY_MODE_AUTO_FRAME_PLAY 0b01
+    #define DISPLAY_MODE_SHIFT 3
+    #define FRAME_START_1 0b000
+    #define FRAME_START_8 0b111
+
+    #define PLAY_LOOP_NUMBER_1 0b001
+    #define PLAY_LOOP_NUMBER_SHIFT 4
+    #define PLAY_FRAME_NUMBER_1 0b001
+
+    #define FRAME_DELAY_TIME 1
+
+    #define FADE_OUT_TIME 5
+    #define FADE_OUT_TIME_SHIFT 4
+    #define FADE_IN_TIME 5
+
+    #define BREATH_ENABLE 1
+    #define BREATH_ENABLE_SHIFT 4
+    #define EXTINGUISH_TIME 0
+
 // Functions:
 
     void InitLedDriver(void);

right/src/slave_drivers/is31fl3731_driver.c

@@ -60,22 +60,48 @@ static led_driver_state_t ledDriverStates[LED_DRIVER_MAX_COUNT] = {
 static uint8_t setFunctionFrameBuffer[] = {LED_DRIVER_REGISTER_FRAME, LED_DRIVER_FRAME_FUNCTION};
 static uint8_t setShutdownModeNormalBuffer[] = {LED_DRIVER_REGISTER_SHUTDOWN, SHUTDOWN_MODE_NORMAL};
 static uint8_t setFrame1Buffer[] = {LED_DRIVER_REGISTER_FRAME, LED_DRIVER_FRAME_1};
+static uint8_t setFrame2Buffer[] = {LED_DRIVER_REGISTER_FRAME, LED_DRIVER_FRAME_2};
+static uint8_t setFrame8Buffer[] = {LED_DRIVER_REGISTER_FRAME, LED_DRIVER_FRAME_8};
+static uint8_t initLedControlRegistersZeroBuffer[19] = { FRAME_REGISTER_LED_CONTROL_FIRST };
+static uint8_t setConfigurationRegisterFadeInBuffer[] = {
+    LED_DRIVER_REGISTER_CONFIGURATION,
+    DISPLAY_MODE_AUTO_FRAME_PLAY << DISPLAY_MODE_SHIFT | FRAME_START_8
+};
+static uint8_t setConfigurationRegisterFadeOutBuffer[] = {
+    LED_DRIVER_REGISTER_CONFIGURATION,
+    DISPLAY_MODE_AUTO_FRAME_PLAY << DISPLAY_MODE_SHIFT | FRAME_START_1
+};
+static uint8_t initAutoPlayControlRegister1Buffer[] = {
+    LED_DRIVER_REGISTER_AUTO_PLAY_CONTROL_1,
+    PLAY_LOOP_NUMBER_1 << PLAY_LOOP_NUMBER_SHIFT | PLAY_FRAME_NUMBER_1
+};
+static uint8_t initAutoPlayControlRegister2Buffer[] = {
+    LED_DRIVER_REGISTER_AUTO_PLAY_CONTROL_2,
+    FRAME_DELAY_TIME
+};
+static uint8_t initBreathControlRegister1Buffer[] = {
+    LED_DRIVER_REGISTER_BREATH_CONTROL_1,
+    FADE_OUT_TIME << FADE_OUT_TIME_SHIFT | FADE_IN_TIME
+};
+static uint8_t initBreathControlRegister2Buffer[] = {
+    LED_DRIVER_REGISTER_BREATH_CONTROL_2,
+    BREATH_ENABLE << BREATH_ENABLE_SHIFT | EXTINGUISH_TIME
+};
 static uint8_t updatePwmRegistersBuffer[PWM_REGISTER_BUFFER_LENGTH];
 
-void LedSlaveDriver_DisableLeds(void)
+void LedSlaveDriver_DisableLeds(uint8_t ledDriverId)
 {
-    for (uint8_t ledDriverId=0; ledDriverId<=LedDriverId_Last; ledDriverId++) {
+    ledDriverStates[ledDriverId].requests[LedDriverRequest_DisableLeds] = true;
-        memset(LedDriverValues[ledDriverId], 0, LED_DRIVER_LED_COUNT);
-    }
 }
 
-void LedSlaveDriver_UpdateLeds(void)
+void LedSlaveDriver_EnableLeds(uint8_t ledDriverId)
 {
-    for (uint8_t ledDriverId=0; ledDriverId<=LedDriverId_Last; ledDriverId++) {
+    ledDriverStates[ledDriverId].requests[LedDriverRequest_EnableLeds] = true;
-        memset(LedDriverValues[ledDriverId], KeyBacklightBrightness, LED_DRIVER_LED_COUNT);
 }
 
-    LedDisplay_UpdateAll();
+void LedSlaveDriver_UpdateLeds(uint8_t ledDriverId)
+{
+    ledDriverStates[ledDriverId].requests[LedDriverRequest_UpdateLeds] = true;
 }
 
 void LedSlaveDriver_Init(uint8_t ledDriverId)
@@ -85,13 +111,9 @@ void LedSlaveDriver_Init(uint8_t ledDriverId)
     }
 
     led_driver_state_t *currentLedDriverState = ledDriverStates + ledDriverId;
-    currentLedDriverState->phase = LedDriverPhase_SetFunctionFrame;
     currentLedDriverState->ledIndex = 0;
     memset(LedDriverValues[ledDriverId], KeyBacklightBrightness, LED_DRIVER_LED_COUNT);
-
+    ledDriverStates[ledDriverId].requests[LedDriverRequest_Init] = true;
-    if (ledDriverId == LedDriverId_Left) {
-        LedDisplay_UpdateAll();
-    }
 }
 
 status_t LedSlaveDriver_Update(uint8_t ledDriverId)
@@ -99,31 +121,62 @@ status_t LedSlaveDriver_Update(uint8_t ledDriverId)
     status_t status = kStatus_Uhk_IdleSlave;
     uint8_t *ledValues = LedDriverValues[ledDriverId];
     led_driver_state_t *currentLedDriverState = ledDriverStates + ledDriverId;
-    uint8_t *ledDriverPhase = &currentLedDriverState->phase;
     uint8_t ledDriverAddress = currentLedDriverState->i2cAddress;
     uint8_t *ledIndex = &currentLedDriverState->ledIndex;
 
-    switch (*ledDriverPhase) {
+    switch (currentLedDriverState->phase) {
-        case LedDriverPhase_SetFunctionFrame:
+        case LedDriverPhase_SetFunctionFrameInit:
             if (ledDriverId == LedDriverId_Left && !Slaves[SlaveId_LeftKeyboardHalf].isConnected) {
                 break;
             }
             status = I2cAsyncWrite(ledDriverAddress, setFunctionFrameBuffer, sizeof(setFunctionFrameBuffer));
-            *ledDriverPhase = LedDriverPhase_SetShutdownModeNormal;
+            currentLedDriverState->phase = LedDriverPhase_SetShutdownModeNormalInit;
             break;
-        case LedDriverPhase_SetShutdownModeNormal:
+        case LedDriverPhase_SetShutdownModeNormalInit:
             status = I2cAsyncWrite(ledDriverAddress, setShutdownModeNormalBuffer, sizeof(setShutdownModeNormalBuffer));
-            *ledDriverPhase = LedDriverPhase_SetFrame1;
+            currentLedDriverState->phase = LedDriverPhase_InitAutoPlayControlRegister1Init;
             break;
-        case LedDriverPhase_SetFrame1:
+        case LedDriverPhase_InitAutoPlayControlRegister1Init:
+            status = I2cAsyncWrite(ledDriverAddress, initAutoPlayControlRegister1Buffer, sizeof(initAutoPlayControlRegister1Buffer));
+            currentLedDriverState->phase = LedDriverPhase_InitAutoPlayControlRegister2Init;
+            break;
+        case LedDriverPhase_InitAutoPlayControlRegister2Init:
+            status = I2cAsyncWrite(ledDriverAddress, initAutoPlayControlRegister2Buffer, sizeof(initAutoPlayControlRegister2Buffer));
+            currentLedDriverState->phase = LedDriverPhase_InitBreathControlRegister1Init;
+            break;
+        case LedDriverPhase_InitBreathControlRegister1Init:
+            status = I2cAsyncWrite(ledDriverAddress, initBreathControlRegister1Buffer, sizeof(initBreathControlRegister1Buffer));
+            currentLedDriverState->phase = LedDriverPhase_InitBreathControlRegister2Init;
+            break;
+        case LedDriverPhase_InitBreathControlRegister2Init:
+            status = I2cAsyncWrite(ledDriverAddress, initBreathControlRegister2Buffer, sizeof(initBreathControlRegister2Buffer));
+            currentLedDriverState->phase = LedDriverPhase_SetFrame2Init;
+            break;
+        case LedDriverPhase_SetFrame2Init:
+            status = I2cAsyncWrite(ledDriverAddress, setFrame2Buffer, sizeof(setFrame2Buffer));
+            currentLedDriverState->phase = LedDriverPhase_InitLedControlRegistersZero1Init;
+            break;
+        case LedDriverPhase_InitLedControlRegistersZero1Init:
+            status = I2cAsyncWrite(ledDriverAddress, initLedControlRegistersZeroBuffer, sizeof(initLedControlRegistersZeroBuffer));
+            currentLedDriverState->phase = LedDriverPhase_SetFrame8Init;
+            break;
+        case LedDriverPhase_SetFrame8Init:
+            status = I2cAsyncWrite(ledDriverAddress, setFrame8Buffer, sizeof(setFrame8Buffer));
+            currentLedDriverState->phase = LedDriverPhase_InitLedControlRegistersZero2Init;
+            break;
+        case LedDriverPhase_InitLedControlRegistersZero2Init:
+            status = I2cAsyncWrite(ledDriverAddress, initLedControlRegistersZeroBuffer, sizeof(initLedControlRegistersZeroBuffer));
+            currentLedDriverState->phase = LedDriverPhase_SetFrame1Init;
+            break;
+        case LedDriverPhase_SetFrame1Init:
             status = I2cAsyncWrite(ledDriverAddress, setFrame1Buffer, sizeof(setFrame1Buffer));
-            *ledDriverPhase = LedDriverPhase_InitLedControlRegisters;
+            currentLedDriverState->phase = LedDriverPhase_InitLedControlRegistersInit;
             break;
-        case LedDriverPhase_InitLedControlRegisters:
+        case LedDriverPhase_InitLedControlRegistersInit:
             status = I2cAsyncWrite(ledDriverAddress, currentLedDriverState->setupLedControlRegistersCommand, LED_CONTROL_REGISTERS_COMMAND_LENGTH);
-            *ledDriverPhase = LedDriverPhase_InitLedValues;
+            currentLedDriverState->phase = LedDriverPhase_InitLedValuesInit;
             break;
-        case LedDriverPhase_InitLedValues:
+        case LedDriverPhase_InitLedValuesInit:
             updatePwmRegistersBuffer[0] = FRAME_REGISTER_PWM_FIRST + *ledIndex;
             uint8_t chunkSize = MIN(LED_DRIVER_LED_COUNT - *ledIndex, PMW_REGISTER_UPDATE_CHUNK_SIZE);
             memcpy(updatePwmRegistersBuffer+1, ledValues + *ledIndex, chunkSize);
@@ -132,10 +185,35 @@ status_t LedSlaveDriver_Update(uint8_t ledDriverId)
             if (*ledIndex >= LED_DRIVER_LED_COUNT) {
                 *ledIndex = 0;
                 memcpy(currentLedDriverState->targetLedValues, ledValues, LED_DRIVER_LED_COUNT);
-                *ledDriverPhase = LedDriverPhase_UpdateChangedLedValues;
+                currentLedDriverState->phase = LedDriverPhase_Idle;
             }
             break;
-        case LedDriverPhase_UpdateChangedLedValues: {
+        case LedDriverPhase_Idle: {
+            uint8_t i;
+
+            for (i = 0; i <= LedDriverRequest_Last; i++) {
+                if (currentLedDriverState->requests[i]) {
+                    currentLedDriverState->requests[i] = false;
+                    break;
+                }
+            }
+            switch (i) {
+                case LedDriverRequest_Init:
+                    currentLedDriverState->phase = LedDriverPhase_SetFunctionFrameInit;
+                    break;
+                case LedDriverRequest_EnableLeds:
+                    currentLedDriverState->phase = LedDriverPhase_SetFunctionFrameEnableLeds;
+                    break;
+                case LedDriverRequest_DisableLeds:
+                    currentLedDriverState->phase = LedDriverPhase_SetFunctionFrameDisableLeds;
+                    break;
+                case LedDriverRequest_UpdateLeds:
+                    currentLedDriverState->phase = LedDriverPhase_UpdateChangedLedValuesUpdateLeds;
+                    break;
+            }
+            break;
+        }
+        case LedDriverPhase_UpdateChangedLedValuesUpdateLeds: {
             uint8_t *targetLedValues = currentLedDriverState->targetLedValues;
 
             uint8_t lastLedChunkStartIndex = LED_DRIVER_LED_COUNT - PMW_REGISTER_UPDATE_CHUNK_SIZE;
@@ -155,6 +233,7 @@ status_t LedSlaveDriver_Update(uint8_t ledDriverId)
             bool foundStartIndex = count < LED_DRIVER_LED_COUNT;
             if (!foundStartIndex) {
                 *ledIndex = 0;
+                currentLedDriverState->phase = LedDriverPhase_Idle;
                 break;
             }
 
@@ -175,9 +254,34 @@ status_t LedSlaveDriver_Update(uint8_t ledDriverId)
             *ledIndex += length;
             if (*ledIndex >= LED_DRIVER_LED_COUNT) {
                 *ledIndex = 0;
+                currentLedDriverState->phase = LedDriverPhase_Idle;
             }
             break;
         }
+        case LedDriverPhase_SetFunctionFrameDisableLeds:
+            status = I2cAsyncWrite(ledDriverAddress, setFunctionFrameBuffer, sizeof(setFunctionFrameBuffer));
+            currentLedDriverState->phase = LedDriverPhase_SetConfigurationRegisterDisableLeds;
+            break;
+        case LedDriverPhase_SetConfigurationRegisterDisableLeds:
+            status = I2cAsyncWrite(ledDriverAddress, setConfigurationRegisterFadeOutBuffer, sizeof(setConfigurationRegisterFadeOutBuffer));
+            currentLedDriverState->phase = LedDriverPhase_SetFrame1DisableLeds;
+            break;
+        case LedDriverPhase_SetFrame1DisableLeds:
+            status = I2cAsyncWrite(ledDriverAddress, setFrame1Buffer, sizeof(setFrame1Buffer));
+            currentLedDriverState->phase = LedDriverPhase_Idle;
+            break;
+        case LedDriverPhase_SetFunctionFrameEnableLeds:
+            status = I2cAsyncWrite(ledDriverAddress, setFunctionFrameBuffer, sizeof(setFunctionFrameBuffer));
+            currentLedDriverState->phase = LedDriverPhase_SetConfigurationRegisterEnableLeds;
+            break;
+        case LedDriverPhase_SetConfigurationRegisterEnableLeds:
+            status = I2cAsyncWrite(ledDriverAddress, setConfigurationRegisterFadeInBuffer, sizeof(setConfigurationRegisterFadeInBuffer));
+            currentLedDriverState->phase = LedDriverPhase_SetFrame1EnableLeds;
+            break;
+        case LedDriverPhase_SetFrame1EnableLeds:
+            status = I2cAsyncWrite(ledDriverAddress, setFrame1Buffer, sizeof(setFrame1Buffer));
+            currentLedDriverState->phase = LedDriverPhase_Idle;
+            break;
     }
 
     return status;

right/src/slave_drivers/is31fl3731_driver.h

@@ -20,6 +20,14 @@
 
 // Typedefs:
 
+    enum {
+        LedDriverRequest_Init,
+        LedDriverRequest_EnableLeds,
+        LedDriverRequest_DisableLeds,
+        LedDriverRequest_UpdateLeds,
+        LedDriverRequest_Last = LedDriverRequest_UpdateLeds
+    };
+
     typedef enum {
         LedDriverId_Right,
         LedDriverId_Left,
@@ -27,15 +35,31 @@
     } led_driver_id_t;
 
     typedef enum {
-        LedDriverPhase_SetFunctionFrame,
+        LedDriverPhase_Idle,
-        LedDriverPhase_SetShutdownModeNormal,
+        LedDriverPhase_SetFunctionFrameInit,
-        LedDriverPhase_SetFrame1,
+        LedDriverPhase_SetFunctionFrameDisableLeds,
-        LedDriverPhase_InitLedControlRegisters,
+        LedDriverPhase_SetFunctionFrameEnableLeds,
-        LedDriverPhase_InitLedValues,
+        LedDriverPhase_SetShutdownModeNormalInit,
-        LedDriverPhase_UpdateChangedLedValues,
+        LedDriverPhase_SetFrame1Init,
+        LedDriverPhase_SetFrame1EnableLeds,
+        LedDriverPhase_SetFrame1DisableLeds,
+        LedDriverPhase_SetFrame2Init,
+        LedDriverPhase_SetFrame8Init,
+        LedDriverPhase_InitAutoPlayControlRegister1Init,
+        LedDriverPhase_InitAutoPlayControlRegister2Init,
+        LedDriverPhase_InitBreathControlRegister1Init,
+        LedDriverPhase_InitBreathControlRegister2Init,
+        LedDriverPhase_InitLedControlRegistersZero1Init,
+        LedDriverPhase_InitLedControlRegistersZero2Init,
+        LedDriverPhase_InitLedValuesInit,
+        LedDriverPhase_UpdateChangedLedValuesUpdateLeds,
+        LedDriverPhase_SetConfigurationRegisterDisableLeds,
+        LedDriverPhase_SetConfigurationRegisterEnableLeds,
+        LedDriverPhase_InitLedControlRegistersInit
     } led_driver_phase_t;
 
     typedef struct {
+        bool requests[LedDriverRequest_Last + 1];
         led_driver_phase_t phase;
         uint8_t targetLedValues[LED_DRIVER_LED_COUNT];
         uint8_t ledIndex;
@@ -50,8 +74,9 @@
 
 // Functions:
 
-    void LedSlaveDriver_DisableLeds(void);
+    void LedSlaveDriver_DisableLeds(uint8_t ledDriverId);
-    void LedSlaveDriver_UpdateLeds(void);
+    void LedSlaveDriver_EnableLeds(uint8_t ledDriverId);
+    void LedSlaveDriver_UpdateLeds(uint8_t ledDriverId);
     void LedSlaveDriver_Init(uint8_t ledDriverId);
     status_t LedSlaveDriver_Update(uint8_t ledDriverId);
 

right/src/usb_composite_device.c

@@ -168,12 +168,16 @@ static volatile bool wakeUpHostAllowed;
 
 static void suspendUhk(void) {
     SleepModeActive = true;
-    LedSlaveDriver_DisableLeds();
+    for (uint8_t i = 0; i <= LedDriverId_Last; i++) {
+        LedSlaveDriver_DisableLeds(i);
+    }
 }
 
 static void wakeUpUhk(void) {
     SleepModeActive = false;
-    LedSlaveDriver_UpdateLeds();
+    for (uint8_t i = 0; i <= LedDriverId_Last; i++) {
+        LedSlaveDriver_EnableLeds(i);
+    }
 }
 
 void WakeUpHost(void) {

scripts/package.json

@@ -15,7 +15,7 @@
     "commander": "^2.11.0",
     "shelljs": "^0.7.8"
   },
-  "firmwareVersion": "8.5.2",
+  "firmwareVersion": "8.5.3",
   "deviceProtocolVersion": "4.5.0",
   "moduleProtocolVersion": "4.0.0",
   "userConfigVersion": "4.1.0",

shared/versions.h

@@ -20,7 +20,7 @@
 
     #define FIRMWARE_MAJOR_VERSION 8
     #define FIRMWARE_MINOR_VERSION 5
-    #define FIRMWARE_PATCH_VERSION 2
+    #define FIRMWARE_PATCH_VERSION 3
 
     #define DEVICE_PROTOCOL_MAJOR_VERSION 4
     #define DEVICE_PROTOCOL_MINOR_VERSION 5