endgame mac + fixing endgame - massdrop/ctrl #14746

rshutt · 2021-10-07T23:22:03Z

Description

This pr both adds an endgame_mac keymap and fixes the endgame keymap.

Changes introduced in #12097 (4b96d58) actually broke the endgame keymap as the function md_rgb_matrix_indicators was replaced with md_rgb_matrix_indicators_advanced. The former had no args and the latter took a led_min and led_max integer argument. Turns out, min = 0 and max = ISSI3733_LED_COUNT

Furthermore, with the endgame_mac keymap, I changed some of the matrix effects and set the default to be something I liked :). As expected, the endgame_mac keymap takes into account the swapped cmd and alt keys!

Types of Changes

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

Issues Fixed or Closed by This PR

Not sure what issues are fixed. I didn't bother to create an issue

Checklist

[ X ] My code follows the code style of this project: C, Python
[ X ] 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.
[ X ] I have read the CONTRIBUTING document.
I have added tests to cover my changes.
[ X ] I have tested the changes and verified that they work and don't break anything (as well as I can manage).

I've also had to fix the md_rgb_matrix_indicator function issue as this was replaced by md_rgb_matrix_indicator_advanced which takes a minimum and maximum led index number.

keyboards/massdrop/ctrl/keymaps/endgame/config_led.c

ash0x0

LGTM. Thanks for the fix.

rshutt · 2021-10-08T05:35:38Z

LGTM. Thanks for the fix.

Anytime :). I did it for selfish reasons since I have two of these keyboards. Maybe I might TAL at what LOE is required to do the things required to save this device from FOSS gulag :/

keyboards/massdrop/ctrl/keymaps/endgame_mac/config.h

keyboards/massdrop/ctrl/keymaps/endgame_mac/config_led.c

keyboards/massdrop/ctrl/keymaps/endgame_mac/keymap.c

keyboards/massdrop/ctrl/keymaps/endgame_mac/keymap.h

keyboards/massdrop/ctrl/keymaps/endgame_mac/rules.mk

rshutt · 2021-10-12T21:34:41Z

OK we should be good to go now, right?

Thanks,
-r

nbauernfeind · 2021-10-29T04:02:57Z

Thanks for doing this! Am new to the game and the first thing I wanted to try is broken! Happy to discover that you have already fixed it.

rshutt · 2021-10-29T06:45:45Z

No problem man. Thing is I guess even though I made the suggested changes, this PR is still sitting here rotting. Needs to be merged!

…

On Fri, Oct 29, 2021, 12:03 AM Nate Bauernfeind ***@***.***> wrote: Thanks for doing this! Am new to the game and the first thing I wanted to try is broken! Happy to discover that you have already fixed it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#14746 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AMIHSGOUVLC5DRMEVDQQUBLUJIMHZANCNFSM5FSL5YCQ> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

nbauernfeind · 2021-10-29T14:20:04Z

keyboards/massdrop/ctrl/keymaps/endgame_mac/keymap.c

+            // ======================================================== CUSTOM KEYCOADS BELOW ========================================================
+            case COPY_ALL:
+                // Selects all and text and copy
+                SEND_STRING(SS_LCTRL("ac"));


Might make sense to swap this from LCTRL to LGUI. As is, this shortcut doesn't work on the mac.

Is that right? LCTRL doesn't work?

MacOS basically swaps the control and gui functionality.

What might work better is:

Suggested change

SEND_STRING(SS_LCTRL("ac"));

register_code(keycode_config(KC_LCTL));

tap_code(KC_A);

tap_code(KC_C);

unregister_code(keycode_config(KC_LCTL));

The keycode_config is from the magic feature, and will swap the code based on the Control-GUI swap option. That would make it windows and mac friendly.

Or, simply, just:

Suggested change

SEND_STRING(SS_LCTRL("ac"));

SEND_STRING(SS_LGUI("ac"));

Ok so what I know is that I simply edited an existing keymap to get this one kicking... And I'm currently using it! So whatever doesn't work isn't something I've needed. Ill take a look, but at this point, the juice is no longer worth the squeeze. It works for me, I put in a PR to try to make it work for others, but apparently, that ain't enough.

References: Reddit discussion: https://www.reddit.com/r/MechanicalKeyboards/comments/qdsm9r/help_compiling_qmk_endgame_for_qmk/ PR for fix: qmk#14746 Commit in PR with the fix: qmk@de7be63

This was the same issue as seen in the endgame keymap. References: Reddit discussion about endgame keymap: https://www.reddit.com/r/MechanicalKeyboards/comments/qdsm9r/help_compiling_qmk_endgame_for_qmk/ PR for endgame keymap fix: qmk#14746 Commit in PR with the fix for endgame keymap: qmk@de7be63

stale · 2022-01-03T21:45:30Z

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

drashna · 2022-01-07T05:17:33Z

keyboards/massdrop/ctrl/keymaps/endgame_mac/config.h

+// #define RGBLIGHT_EFFECT_SNAKE_LENGTH 4              // The number of LEDs to light up for the "Snake" animation
+
+// This list in in the correct mode order. Next mode is the following line, previous mode is previous line. Loops around.
+#define DISABLE_RGB_MATRIX_SOLID_COLOR                   // Static single hue, no speed support


these are no longer used.

When you say "these" are you including the other #define DISABLE.*'s seen listed afterwards?

e.g. DISABLE_RGB_MATRIX_ALPHAS_MOD... DISABLE_RGB_MATRIX_GRADIENT_UP_DOWN... etc?

the disable defines are no used anymore, instead you have to explicitly enable the animation modes

So if I nuke all of them, is the PR ok to merge?

@drashna I'm trying to understand if I make the changes you are suggesting in this comment, will my PR be approved and merged? Or are there yet other changes that have to be made? The level of effort here for making this PR part of the code base is very quickly eclipsing any personal value I would receive from the same.

We switched from an implicitly enabled unless explicitly disabled approach, to an explicitly enabled approach.

So the "disable mode" defines simply don't do anything anymore and should be removed.

Instead, you want to use the "enable mode" defines for the modes that you want.

This should be the full list of animation modes available:

# define ENABLE_RGB_MATRIX_ALPHAS_MODS # define ENABLE_RGB_MATRIX_GRADIENT_UP_DOWN # define ENABLE_RGB_MATRIX_GRADIENT_LEFT_RIGHT # define ENABLE_RGB_MATRIX_BREATHING # define ENABLE_RGB_MATRIX_BAND_SAT # define ENABLE_RGB_MATRIX_BAND_VAL # define ENABLE_RGB_MATRIX_BAND_PINWHEEL_SAT # define ENABLE_RGB_MATRIX_BAND_PINWHEEL_VAL # define ENABLE_RGB_MATRIX_BAND_SPIRAL_SAT # define ENABLE_RGB_MATRIX_BAND_SPIRAL_VAL # define ENABLE_RGB_MATRIX_CYCLE_ALL # define ENABLE_RGB_MATRIX_CYCLE_LEFT_RIGHT # define ENABLE_RGB_MATRIX_CYCLE_UP_DOWN # define ENABLE_RGB_MATRIX_RAINBOW_MOVING_CHEVRON # define ENABLE_RGB_MATRIX_CYCLE_OUT_IN # define ENABLE_RGB_MATRIX_CYCLE_OUT_IN_DUAL # define ENABLE_RGB_MATRIX_CYCLE_PINWHEEL # define ENABLE_RGB_MATRIX_CYCLE_SPIRAL # define ENABLE_RGB_MATRIX_DUAL_BEACON # define ENABLE_RGB_MATRIX_RAINBOW_BEACON # define ENABLE_RGB_MATRIX_RAINBOW_PINWHEELS # define ENABLE_RGB_MATRIX_RAINDROPS # define ENABLE_RGB_MATRIX_JELLYBEAN_RAINDROPS # define ENABLE_RGB_MATRIX_HUE_BREATHING # define ENABLE_RGB_MATRIX_HUE_PENDULUM # define ENABLE_RGB_MATRIX_HUE_WAVE # define ENABLE_RGB_MATRIX_PIXEL_RAIN # define ENABLE_RGB_MATRIX_PIXEL_FLOW # define ENABLE_RGB_MATRIX_PIXEL_FRACTAL // enabled only if RGB_MATRIX_FRAMEBUFFER_EFFECTS is defined # define ENABLE_RGB_MATRIX_TYPING_HEATMAP # define ENABLE_RGB_MATRIX_DIGITAL_RAIN // enabled only of RGB_MATRIX_KEYPRESSES or RGB_MATRIX_KEYRELEASES is defined # define ENABLE_RGB_MATRIX_SOLID_REACTIVE_SIMPLE # define ENABLE_RGB_MATRIX_SOLID_REACTIVE # define ENABLE_RGB_MATRIX_SOLID_REACTIVE_WIDE # define ENABLE_RGB_MATRIX_SOLID_REACTIVE_MULTIWIDE # define ENABLE_RGB_MATRIX_SOLID_REACTIVE_CROSS # define ENABLE_RGB_MATRIX_SOLID_REACTIVE_MULTICROSS # define ENABLE_RGB_MATRIX_SOLID_REACTIVE_NEXUS # define ENABLE_RGB_MATRIX_SOLID_REACTIVE_MULTINEXUS # define ENABLE_RGB_MATRIX_SPLASH # define ENABLE_RGB_MATRIX_MULTISPLASH # define ENABLE_RGB_MATRIX_SOLID_SPLASH # define ENABLE_RGB_MATRIX_SOLID_MULTISPLASH

keyboards/massdrop/ctrl/keymaps/endgame_mac/rules.mk

stale · 2022-04-25T05:45:36Z

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

stale · 2022-06-12T15:55:11Z

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.

rshutt added 2 commits October 7, 2021 19:09

This mostly just creates an endgame keymap, but for a mac layout.

14128c3

I've also had to fix the md_rgb_matrix_indicator function issue as this was replaced by md_rgb_matrix_indicator_advanced which takes a minimum and maximum led index number.

Also fixing the original endgame keymap

de7be63

github-actions bot added the keymap label Oct 7, 2021

zvecr reviewed Oct 8, 2021

View reviewed changes

keyboards/massdrop/ctrl/keymaps/endgame/config_led.c Show resolved Hide resolved

ash0x0 approved these changes Oct 8, 2021

View reviewed changes

drashna requested changes Oct 11, 2021

View reviewed changes

rshutt added 2 commits October 12, 2021 17:30

Adding changes suggestged by @drashna

25ec222

Hmm where'd that whitespace come from\!

6e7bae7

nbauernfeind reviewed Oct 29, 2021

View reviewed changes

Missing whitespace after GPL-compatible license?

38eb6bc

rshutt requested a review from drashna October 30, 2021 02:18

stale bot added the awaiting changes label Jan 3, 2022

drashna requested changes Jan 7, 2022

View reviewed changes

stale bot removed the awaiting changes label Jan 7, 2022

Setting BOOTMAGIC_ENABLE = yes (which enables lite per @drashna)

c0e4c8b

zvecr mentioned this pull request Mar 7, 2022

Endgame failing to compile #16535

Closed

stale bot added the awaiting changes label Apr 25, 2022

stale bot closed this Jun 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

endgame mac + fixing endgame - massdrop/ctrl #14746

endgame mac + fixing endgame - massdrop/ctrl #14746

rshutt commented Oct 7, 2021

ash0x0 left a comment

rshutt commented Oct 8, 2021

rshutt commented Oct 12, 2021

nbauernfeind commented Oct 29, 2021

rshutt commented Oct 29, 2021 via email

nbauernfeind Oct 29, 2021

rshutt Oct 29, 2021

drashna Nov 4, 2021

rshutt Mar 10, 2022

stale bot commented Jan 3, 2022

drashna Jan 7, 2022

rshutt-va Jan 10, 2022

drashna Jan 11, 2022

rshutt-va Jan 13, 2022

rshutt Mar 10, 2022

drashna Mar 10, 2022

stale bot commented Apr 25, 2022

stale bot commented Jun 12, 2022

-                SEND_STRING(SS_LCTRL("ac"));
+                register_code(keycode_config(KC_LCTL));
+                tap_code(KC_A);
+                tap_code(KC_C);
+                unregister_code(keycode_config(KC_LCTL));

endgame mac + fixing endgame - massdrop/ctrl #14746

endgame mac + fixing endgame - massdrop/ctrl #14746

Conversation

rshutt commented Oct 7, 2021

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

ash0x0 left a comment

Choose a reason for hiding this comment

rshutt commented Oct 8, 2021

rshutt commented Oct 12, 2021

nbauernfeind commented Oct 29, 2021

rshutt commented Oct 29, 2021 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stale bot commented Jan 3, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stale bot commented Apr 25, 2022

stale bot commented Jun 12, 2022