Expose the time of the last change to the LED state

[joukewitteveen]

This PR is in part a question: would the proposed change make sense? It extends #11552 and #11595.

Description

This introduces two new functions that mimic

uint32_t last_input_activity_time(void);    // Timestamp of the last matrix or encoder activity
uint32_t last_input_activity_elapsed(void); // Number of milliseconds since the last matrix or encoder activity

from keyboard.h.

Keyboard activity can come from inside (key and/or encoder activity), or from outside. Outside activity happens in the form of USB 'set' reports that can change the LED state.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

• Some keyboards use a screen to show the LED state. If such keyboards have a need to implement custom screen timeout logic (e.g. when they show an animation that breaks the normal timeout logic), it would be real easy if they could just do:

    if (timer_elapsed32(MAX(last_input_activity_time(), last_led_activity_time())) > CUSTOM_TIMEOUT) screen_off();

I found user code by @bcat and @snowe2010 that wold benefit from this.

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[joukewitteveen]

We could even consider adding

#if defined(OLED_ENABLE) && OLED_TIMEOUT > 0
    oled_on();
#endif

below led_set() at the bottom of let_task. This is not strictly needed since if the oled task acts on the led state change, the changed oled content will also trigger oled_on(). The main difference will be when the led change is not rendered on the screen: those screens currently stay off but would turn on with the code suggested above.

[bcat]

I like the idea for consistency, but what's the advantage over the "leds != prev_leds" check I use now (

qmk_firmware/users/bcat/bcat_oled_pet_isda.c

Line 73 in f5d091a

if (redraw || leds.raw != prev_leds.raw) {

)?

Not intended as a criticism, I'm just curious if checking the last-modified time is roughly equivalent or if my sleepy weekend brain is missing something. :)

[bcat]

Oh, I guess maybe one difference is that right now, if the host changes LED state (e.g., it's not as a side effect of a key press), then with my current OLED code, the OLED won't wake up, whereas looking at the timer, it would.

I'm honestly not sure which behavior I'd prefer, but having the choice would be nice.

[joukewitteveen]

Oh, I guess maybe one difference is that right now, if the host changes LED state (e.g., it's not as a side effect of a key press), then with my current OLED code, the OLED won't wake up, whereas looking at the timer, it would.

I'm honestly not sure which behavior I'd prefer, but having the choice would be nice.

That is indeed the difference, so the interesting line would be this one:

qmk_firmware/users/bcat/bcat_oled.c

Line 47 in f5d091a

if (last_input_activity_elapsed() < TIMEOUT_MILLIS) {

Note that if you change the screen based on a LED state change, than the screen change will trigger oled_on(). However, if you have a custom timeout logic, than that would typically turn the screen back off immediately after. The result is that the host-initiated led state change caused the screen to flash briefly.

[bcat]

I see the point, that is an odd edge case. Yeah, it'd be nice to have a more complete "did anything change" API. Thanks for explaining!