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

AMJ96 Refactor and Configurator update #3707

Merged
merged 7 commits into from Aug 22, 2018
Merged

Conversation

noroadsleft
Copy link
Member

AMJ96 Refactor

LAYOUT matrix and default keymap refactored to use full-length keycodes (instead of shorthand).

Configurator update

Minor physical layout update and made the labels more sensible.

Readme update

White space correction.

Default keymap update: #include QMK_KEYBOARD_H

rules.mk update

Disabled the following options as my local test compile's .hex ended up too big to fit in the available flash space:

  • MOUSEKEY_ENABLE
  • EXTRAKEY_ENABLE
  • BACKLIGHT_ENABLE
  • RGBLIGHT_ENABLE

If this is undesirable, suggestions are welcome.

LAYOUT matrix and default keymap refactored to use full-length keycodes (instead of shorthand).
Minor physical layout update and made the labels more sensible.
White space correction.
Disabled the following options as my local test compile's .hex ended up too big to fit in the available flash space:

MOUSEKEY_ENABLE
EXTRAKEY_ENABLE
BACKLIGHT_ENABLE
RGBLIGHT_ENABLE

If this is undesirable, suggestions are welcome.
@@ -18,25 +18,28 @@

#include "quantum.h"

// readability
#define ___ KC_NO
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we typically have KC_NO shorthanded as rather than shorter spaces as it can be misread as a KC_TRNS which has 7 spaces.

Copy link
Member

Choose a reason for hiding this comment

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

I think this should be fine, as most people don't go looking in the keyboard files for stuff.

Copy link
Member Author

Choose a reason for hiding this comment

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

I use ___ for KC_NO in keyboard.h files because I find it easier to spot at a glance than XXX, because the switch identifiers are shorter length.

I do stick with the standard usage for keymap.c though.

Copy link
Contributor

Choose a reason for hiding this comment

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

i think my issue is more the fact that there are two shorthands for it. People who don't know the code quite as well will think the spaces is KC_TRNS. Can we agree on one form of shorthand or atleast document that ___ is KC_NO and _______ is KC_TRNS.

FAUXCLICKY_ENABLE = no # Use buzzer to emulate clicky switches
NKRO_ENABLE ?= yes # USB Nkey Rollover
BACKLIGHT_ENABLE ?= no # Enable keyboard backlight functionality on B7 by default
RGBLIGHT_ENABLE = no
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to keep both lighting options turned on as we have no way to "turn it on" via the configurator this way. Also BACKLIGHT_ENABLE = yes should be sufficient. No need to put that ? in.

Copy link
Member

Choose a reason for hiding this comment

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

Turning off COMMAND would be safer, I think, as that will save a good deal of space.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh yes COMMAND is better!

Copy link
Member Author

Choose a reason for hiding this comment

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

@mechmerlin I left the question mark because it was already there, and I didn't know what the difference was between = and ?= other than knowing there was one until five minutes ago.

Will disable COMMAND_ENABLE (saves about 3 KB in my local compile).

@drashna
Copy link
Member

drashna commented Aug 22, 2018

@noroadsleft to save space, it may be best to turn off the COMMAND functionality, since that uses a lot of space.
Additionally, adding this to the config.h may help too:

#define NO_ACTION_MACRO
#define NO_ACTION_FUNCTION

Worst case, it may help to add EXTRAFLAGS += -flto to the rules.mk.

Re-enabled MOUSEKEY_ENABLE, EXTRAKEY_ENABLE, BACKLIGHT_ENABLE, and RGBLIGHT_ENABLE and disabled COMMAND_ENABLE in rules.mk; enabled NO_ACTION_MACRO and NO_ACTION_FUNCTION per @drashna
@drashna
Copy link
Member

drashna commented Aug 22, 2018

Thanks!

@drashna drashna merged commit 356fe59 into qmk:master Aug 22, 2018
@noroadsleft noroadsleft deleted the cf/amj96 branch August 22, 2018 17:53
muramasa64 pushed a commit to muramasa64/qmk_firmware that referenced this pull request Aug 25, 2018
* AMJ96 Refactor

LAYOUT matrix and default keymap refactored to use full-length keycodes (instead of shorthand).

