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] Add andrebrait layout for GMMK Pro #13932

Merged
merged 11 commits into from Aug 12, 2021

Conversation

andrebrait
Copy link
Contributor

Description

Just my own layout with an implementation for the CAPS LOCK indicator that works regardless of the actual status of the RGB Matrix.

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

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 Aug 9, 2021
@andrebrait andrebrait requested a review from drashna August 9, 2021 19:17
@stickandgum
Copy link

stickandgum commented Aug 10, 2021

Hi @andrebrait, I'm new to QMK and the GMMK Pro and trying my own custom keymap.c. Learning from what you've done here. (My messy keymap is not quite right : https://paste.ofcode.org/4vYCedvzfpKBcN4qKBuSb4 )

Can you tell me what each line does in the following snippet (I understand IFDEF checks to make sure RGB_MATRIX_ENABLE is yes in rules.mk:

#ifdef RGB_MATRIX_ENABLE
void keyboard_post_init_user(void) {
    /*
    Custom handling of RGB_TOG means it is impossible to toggle the RGB Matrix on and off and we
    need it to be enabled for the CAPS LOCK indicator to work, so enable it and then make sure it
    gets persisted to the EEPROM.
     */
    if (!rgb_matrix_is_enabled()) {
        rgb_matrix_enable_noeeprom();
        eeconfig_update_rgb_matrix();
    }
}

If the RGB is NOT enabled you have :
rgb_matrix_enable_noeeprom();
eeconfig_update_rgb_matrix();

Aren't these two functions the same as: rgb_matrix_enable() alone?

@andrebrait

This comment has been minimized.

@andrebrait

This comment has been minimized.

case RGB_VAI:
case RGB_VAD:
case RGB_SPI:
case RGB_SPD:

This comment was marked as off-topic.

@@ -14,3 +14,15 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include "pro.h"

#ifdef RGB_MATRIX_ENABLE
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@drashna without this the GMMK Pro wouldn't wake up from a USB suspend. I guess this now counts as a bugfix 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With it, it's somewhat random whether or not it will wake, but I won't spend too much time on this. I can revert this bit if you think it's better to leave this out.

@andrebrait
Copy link
Contributor Author

My bad for the apparent mess and number of commits. I was refining the logic yesterday and I just finished it.

Now I don't control writes to the EEPROM manually. Using flags to control state is quite clean, in the end.
I implemented a blinking indicator when toggling NKRO. Since it's just a little of RGB stuff, I did it on the keymap itself.

@andrebrait
Copy link
Contributor Author

@stickandgum 5ms because I had a couple double leys with 2ms and with the new debouncing algorithm I figured I would have low latency even when pressing keys like crazy, but I have a proposal for making debounce changeable during execution that hasn't received comments yet.

As for waking up, currently any key works for me. I guess it's more of a computer firmware thing than an OS thing (though I think you can use the OS to configure that on the computer's firmware level).

@github-actions github-actions bot added via Adds via keymap and/or updates keyboard for via support and removed keyboard labels Aug 11, 2021
@drashna drashna requested a review from a team August 12, 2021 02:47
@drashna
Copy link
Member

drashna commented Aug 12, 2021

Thanks!

@drashna drashna merged commit 33f6490 into qmk:develop Aug 12, 2021
nhongooi pushed a commit to nhongooi/qmk_firmware that referenced this pull request Dec 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keymap via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants