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.

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

[Keymap] Update bcat's keymaps/userspace to share logic, add OLED functionality, and set up one of my macropads for WFH #14702

Merged
merged 23 commits into from
Dec 27, 2021

Conversation

bcat
Copy link
Contributor

@bcat bcat commented Oct 4, 2021

Description

There's a few changes here that I've been testing over the past several months and that I'd like to go ahead and merge before anything bit rots too much. I can split this into smaller PRs if that's preferable, just let me know.

Unfortunately, my Corne keymap is getting a bit big, and is now oversized on avr-gcc 5.4.0 that Debian Bullseye ships with. I managed to squeak just under the limit (just 28 bytes to spare!) with an avr-gcc 8.5.0 Linux toolchain built by Lumito, but will probably need something like NO_ACTION_TAPPING (with the layer fixes from #11528) if core grows any bigger. Or maybe I should just give up some RGB modes.... :)

After merging in latest QMK changes, it seems I've got a fair number of bytes left for my Crkbd (712 bytes free with avr-gcc 8.5.0). :)

First, some organizational improvements and cleanups:

  • Added a script (users/bcat/compile.sh) to my userspace to build all the keyboard/keymap combos I use. This makes it easy to test changes and make sure I'm not breaking anything, as well as to rebuild all my firmware binaries after pulling latest QMK master into my fork.
  • Moved userspace RGB configuration into its own source file.
  • Factored my standard layer configurations for ortho/columnar-staggered and row-staggered keyboards into my userspace to reduce duplication across keymaps. (The BCAT_ORTHO_LAYERS macro controls which set of layers is used.)
  • Factored my standard keycode aliases into my userspace to further cut down on boilerplate.
  • Disabled a variety of unused features to enable my Crkbd to build with OLED + RGB Matrix features enabled.
  • Removed NK_TOGG keybindings since I use FORCE_NKRO now. (It "just works" on all my hardware, even in BIOS.)
  • Only activate Bootmagic Lite for keyboards that don't have a reset button accessible: Polaris (no reset hole on case), DZ60 (early rev. 3 PCBs put reset button in a nonstandard place), KBD67 Hotswap (no reset hole on case). This lets me save a few bytes on boards like the the Crkbd that do have accessible reset buttons.

Second, the addition of an OLED library to my userspace:

  • Added a basic library of helpers for sharing the bulk of my oled_task_user implementation across keymaps. I added a few weak functions to my userspace following the pattern established by QMK core.
  • Added a framework to my userspace for implementing "keyboard pets", simple animations of a few frames each that can respond to keystrokes, etc.
  • This framework was originally inspired by the Luna keyboard pet by HellSingCoder already in the repo. However, I didn't share any code with that implementation in the framework itself, but instead generalized the concept to support different animations and to better fit the particular structure of my keymaps.
  • Implemented custom OLED timeout behavior since the QMK core implementation doesn't allow the OLED to ever turn off if an animation is continuously running.
  • Use the new SPLIT_OLED_ENABLE feature to sync OLED on/off status set by the custom timeout impl between halves of the keyboard. :)

