Moonlander: Add RGB LED layout map macro #18745
Conversation
|
This PR is the result of a piece of documentation:
In other words, I would like to use the macro proposed here in a keymap that I intend to open a merge request for. |
joukewitteveen
force-pushed
the
moonlander-led-layout
branch
2 times, most recently
from
14d7629 to
8b5a886
Compare
keyboards/moonlander/moonlander.c
Outdated
| 1, 4, 4, 4, 4, 4, | ||
| 4, 4, 1, 1, 1, 1 | ||
| } }; | ||
| }, LED_LAYOUT_moonlander( |
There was a problem hiding this comment.
Unfortunately, qmk lint (lib/python/qmk/c_parse.py) does not like the use of macros here. Reverting would be easy, but using this macro brought to light some issues, so there is value in using a macro here.
There was a problem hiding this comment.
See this thread #18973 (comment)
There was a problem hiding this comment.
I don't believe g_led_config can be represented in info.json yet, can it? I guess the benefits provided by using a macro here (i.e. catching mapping oddities) are out of reach to the DD approach, or are there some cool plans in this direction?
There was a problem hiding this comment.
It's possible, but a lot of the data-driven stuff is not well documented just yet, and some things are only in develop (which should merge in a day or so).
https://github.com/qmk/qmk_firmware/blob/master/keyboards/1upkeyboards/pi60_hse/info.json#L31-L51
https://github.com/qmk/qmk_firmware/blob/master/data/schemas/keyboard.jsonschema#L294-L320
There was a problem hiding this comment.
Hmm, unfortunately the order in the json file is the LED indexing order. This precludes generating a layout macro from them automatically. Automatic generation on the basis of a key layout is possible, but only if every led has a position on the matrix that also has a key assigned to it. Even then, it is quite involved and we don't have a way to configure the name of the macro (LED_{layout_name} is possible, though).
I guess for now this leaves no other option than to set the LED layout macro the old fashioned way in moonlander.h, but revert its usage in defining g_led_config. I'll do that once I know what to do with the weird positions currently set in g_led_config.
keyboards/moonlander/moonlander.c
Outdated
| 4, 4, 1, 1, 1, 1 | ||
| } }; | ||
| }, LED_LAYOUT_moonlander( | ||
| { 0, 0 }, { 17, 0 }, { 34, 0 }, { 51, 0 }, { 68, 0 }, { 86, 0 }, { 105, 0 }, { 146, 0 }, { 163, 0 }, { 181, 0 }, { 198, 0 }, { 216, 0 }, { 233, 0 }, { 250, 0 }, |
There was a problem hiding this comment.
I noted that the steps in the x-coordinate are uneven. They are mostly 17, but between 86 and 105 is a step size of 19. Also, the distribution of some excess horizontal spacing is uneven: the columns between which we have a step size of 18 differ between the left and right halves.
Moreover, the midpoint (set to 125) is not halfway between 105 and 146.
keyboards/moonlander/moonlander.c
Outdated
| { 0, 25 }, { 17, 25 }, { 34, 25 }, { 51, 25 }, { 68, 25 }, { 86, 25 }, { 105, 25 }, { 146, 25 }, { 163, 25 }, { 181, 25 }, { 198, 25 }, { 216, 25 }, { 233, 25 }, { 250, 25 }, | ||
| { 0, 38 }, { 17, 38 }, { 34, 38 }, { 51, 38 }, { 68, 38 }, { 86, 38 }, { 105, 38 }, { 146, 38 }, { 163, 38 }, { 181, 38 }, { 198, 38 }, { 216, 38 }, { 233, 38 }, { 250, 38 }, | ||
| { 0, 51 }, { 17, 51 }, { 34, 51 }, { 51, 51 }, { 68, 51 }, { 116, 59 }, { 131, 59 }, { 181, 51 }, { 198, 51 }, { 216, 51 }, { 233, 51 }, { 250, 51 }, | ||
| { 90, 55 }, { 105, 68 }, { 116, 86 }, { 146, 86 }, { 161, 68 }, { 161, 55 } |
There was a problem hiding this comment.
Something weird is going on with the column coordinates of the right thumb cluster. The coordinates in the left thumb cluster seem reasonable.
Before addressing this or the other layout issue, I would like to know how these coordinates were obtained. @drashna, do you know?
And fix led positions. Mistakes were uncovered when introducing the LED layout map macro to the definition of g_led_config, but that had to be reverted because our tooling does not expand macros yet wants to read the led configuration.
joukewitteveen
force-pushed
the
moonlander-led-layout
branch
from
8b5a886 to
695b007
Compare
Description
A macro that allows pretty definitions of RGB LED maps. The macro name
LED_LAYOUT_moonlanderis rather generic, maybeRGB_LAYOUT_moonlanderwas better, but it is in line with the macro name used for ergodox_ez (LED_LAYOUT_ergodox_pretty) and the name used for the keyhive sofle (LED_LAYOUT).Types of Changes
Checklist