If RGBLIGHT_EFFECT_BREATHE_CENTER is undefined, use fixed breathe table instead of exp() and sin()

[mtei]

Description

This is alternative (radical) version of #5480.

Changed rgblight.c's breathing effect to use a fixed table instead of calculating with exp() and sin(). This change reduces the size of the firmware when using the rgblight breathing effect.

I could not decide whether to adopt #5480 or this. So I presented both.
This is the result of compilation smaller than #5480, so use this.

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

• add RGBLIGHT_EFFECT_BREATHE_TABLE macro, If defined, use fixed breath table instead of exp() and sin() #5480 (comment)

Checklist

• My code follows the code style of this project.
• 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).

[Lenbok]

I've been testing this one for the last few days, it gives a nice reduction in firmware size!

[drashna]

Is it possible to specify a custom table for this?

[Lenbok]

@drashna In the top of rgblight_breathing_table_calc.c it tells you how to generate a custom table for your keymap.

[drashna]

Ah, I missed that.

And I see that if you add the define for the table in a config.h, you can use your own.

[drashna]

It's not really in the scope of this PR, but unfortunately, you cannot use the userspace folder to host the file. It has to be in a keymap file. Which is unfortunate.

Otherwise, it looks good!

[mtei]

The current makefile system has a search path set as follows:

$ make crkbd:drashna
....
$ cat .build/obj_crkbd_rev1_drashna/cflags.txt | tr ' ' '\n' | grep ^-I
-Ikeyboards/crkbd/keymaps/drashna       ** keymap **
-Ikeyboards/.
-Ikeyboards/.
-Ikeyboards/.
-Ikeyboards/crkbd
-Ikeyboards/crkbd/rev1
-I.
-Itmk_core
-Iquantum                               ** quantum **
-Iquantum/keymap_extras
-Iquantum/audio
-Iquantum/process_keycode
-Iquantum/api
-Idrivers
-Iquantum/serial_link
-Idrivers/oled
-Idrivers/avr
-Iusers/drashna                         ** users ***
-Itmk_core/protocol
-Itmk_core/common
-Itmk_core/protocol/lufa
-Ilib/lufa

I feel that it is ideal to change build_keyboard.mk as follows.

diff --git a/build_keyboard.mk b/build_keyboard.mk
index 5d633f271..510923ebe 100644
--- a/build_keyboard.mk
+++ b/build_keyboard.mk
@@ -334,9 +334,9 @@ SRC += $(KEYBOARD_SRC) \
 
 # Search Path
 VPATH += $(KEYMAP_PATH)
+VPATH += $(USER_PATH)
 VPATH += $(KEYBOARD_PATHS)
 VPATH += $(COMMON_VPATH)
-VPATH += $(USER_PATH)
 
 include common_features.mk
 include $(TMK_PATH)/protocol.mk

However, the easiest way is the following.

diff --git a/users/drashna/rules.mk b/users/drashna/rules.mk
index bef25e259..37554489d 100644
--- a/users/drashna/rules.mk
+++ b/users/drashna/rules.mk
@@ -1,3 +1,4 @@
+VPATH += $(USER_PATH)
 SRC += drashna.c \
        process_records.c

[mtei]

Another way is to do the following:

$ ./util/rgblight_breathing_table_calc > users/drashna/breathe_table.h
$ echo "#include <breathe_table.h>" > keyboards/crkbd/keymaps/drashna/rgblight_breathe_table.h

[drashna]

Thanks for the info!

I think the changes to build_keyboard.mk is the best option, long term. But outside the scope of this PR. Until that change drops, the change to the userspace looks fine.

Also, I've tested the code, and it works perfectly. I don't see any issues with it, and I do see a drop in the firmware size. And I'd like to see this merged sooner, rather than later.

Also, a similar change for the rgb matrix feature may be a good idea, for the future.

[mechmerlin]

Thanks!