* Configurator update

Minor physical layout update and made the labels more sensible.

* Readme update

White space correction.

* Default keymap update: #include QMK_KEYBOARD_H

* rules.mk update

Disabled the following options as my local test compile's .hex ended up too big to fit in the available flash space:

MOUSEKEY_ENABLE
EXTRAKEY_ENABLE
BACKLIGHT_ENABLE
RGBLIGHT_ENABLE

If this is undesirable, suggestions are welcome.

* Config/rules update

Re-enabled MOUSEKEY_ENABLE, EXTRAKEY_ENABLE, BACKLIGHT_ENABLE, and RGBLIGHT_ENABLE and disabled COMMAND_ENABLE in rules.mk; enabled NO_ACTION_MACRO and NO_ACTION_FUNCTION per @drashna

* Swapped `___` for `XXX` in amj96.h per @mechmerlin
alexey-danilov pushed a commit to alexey-danilov/qmk_firmware that referenced this pull request Sep 6, 2018
* AMJ96 Refactor

LAYOUT matrix and default keymap refactored to use full-length keycodes (instead of shorthand).

* Configurator update

Minor physical layout update and made the labels more sensible.

* Readme update

White space correction.

* Default keymap update: #include QMK_KEYBOARD_H

* rules.mk update

Disabled the following options as my local test compile's .hex ended up too big to fit in the available flash space:

MOUSEKEY_ENABLE
EXTRAKEY_ENABLE
BACKLIGHT_ENABLE
RGBLIGHT_ENABLE

If this is undesirable, suggestions are welcome.

* Config/rules update

Re-enabled MOUSEKEY_ENABLE, EXTRAKEY_ENABLE, BACKLIGHT_ENABLE, and RGBLIGHT_ENABLE and disabled COMMAND_ENABLE in rules.mk; enabled NO_ACTION_MACRO and NO_ACTION_FUNCTION per @drashna

* Swapped `___` for `XXX` in amj96.h per @mechmerlin
ChrissiQ pushed a commit to ChrissiQ/qmk_firmware that referenced this pull request Sep 25, 2018
* AMJ96 Refactor

LAYOUT matrix and default keymap refactored to use full-length keycodes (instead of shorthand).

* Configurator update

Minor physical layout update and made the labels more sensible.

* Readme update

White space correction.

* Default keymap update: #include QMK_KEYBOARD_H

* rules.mk update

Disabled the following options as my local test compile's .hex ended up too big to fit in the available flash space:

MOUSEKEY_ENABLE
EXTRAKEY_ENABLE
BACKLIGHT_ENABLE
RGBLIGHT_ENABLE

If this is undesirable, suggestions are welcome.

* Config/rules update

Re-enabled MOUSEKEY_ENABLE, EXTRAKEY_ENABLE, BACKLIGHT_ENABLE, and RGBLIGHT_ENABLE and disabled COMMAND_ENABLE in rules.mk; enabled NO_ACTION_MACRO and NO_ACTION_FUNCTION per @drashna

* Swapped `___` for `XXX` in amj96.h per @mechmerlin
yamad pushed a commit to yamad/qmk_firmware that referenced this pull request Apr 10, 2019
* AMJ96 Refactor

LAYOUT matrix and default keymap refactored to use full-length keycodes (instead of shorthand).

* Configurator update

Minor physical layout update and made the labels more sensible.

* Readme update

White space correction.

* Default keymap update: #include QMK_KEYBOARD_H

* rules.mk update

Disabled the following options as my local test compile's .hex ended up too big to fit in the available flash space:

MOUSEKEY_ENABLE
EXTRAKEY_ENABLE
BACKLIGHT_ENABLE
RGBLIGHT_ENABLE

If this is undesirable, suggestions are welcome.

* Config/rules update

Re-enabled MOUSEKEY_ENABLE, EXTRAKEY_ENABLE, BACKLIGHT_ENABLE, and RGBLIGHT_ENABLE and disabled COMMAND_ENABLE in rules.mk; enabled NO_ACTION_MACRO and NO_ACTION_FUNCTION per @drashna

* Swapped `___` for `XXX` in amj96.h per @mechmerlin
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

3 participants