Third, implemented a couple of "keyboard pets" in my userspace framework using open-source artwork:

  • Implemented "Luna" keyboard pet in my userspace using GPL'd artwork by @HellSingCoder from his original Luna implementation. I preserved his original copyright notice and included attribution, so it should be clear that this isn't the original implementation, but rather an adaption. (However, if HellSingCoder would rather I rename the bcat_oled_pet_luna.c file to something else to make it extra clear this isn't his canonical version, I'd be happy to do that too. And he's good with the naming too. :D)
  • Implemented "Isda" keyboard pet in my userspace using GPL'd artwork from OpenGameArt user sparrow666. Once again I included attribution/license info, and in this case the name (bcat_oled_pet_isda.c) is just a reference to a D&D campaign I play in. :)

Finally, actual functional changes to keymaps:

  • Wired up OLED for Lily58 and Crkbd, using the "Luna" keyboard pet. (The "Isda" keyboard pet is used by my Unicorne keymap, but that keyboard isn't merged into upstream QMK yet, so I excluded the associated keymap from this PR.)
  • Updated my 9-Key macropad's keyboard for use when working from home.

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

None

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).

@github-actions github-actions bot added the keymap label Oct 4, 2021
@HellSingCoder
Copy link

Good job! I'm happy for you to keep luna as a name 👍🏼

@bcat
Copy link
Contributor Author

bcat commented Oct 4, 2021

Good job! I'm happy for you to keep luna as a name 👍🏼

Sounds good, thanks much for the idea & original implementation!

@HellSingCoder
Copy link

I wasn't expecting all the attention that it got to be honest but, I'm glad that a lot of people liked it. If you have a video of it working I'll be happy to see it 😄

users/bcat/bcat.c Outdated Show resolved Hide resolved
users/bcat/bcat_oled.c Outdated Show resolved Hide resolved
@bcat
Copy link
Contributor Author

bcat commented Oct 5, 2021

Thanks for the review! BTW, the review email I got showed a change to BCAT_OLED_PET logic in rules.mk, but I don't see the suggestion in GitHub for some reason. Still want me to give that a try? I'm not opposed if there's a way to make that logic more general purpose.

@bcat
Copy link
Contributor Author

bcat commented Oct 5, 2021

I went ahead and incorporated a simplified version of the logic from the email. It doesn't explicitly check for a missing file in the makefile, but instead just lets it linker error like so:

Linking: .build/crkbd_rev1_bcat_split_3x6_3.elf                                                     [ERRORS]
 |
 | avr-gcc: error: .build/obj_crkbd_rev1_bcat_split_3x6_3/bcat_oled_pet_cow.o: No such file or directory
 |

I feel like the error is clear enough that that's fine in practice, though.

@bcat bcat requested a review from drashna October 5, 2021 05:09
@drashna
Copy link
Member

drashna commented Oct 5, 2021

Thanks for the review! BTW, the review email I got showed a change to BCAT_OLED_PET logic in rules.mk, but I don't see the suggestion in GitHub for some reason. Still want me to give that a try? I'm not opposed if there's a way to make that logic more general purpose.

I removed it, because I ran into issues with it, and ... yeah.

@drashna
Copy link
Member

drashna commented Oct 5, 2021

And it quick check looks like this may be what you want to actually check if the file exists.

ifneq ("$(wildcard $(USER_PATH)/bcat_oled_pet_$(BCAT_OLED_PET).c)","")

@bcat
Copy link
Contributor Author

bcat commented Oct 5, 2021

And it quick check looks like this may be what you want to actually check if the file exists.

ifneq ("$(wildcard $(USER_PATH)/bcat_oled_pet_$(BCAT_OLED_PET).c)","")

Do you mind if I leave this out? I feel like it's a little cleaner to have a build error if the BCAT_OLED_PET value doesn't correspond to a valid source file instead of silently ignoring invalid values.

But I don't feel strongly about this, so if you do have a preference I'll go with whatever you suggest.

@drashna
Copy link
Member

drashna commented Oct 5, 2021

Well, you can have it generate an error if it's not found, etc. But this should be fine. Mostly, it was to allow for a simple way to add more.

@drashna drashna requested a review from a team October 11, 2021 03:35
@bcat
Copy link
Contributor Author

bcat commented Nov 16, 2021

Rebased against latest master and resolved the merge conflict. Everything still builds okay. :)

@drashna
Copy link
Member

drashna commented Nov 28, 2021

Sorry, but again?

users/bcat/bcat_oled.c Outdated Show resolved Hide resolved
@bcat
Copy link
Contributor Author

bcat commented Nov 28, 2021

Sorry, but again?

No worries, all good. Everything should be re-merged now.

By the way, I also have a new keymap for the Unicorne keyboard ready: bcat@a8ed985. I had left it out of this PR since the Unicorne keyboard was in various stages of "not merged yet" and "doesn't compile" for a bit. But now the Unicorne keyboard is working fine at HEAD, and I wasn't sure if I should add my keymap for it to this PR, or if it would be better to make a separate (much smaller) PR after this one is merged.

No preference on my end, and I'm not in any rush on this. :) Just figured I'd ask which option is preferable.

@drashna
Copy link
Member

drashna commented Nov 28, 2021

i'd say wait for it, or open up another pr.

@bcat
Copy link
Contributor Author

bcat commented Nov 28, 2021

Sounds good, I'll just wait till this is merged then. (Again, no rush from my end, and I know there's a big backlog of PRs. So I don't mind waiting.)

@drashna drashna requested a review from a team November 28, 2021 22:59
bcat and others added 16 commits December 2, 2021 10:45
This enables OLED pets to draw custom widgets (e.g., LED indicator
status) on top of their animation frames.
For future use on my Unicorne keyboard. Unicorn artwork by sparrow666,
licensed under GPL v2.0.

See also: https://opengameart.org/content/unicorn-2
The default implementation never lets the OLED turn off if a continuous
animation is in progress. The custom one does.
No change in firmware size, but makes keymaps read a little nicer and
enables more functionality in OLED pets.
The new extensible split transport for Split Common finally allows OLED
on/off status to be synced between halves of the keyboard. :)

Unfortunately, this required disabling Bootmagic Lite to keep my Crkbd
under the firmware size limit. (I now after 28 bytes free on avr-gcc
version 8.5.0.) So now I'll enable Bootmagic only on keyboards that
actually require it, i.e., ones lacking an accessible reset button.
The default max brightness is only 120 rather than 150, but that might
actually fix some weirdness I've seen with bright white LED settings.
The general trend these days seems to be enabling only the modes you
want, so I'm manually expanding the ones currently enabled by
RGBLIGHT_ANIMATIONS.

I'd like to try out the TWINKLE mode too, but it seems not to work at
all on ARM right now, and all my usable RGBLIGHT keebs are ARM boards.
My Crkbd still has a reasonable amount of free space with these:
27974/28672 (97%, 698 bytes free). The RGB_MATRIX_KEYPRESSES effects
would put it over the firmware size limit, but I really don't ever use
those anyway.
@bcat
Copy link
Contributor Author

bcat commented Dec 2, 2021

Just one more rebase atop master, no extra changes. Nothing to see here... :)

@tzarc tzarc merged commit 93bc737 into qmk:master Dec 27, 2021
@bcat bcat deleted the bcat_keebs branch December 28, 2021 03:21
tiktuk pushed a commit to tiktuk/qmk_firmware that referenced this pull request Jan 1, 2022
…ctionality, and set up one of my macropads for WFH (qmk#14702)

* Add script to build all bcat keymaps at once

* Move userspace RGB to separate source file

* Move layer handling logic into userspace

* Move keycap aliases into userspace

* Add OLED userspace library and Lily58 OLED setup

* Add Luna keyboard pet, generic OLED pet framework

Luna artwork and original implementation by HellSingCoder, licensed
under GPL v2.0.

See also: https://github.com/qmk/qmk_firmware/blob/6dfe915e26d7147e6c2bed495d3b01cf5b21e6ec/keyboards/sofle/keymaps/helltm/keymap.c

* Use OLED on bcat's Crkbd

I had to turn off a few unused features to address firmware size limits.

* Remove vestigial NK_TOGG keybindings

* Add post-render hook to OLED pet API

This enables OLED pets to draw custom widgets (e.g., LED indicator
status) on top of their animation frames.

* Add Isda keyboard pet

For future use on my Unicorne keyboard. Unicorn artwork by sparrow666,
licensed under GPL v2.0.

See also: https://opengameart.org/content/unicorn-2

* Replace OLED timeout implementation with custom

The default implementation never lets the OLED turn off if a continuous
animation is in progress. The custom one does.

* Move keyboard state for OLED functions into struct

No change in firmware size, but makes keymaps read a little nicer and
enables more functionality in OLED pets.

* Enable continuously running OLED pet (for Luna)

* Sync OLED state; enable Bootmagic only when needed

The new extensible split transport for Split Common finally allows OLED
on/off status to be synced between halves of the keyboard. :)

Unfortunately, this required disabling Bootmagic Lite to keep my Crkbd
under the firmware size limit. (I now after 28 bytes free on avr-gcc
version 8.5.0.) So now I'll enable Bootmagic only on keyboards that
actually require it, i.e., ones lacking an accessible reset button.

* Update 9-Key macropad keymap for working from home

* Remove includes redundant with quantum.h

Co-authored-by: Drashna Jaelre <[email protected]>

