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

add tada68 default ANSI layout #143

Merged
merged 1 commit into from
Jun 19, 2018
Merged

add tada68 default ANSI layout #143

merged 1 commit into from
Jun 19, 2018

Conversation

shalzz
Copy link
Contributor

@shalzz shalzz commented Jun 11, 2018

Default layout same as https://github.com/qmk/qmk_firmware/tree/master/keyboards/tada68/keymaps/default

Though the mouse keycodes do not render properly on the editor.
And I'm getting some compile errors after changing the Keymap name due to "Keymap name collision!"

@mechmerlin
Copy link
Contributor

The TADA68 is not fully supported on the Configurator. You need a bin file in order to properly flash the TADA68. The configurator currently does not let you do this. All the defaults will also be automatically generated in the near future. @yanfali the .json looks correct, but I don't think it will do anything useful. I suppose people could make the keymap, compile, and download source, and then compile the bin file?

@shalzz
Copy link
Contributor Author

shalzz commented Jun 11, 2018

@mechmerlin Yes this is mainly so that people can at least configure their choice of layout with an editor instead of doing it by hand and then either export the json or just download the sources (or the hex for those who have flashed the dfu bootloader).
Especially since the "official" TADA68 layout editor site at http://123.57.250.164:3000/tada68 is no longer working/intermittent.

@yanfali
Copy link
Collaborator

yanfali commented Jun 11, 2018

@shalzz this looks great, but can we fix the compile issues?

@shalzz
Copy link
Contributor Author

shalzz commented Jun 11, 2018

@yanfali This is the compile errors I get:

�
 Some git sub-modules are out of date or modified, please consider runnning:�
 make git-submodule
 You can ignore this warning if you are not compiling any ChibiOS keyboards,
 or if you have modified the ChibiOS libraries yourself. 

�

avr-gcc (GCC) 4.9.2
Copyright (C) 2014 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Compiling: keyboards/tada68/tada68.c                                                                �
Compiling: keyboards/tada68/keymaps/default1/keymap.c                                              In file included from ��[K:
��[K’ undeclared here (not in a function)
  O(_FL), KC_RCTL, KC_LEFT, KC_DOWN, KC_RGHT),
��[K
��[K’
  {k40, k41, k42, XXX, XXX, k45, XXX, XXX, XXX, k49, k4a, k4b, k4c, k4d, k4e}  \
��[K
��[K’
  O(_FL), KC_RCTL, KC_LEFT, KC_DOWN, KC_RGHT),
��[K
 �
 | 
 | 
 | 
tmk_core/rules.mk:357: recipe for target '.build/obj_tada68_default1/keyboards/tada68/keymaps/default1/keymap.o' failed
makeap.o] Error 1
Makefile:534: recipe for target 'tada68:default1' failed
make: *** [tada68:default1] Error 1
�ake finished with errors
�

Which, as you can imagine, is difficult for me to decipher. I dunno if its because of my system font or the websites.
It seems it is an issue with the compile server and not directly with the json. Maybe perhaps the MOUSEKEY_ENABLE option is not enabled?

@yanfali
Copy link
Collaborator

yanfali commented Jun 11, 2018 via email

@yanfali
Copy link
Collaborator

yanfali commented Jun 11, 2018

OK, looks like there's an error in the info.json "Error: tada68: KEYMAP_ISO: Number of elements in info.json does not match! info.json:69 != KEYMAP_ISO:68" I'm not sure what the backend does if only 1 layout is wrong. That may explain the name collision error, which I've never seen before. @skullydazed any feedback?

@shalzz
Copy link
Contributor Author

shalzz commented Jun 12, 2018

How exactly is the server setup and is the source available? If it is maybe I could look into it.

@yanfali
Copy link
Collaborator

yanfali commented Jun 12, 2018

@noroadsleft
Copy link
Member

I just saw this.

JSON file looks good, but the error @yanfali mentioned here was fixed on qmk_firmware in #3178.

Regarding the compile error, it looks like the generated keymap.c file is invalid somehow, but I can't tell for sure. I'm not that knowledgeable about the API under-the-hood.

{
"keyboard": "tada68",
"keymap": "default",
"layout": "KEYMAP_ANSI",
Copy link
Member

Choose a reason for hiding this comment

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

Change KEYMAP_ANSI to LAYOUT_ansi here.

The name of the key matrix was changed after you submitted this PR.

"KC_TAB","KC_Q","KC_W","KC_E","KC_R","KC_T","KC_Y","KC_U","KC_I","KC_O","KC_P","KC_LBRC","KC_RBRC","KC_BSLS","KC_DEL",
"KC_CAPS","KC_A","KC_S","KC_D","KC_F","KC_G","KC_H","KC_J","KC_K","KC_L","KC_SCLN","KC_QUOT","KC_ENT","KC_PGUP",
"KC_LSFT","KC_Z","KC_X","KC_C","KC_V","KC_B","KC_N","KC_M","KC_COMM","KC_DOT","KC_SLSH","KC_RSFT","KC_UP","KC_PGDN",
"KC_LCTL","KC_LGUI","KC_LALT","KC_SPC","KC_RALT","MO(_FL)","KC_RCTL","KC_LEFT","KC_DOWN","KC_RGHT"],
Copy link
Collaborator

Choose a reason for hiding this comment

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

@shalzz please change MO(_FL) to MO(1) - we only support integer layer changes at this time. That should fix the compile

@yanfali
Copy link
Collaborator

yanfali commented Jun 19, 2018

LGTM. Thank you @shalzz

@yanfali yanfali merged commit 8a01a6f into qmk:master Jun 19, 2018
@shalzz
Copy link
Contributor Author

shalzz commented Jun 19, 2018

@noroadsleft @yanfali Thanks for your work and review. I have made the relevant changes and it does compile successfully now.

There is still the name collision error because of the already existing "default" keymap in qmk_firmware. Maybe add a prefix like "config-" for all keymap names used with qmk_configurator?

@yanfali
Copy link
Collaborator

yanfali commented Jun 19, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants