Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rgblight idle timeout feature #6450

Closed
wants to merge 3 commits into from
Closed

rgblight idle timeout feature #6450

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2cd1e69
rgblight idle timeout feature
nvitaterna Jul 31, 2019
5a2aa15
Update feature_rgblight.md
nvitaterna Jul 31, 2019
2595132
changes for comments
nvitaterna Jul 31, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/feature_rgblight.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ Your RGB lighting can be configured by placing these `#define`s in your `config.
|`RGBLIGHT_LIMIT_VAL` |`255` |The maximum brightness level |
|`RGBLIGHT_SLEEP` |*Not defined*|If defined, the RGB lighting will be switched off when the host goes to sleep|
|`RGBLIGHT_SPLIT` |*Not defined*|If defined, synchronization functionality for split keyboards is added|
|`RGBLIGHT_IDLE_TIMEOUT`|*Not defined*|If defined, turn the backlight off after this many minutes of inactivity.|

## Effects and Animations

Expand Down
3 changes: 3 additions & 0 deletions quantum/quantum.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,9 @@ bool process_record_quantum(keyrecord_t *record) {
#endif
#ifdef SPACE_CADET_ENABLE
process_space_cadet(keycode, record) &&
#endif
#ifdef RGBLIGHT_USE_PROCESS
process_rgblight(keycode, record) &&
#endif
true)) {
return false;
Expand Down
34 changes: 33 additions & 1 deletion quantum/rgblight.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,16 @@ bool is_rgblight_initialized = false;
animation_status_t animation_status = {};
#endif

#ifdef RGBLIGHT_IDLE_ENABLE
bool rgblight_idle_timedout = false;
uint32_t rgblight_idle_timer;
#endif

#ifndef LED_ARRAY
LED_TYPE led[RGBLED_NUM];
#define LED_ARRAY led
#endif


static uint8_t clipping_start_pos = 0;
static uint8_t clipping_num_leds = RGBLED_NUM;
static uint8_t effect_start_pos = 0;
Expand Down Expand Up @@ -220,6 +224,10 @@ void rgblight_init(void) {
rgblight_mode_noeeprom(rgblight_config.mode);
}

#ifdef RGBLIGHT_IDLE_ENABLE
rgblight_idle_timer = timer_read32();
#endif

is_rgblight_initialized = true;

}
Expand Down Expand Up @@ -797,6 +805,15 @@ void rgblight_task(void) {
// static light mode, do nothing here
if ( 1 == 0 ) { //dummy
}
#ifdef RGBLIGHT_IDLE_ENABLE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no need for multiple defines for this feature as you can just used #if RGBLIGHT_IDLE_TIMEOUT > 0

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. Should I just use the rgb matrix variable or still use a separate one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep the RGB Light define for this feature separate as there has been interest in running both RGB Light & Matrix at the same time.

// exit early if we are timedout
else if (rgblight_idle_timedout) {
return;
} else if (timer_elapsed32(rgblight_idle_timer) >= (RGBLIGHT_IDLE_TIMEOUT * RGBLIGHT_IDLE_MULTIPLIER)) {
rgblight_idle_timedout = true;
rgblight_disable_noeeprom();
}
#endif
#ifdef RGBLIGHT_EFFECT_BREATHING
else if (rgblight_status.base_mode == RGBLIGHT_MODE_BREATHING) {
// breathing mode
Expand Down Expand Up @@ -885,6 +902,21 @@ void rgblight_task(void) {

#endif /* RGBLIGHT_USE_TIMER */

#ifdef RGBLIGHT_USE_PROCESS
bool process_rgblight(uint16_t keycode, keyrecord_t *record) {
#ifdef RGBLIGHT_IDLE_ENABLE
if (record->event.pressed) {
if (rgblight_idle_timedout) {
rgblight_idle_timedout = false;
rgblight_enable_noeeprom();
Copy link
Contributor

@XScorpion2 XScorpion2 Aug 3, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't change the enable/disable state, as a user could have previously disabled the rgblight, and this would turn it back on against the users wishes. Use a similar pattern to what rgb matrix uses to turn off the LEDs without changing user state.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. I will look at the rgb matrix code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the rgb_matrix code uses a g_suspend_state boolean locally to determine if it's suspended or not, and renders based on that.

}
rgblight_idle_timer = timer_read32();
}
#endif
return true;
}
#endif

// Effects
#ifdef RGBLIGHT_EFFECT_BREATHING

Expand Down
9 changes: 9 additions & 0 deletions quantum/rgblight.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,15 @@ void rgblight_timer_enable(void);
void rgblight_timer_disable(void);
void rgblight_timer_toggle(void);

// if timeout is set, enable the rgblight idle feature
#ifdef RGBLIGHT_IDLE_TIMEOUT
#define RGBLIGHT_IDLE_ENABLE
#define RGBLIGHT_IDLE_MULTIPLIER 60000
Copy link
Contributor

@yanfali yanfali Jul 31, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe something like RGB_IDLE_MULTIPLIER_MILLIS

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about RGB_DISABLE_AFTER_TIMEOUT, as we're using that with RGB Matrix:

https://docs.qmk.fm/#/feature_rgb_matrix?id=additional-configh-options

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with that, is just like to know what the unit of measurement is

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ... not exactly sure, but the code is called:

qmk_firmware/quantum/rgb_matrix.c

Line 373 in cd7bc03

bool suspend_backlight = ((g_suspend_state && RGB_DISABLE_WHEN_USB_SUSPENDED) || (RGB_DISABLE_AFTER_TIMEOUT > 0 && g_rgb_counters.any_key_hit > RGB_DISABLE_AFTER_TIMEOUT * 60 * 20));

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@XScorpion2 what is the unit in, for RGB_DISABLE_AFTER_TIMEOUT (my brain is failing me)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically it's 1.2s units. No idea why it was coded like that. Never bothered changing it so it made sense.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be changed to seconds or minutes. Documentation says "ticks" which doesn't really mean anything

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just submitted a pr for RGB Matrix to clean up it's idle timeout and change it to seconds: #6480

#endif

#include "quantum.h"
bool process_rgblight(uint16_t keycode, keyrecord_t *record);

#ifdef RGBLIGHT_SPLIT
#define RGBLIGHT_STATUS_CHANGE_MODE (1<<0)
#define RGBLIGHT_STATUS_CHANGE_HSVS (1<<1)
Expand Down
7 changes: 6 additions & 1 deletion quantum/rgblight_reconfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
#define RGBLIGHT_EFFECT_STATIC_GRADIENT
#endif

#if defined(RGBLIGHT_IDLE_TIMEOUT)
#define RGBLIGHT_USE_PROCESS
#endif

// check dynamic animation effects chose ?
#if defined(RGBLIGHT_EFFECT_BREATHING) || \
defined(RGBLIGHT_EFFECT_RAINBOW_MOOD) || \
Expand All @@ -26,7 +30,8 @@
defined(RGBLIGHT_EFFECT_KNIGHT) || \
defined(RGBLIGHT_EFFECT_CHRISTMAS) || \
defined(RGBLIGHT_EFFECT_RGB_TEST) || \
defined(RGBLIGHT_EFFECT_ALTERNATING)
defined(RGBLIGHT_EFFECT_ALTERNATING) || \
defined(RGBLIGHT_IDLE_ENABLE)
#define RGBLIGHT_USE_TIMER
#ifndef RGBLIGHT_ANIMATIONS
#define RGBLIGHT_ANIMATIONS // for backward compatibility
Expand Down