Skip to content
Permalink
Browse files
RGB Matrix refactoring to open up for new drivers (#3913)
* rgb_matrix: use a driver ops struct

This is intended to avoid #ifdef proliferation on adding more drivers,
eg. model01, which use different architectures.

* rgb_matrix: document driver struct members

* rgb_matrix: remove unused LED testing code

* rgb_matrix: don't build into IS31x drivers unless being used

* rgb_matrix: refactor make config options

This ensures that the necessary files are included for any custom
RGB_MATRIX_ENABLE value, without having to add entries here for specific
boards. This particularly affects model01 because its controller is
integrated and won't be used anywhere else, so it's preferable not to
put it in common_features.mk.

This now validates the value of RGB_MATRIX_ENABLE.

It was necessary to fix an error in ergodox_ez rules.mk using the wrong
comment separator, yielding an invalid value.

* IS31x drivers: don't write the control registers all the time

This is only needed when they are changed. This is done in init() and
board- or keymap-specific code is free to make further changes.

* rgb_matrix: move structs from chip drivers to rgb_matrix_drivers.c

This approach is specific to the rgb_matrix functionality, so keep it
neatly separated from the raw chip drivers.
  • Loading branch information
abrasive authored and jackhumbert committed Sep 27, 2018
1 parent 12ad59f commit f70f45ee677a2a39a759052a356e0c5d82e25424
Showing with 113 additions and 129 deletions.
  1. +11 −13 common_features.mk
  2. +0 −1 drivers/issi/is31fl3731.c
  3. +1 −2 drivers/issi/is31fl3733.c
  4. +1 −1 keyboards/ergodox_ez/rules.mk
  5. +4 −109 quantum/rgb_matrix.c
  6. +14 −3 quantum/rgb_matrix.h
  7. +82 −0 quantum/rgb_matrix_drivers.c
@@ -114,37 +114,35 @@ ifeq ($(strip $(RGBLIGHT_ENABLE)), yes)
endif
endif

ifeq ($(strip $(RGB_MATRIX_ENABLE)), yes)
RGB_MATRIX_ENABLE ?= no
VALID_MATRIX_TYPES := yes IS31FL3731L IS31FL3733L custom
ifneq ($(strip $(RGB_MATRIX_ENABLE)), no)
ifeq ($(filter $(RGB_MATRIX_ENABLE),$(VALID_MATRIX_TYPES)),)
$(error RGB_MATRIX_ENABLE="$(RGB_MATRIX_ENABLE)" is not a valid matrix type)
endif
OPT_DEFS += -DRGB_MATRIX_ENABLE
OPT_DEFS += -DIS31FL3731
COMMON_VPATH += $(DRIVER_PATH)/issi
SRC += is31fl3731.c
SRC += i2c_master.c
SRC += $(QUANTUM_DIR)/color.c
SRC += $(QUANTUM_DIR)/rgb_matrix.c
SRC += $(QUANTUM_DIR)/rgb_matrix_drivers.c
CIE1931_CURVE = yes
endif

ifeq ($(strip $(RGB_MATRIX_ENABLE)), yes)
RGB_MATRIX_ENABLE = IS31FL3731
endif

ifeq ($(strip $(RGB_MATRIX_ENABLE)), IS31FL3731)
OPT_DEFS += -DRGB_MATRIX_ENABLE
OPT_DEFS += -DIS31FL3731
COMMON_VPATH += $(DRIVER_PATH)/issi
SRC += is31fl3731.c
SRC += i2c_master.c
SRC += $(QUANTUM_DIR)/color.c
SRC += $(QUANTUM_DIR)/rgb_matrix.c
CIE1931_CURVE = yes
endif

ifeq ($(strip $(RGB_MATRIX_ENABLE)), IS31FL3733)
OPT_DEFS += -DRGB_MATRIX_ENABLE
OPT_DEFS += -DIS31FL3733
COMMON_VPATH += $(DRIVER_PATH)/issi
SRC += is31fl3733.c
SRC += i2c_master.c
SRC += $(QUANTUM_DIR)/color.c
SRC += $(QUANTUM_DIR)/rgb_matrix.c
CIE1931_CURVE = yes
endif

ifeq ($(strip $(TAP_DANCE_ENABLE)), yes)
@@ -268,4 +268,3 @@ void IS31FL3731_update_led_control_registers( uint8_t addr1, uint8_t addr2 )
}
}
}

@@ -24,10 +24,10 @@
#include "wait.h"
#endif

#include "is31fl3733.h"
#include <string.h>
#include "i2c_master.h"
#include "progmem.h"
#include "rgb_matrix.h"

// This is a 7-bit address, that gets left-shifted and bit 0
// set to 0 for write, 1 for read (as per I2C protocol)
@@ -250,4 +250,3 @@ void IS31FL3733_update_led_control_registers( uint8_t addr1, uint8_t addr2 )
}
}
}

@@ -83,6 +83,6 @@ SWAP_HANDS_ENABLE= yes # Allow swapping hands of keyboard
SLEEP_LED_ENABLE = no
API_SYSEX_ENABLE = no
RGBLIGHT_ENABLE = yes
RGB_MATRIX_ENABLE = no // enable later
RGB_MATRIX_ENABLE = no # enable later

LAYOUTS = ergodox
@@ -18,7 +18,6 @@


#include "rgb_matrix.h"
#include "i2c_master.h"
#include "progmem.h"
#include "config.h"
#include "eeprom.h"
@@ -111,29 +110,15 @@ void map_row_column_to_led( uint8_t row, uint8_t column, uint8_t *led_i, uint8_t
}

void rgb_matrix_update_pwm_buffers(void) {
#ifdef IS31FL3731
IS31FL3731_update_pwm_buffers( DRIVER_ADDR_1, DRIVER_ADDR_2 );
IS31FL3731_update_led_control_registers( DRIVER_ADDR_1, DRIVER_ADDR_2 );
#elif defined(IS31FL3733)
IS31FL3733_update_pwm_buffers( DRIVER_ADDR_1, DRIVER_ADDR_2 );
IS31FL3733_update_led_control_registers( DRIVER_ADDR_1, DRIVER_ADDR_2 );
#endif
rgb_matrix_driver.flush();
}

void rgb_matrix_set_color( int index, uint8_t red, uint8_t green, uint8_t blue ) {
#ifdef IS31FL3731
IS31FL3731_set_color( index, red, green, blue );
#elif defined(IS31FL3733)
IS31FL3733_set_color( index, red, green, blue );
#endif
rgb_matrix_driver.set_color(index, red, green, blue);
}

void rgb_matrix_set_color_all( uint8_t red, uint8_t green, uint8_t blue ) {
#ifdef IS31FL3731
IS31FL3731_set_color_all( red, green, blue );
#elif defined(IS31FL3733)
IS31FL3733_set_color_all( red, green, blue );
#endif
rgb_matrix_driver.set_color_all(red, green, blue);
}

bool process_rgb_matrix(uint16_t keycode, keyrecord_t *record) {
@@ -196,47 +181,6 @@ void rgb_matrix_test(void) {
}
}

// This tests the LEDs
// Note that it will change the LED control registers
// in the LED drivers, and leave them in an invalid
// state for other backlight effects.
// ONLY USE THIS FOR TESTING LEDS!
void rgb_matrix_single_LED_test(void) {
static uint8_t color = 0; // 0,1,2 for R,G,B
static uint8_t row = 0;
static uint8_t column = 0;

static uint8_t tick = 0;
tick++;

if ( tick > 2 )
{
tick = 0;
column++;
}
if ( column > MATRIX_COLS )
{
column = 0;
row++;
}
if ( row > MATRIX_ROWS )
{
row = 0;
color++;
}
if ( color > 2 )
{
color = 0;
}

uint8_t led[8], led_count;
map_row_column_to_led(row,column,led,&led_count);
for(uint8_t i = 0; i < led_count; i++) {
rgb_matrix_set_color_all( 40, 40, 40 );
rgb_matrix_test_led( led[i], color==0, color==1, color==2 );
}
}

// All LEDs off
void rgb_matrix_all_off(void) {
rgb_matrix_set_color_all( 0, 0, 0 );
@@ -817,7 +761,7 @@ void rgb_matrix_indicators_user(void) {}
// }

void rgb_matrix_init(void) {
rgb_matrix_setup_drivers();
rgb_matrix_driver.init();

// TODO: put the 1 second startup delay here?

@@ -841,33 +785,6 @@ void rgb_matrix_init(void) {
eeconfig_debug_rgb_matrix(); // display current eeprom values
}

void rgb_matrix_setup_drivers(void) {
// Initialize TWI
i2c_init();
#ifdef IS31FL3731
IS31FL3731_init( DRIVER_ADDR_1 );
IS31FL3731_init( DRIVER_ADDR_2 );
#elif defined (IS31FL3733)
IS31FL3733_init( DRIVER_ADDR_1 );
#endif

for ( int index = 0; index < DRIVER_LED_TOTAL; index++ ) {
bool enabled = true;
// This only caches it for later
#ifdef IS31FL3731
IS31FL3731_set_led_control_register( index, enabled, enabled, enabled );
#elif defined (IS31FL3733)
IS31FL3733_set_led_control_register( index, enabled, enabled, enabled );
#endif
}
// This actually updates the LED drivers
#ifdef IS31FL3731
IS31FL3731_update_led_control_registers( DRIVER_ADDR_1, DRIVER_ADDR_2 );
#elif defined (IS31FL3733)
IS31FL3733_update_led_control_registers( DRIVER_ADDR_1, DRIVER_ADDR_2 );
#endif
}

// Deals with the messy details of incrementing an integer
uint8_t increment( uint8_t value, uint8_t step, uint8_t min, uint8_t max ) {
int16_t new_value = value;
@@ -910,28 +827,6 @@ uint8_t decrement( uint8_t value, uint8_t step, uint8_t min, uint8_t max ) {
// }
// }

void rgb_matrix_test_led( uint8_t index, bool red, bool green, bool blue ) {
for ( int i=0; i<DRIVER_LED_TOTAL; i++ )
{
if ( i == index )
{
#ifdef IS31FL3731
IS31FL3731_set_led_control_register( i, red, green, blue );
#elif defined (IS31FL3733)
IS31FL3733_set_led_control_register( i, red, green, blue );
#endif
}
else
{
#ifdef IS31FL3731
IS31FL3731_set_led_control_register( i, false, false, false );
#elif defined (IS31FL3733)
IS31FL3733_set_led_control_register( i, false, false, false );
#endif
}
}
}

uint32_t rgb_matrix_get_tick(void) {
return g_tick;
}
@@ -100,8 +100,6 @@ void rgb_matrix_indicators(void);
void rgb_matrix_indicators_kb(void);
void rgb_matrix_indicators_user(void);

void rgb_matrix_single_LED_test(void);

void rgb_matrix_init(void);
void rgb_matrix_setup_drivers(void);

@@ -126,7 +124,6 @@ void rgb_matrix_decrease(void);
// void backlight_get_key_color( uint8_t led, HSV *hsv );
// void backlight_set_key_color( uint8_t row, uint8_t column, HSV hsv );

void rgb_matrix_test_led( uint8_t index, bool red, bool green, bool blue );
uint32_t rgb_matrix_get_tick(void);

void rgblight_toggle(void);
@@ -143,4 +140,18 @@ void rgblight_decrease_speed(void);
void rgblight_mode(uint8_t mode);
uint32_t rgblight_get_mode(void);

typedef struct {
/* Perform any initialisation required for the other driver functions to work. */
void (*init)(void);

/* Set the colour of a single LED in the buffer. */
void (*set_color)(int index, uint8_t r, uint8_t g, uint8_t b);
/* Set the colour of all LEDS on the keyboard in the buffer. */
void (*set_color_all)(uint8_t r, uint8_t g, uint8_t b);
/* Flush any buffered changes to the hardware. */
void (*flush)(void);
} rgb_matrix_driver_t;

extern const rgb_matrix_driver_t rgb_matrix_driver;

#endif
@@ -0,0 +1,82 @@
/* Copyright 2018 James Laird-Wah
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include "rgb_matrix.h"

/* Each driver needs to define the struct
* const rgb_matrix_driver_t rgb_matrix_driver;
* All members must be provided.
* Keyboard custom drivers can define this in their own files, it should only
* be here if shared between boards.
*/

#if defined(IS31FL3731) || defined(IS31FL3733)

#include "i2c_master.h"

static void init( void )
{
i2c_init();
#ifdef IS31FL3731
IS31FL3731_init( DRIVER_ADDR_1 );
IS31FL3731_init( DRIVER_ADDR_2 );
#else
IS31FL3733_init( DRIVER_ADDR_1 );
#endif
for ( int index = 0; index < DRIVER_LED_TOTAL; index++ ) {
bool enabled = true;
// This only caches it for later
#ifdef IS31FL3731
IS31FL3731_set_led_control_register( index, enabled, enabled, enabled );
#else
IS31FL3733_set_led_control_register( index, enabled, enabled, enabled );
#endif
}
// This actually updates the LED drivers
#ifdef IS31FL3731
IS31FL3731_update_led_control_registers( DRIVER_ADDR_1, DRIVER_ADDR_2 );
#else
IS31FL3733_update_led_control_registers( DRIVER_ADDR_1, DRIVER_ADDR_2 );
#endif
}

#ifdef IS31FL3731
static void flush( void )
{
IS31FL3731_update_pwm_buffers( DRIVER_ADDR_1, DRIVER_ADDR_2 );
}

const rgb_matrix_driver_t rgb_matrix_driver = {
.init = init,
.flush = flush,
.set_color = IS31FL3731_set_color,
.set_color_all = IS31FL3731_set_color_all,
};
#else
static void flush( void )
{
IS31FL3733_update_pwm_buffers( DRIVER_ADDR_1, DRIVER_ADDR_2 );
}

const rgb_matrix_driver_t rgb_matrix_driver = {
.init = init,
.flush = flush,
.set_color = IS31FL3733_set_color,
.set_color_all = IS31FL3733_set_color_all,
};
#endif

#endif

0 comments on commit f70f45e

Please sign in to comment.