* Simplify BCAT_OLED_PET makefile logic

* Swap some keys on my 9-Key macropad around

* Inline spurious variable in OLED code

* Remove max brightness that's now set by default

The default max brightness is only 120 rather than 150, but that might
actually fix some weirdness I've seen with bright white LED settings.

* Enable specific RGBLIGHT modes instead of default

The general trend these days seems to be enabling only the modes you
want, so I'm manually expanding the ones currently enabled by
RGBLIGHT_ANIMATIONS.

I'd like to try out the TWINKLE mode too, but it seems not to work at
all on ARM right now, and all my usable RGBLIGHT keebs are ARM boards.

* Reenable RGB_MATRIX animations after qmk#15018

My Crkbd still has a reasonable amount of free space with these:
27974/28672 (97%, 698 bytes free). The RGB_MATRIX_KEYPRESSES effects
would put it over the firmware size limit, but I really don't ever use
those anyway.

* Use new get_u8_str function for WPM display

Co-authored-by: Drashna Jaelre <[email protected]>
ryphon pushed a commit to ryphon/qmk_firmware that referenced this pull request Jan 12, 2022
…ctionality, and set up one of my macropads for WFH (qmk#14702)

* Add script to build all bcat keymaps at once

* Move userspace RGB to separate source file

* Move layer handling logic into userspace

* Move keycap aliases into userspace

* Add OLED userspace library and Lily58 OLED setup

* Add Luna keyboard pet, generic OLED pet framework

Luna artwork and original implementation by HellSingCoder, licensed
under GPL v2.0.

See also: https://github.com/qmk/qmk_firmware/blob/6dfe915e26d7147e6c2bed495d3b01cf5b21e6ec/keyboards/sofle/keymaps/helltm/keymap.c

* Use OLED on bcat's Crkbd

I had to turn off a few unused features to address firmware size limits.

* Remove vestigial NK_TOGG keybindings

* Add post-render hook to OLED pet API

This enables OLED pets to draw custom widgets (e.g., LED indicator
status) on top of their animation frames.

* Add Isda keyboard pet

For future use on my Unicorne keyboard. Unicorn artwork by sparrow666,
licensed under GPL v2.0.

See also: https://opengameart.org/content/unicorn-2

* Replace OLED timeout implementation with custom

The default implementation never lets the OLED turn off if a continuous
animation is in progress. The custom one does.

* Move keyboard state for OLED functions into struct

No change in firmware size, but makes keymaps read a little nicer and
enables more functionality in OLED pets.

* Enable continuously running OLED pet (for Luna)

* Sync OLED state; enable Bootmagic only when needed

The new extensible split transport for Split Common finally allows OLED
on/off status to be synced between halves of the keyboard. :)

Unfortunately, this required disabling Bootmagic Lite to keep my Crkbd
under the firmware size limit. (I now after 28 bytes free on avr-gcc
version 8.5.0.) So now I'll enable Bootmagic only on keyboards that
actually require it, i.e., ones lacking an accessible reset button.

* Update 9-Key macropad keymap for working from home

* Remove includes redundant with quantum.h

Co-authored-by: Drashna Jaelre <[email protected]>

* Simplify BCAT_OLED_PET makefile logic

* Swap some keys on my 9-Key macropad around

* Inline spurious variable in OLED code

* Remove max brightness that's now set by default

The default max brightness is only 120 rather than 150, but that might
actually fix some weirdness I've seen with bright white LED settings.

* Enable specific RGBLIGHT modes instead of default

The general trend these days seems to be enabling only the modes you
want, so I'm manually expanding the ones currently enabled by
RGBLIGHT_ANIMATIONS.

I'd like to try out the TWINKLE mode too, but it seems not to work at
all on ARM right now, and all my usable RGBLIGHT keebs are ARM boards.

* Reenable RGB_MATRIX animations after qmk#15018

My Crkbd still has a reasonable amount of free space with these:
27974/28672 (97%, 698 bytes free). The RGB_MATRIX_KEYPRESSES effects
would put it over the firmware size limit, but I really don't ever use
those anyway.

* Use new get_u8_str function for WPM display

Co-authored-by: Drashna Jaelre <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants