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

restructure converters #1825

Merged
merged 6 commits into from
Nov 8, 2017
Merged

Conversation

BalzGuenat
Copy link
Contributor

each converter is its own keyboard and different hardware variants are different subprojects.

remove (seemingly) old method of loading layouts from main Makefile

each converter is its own keyboard and different hardware variants are different subprojects.

remove (seemingly) old method of loading layouts from main Makefile
Makefile Outdated
# endif

# LAYOUT_KEYMAPS :=
# $$(foreach LAYOUT,$$(KEYBOARD_LAYOUTS),$$(eval LAYOUT_KEYMAPS += $$(notdir $$(patsubst %/.,%,$$(wildcard $(ROOT_DIR)/layouts/*/$$(LAYOUT)/*/.)))))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't seen, known or used this method of loading layouts. I compiled all the keyboards with this commented out and got no errors, so I assume this is old and unused.

The reason why I removed it is because for my USB-USB converter, I need to include a makefile from the keyboard-rules.mk. The path of this makefile uses $(TMK_DIR), which is apparently not defined when the keyboard-rules.mk is first read in the above code segment. The next time that file is read, $(TMK_DIR) has been defined by common.mk and the build runs through fine.

If the removal of this segment is a problem, let me know and we can try to find another solution.

Copy link
Member

Choose a reason for hiding this comment

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

This is new and being used - no errors were generated because commenting it out just ignore layouts/, effectively removing the feature ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

I have worked up a project for the KC60 SE. I think is made by the same company as KC60, NPKC. Do you have a good example of the new structure that you are implementing? My KC60SE project is at 'https://github.com/BlakeCLewis/kc60se'. I think that GH60, KC60, XD60 could all be subprojects. Though It loks like many of these keyboards have legacy code that do not use QMK features.
I had a much easier time getting QMK project to work on the keyboard than the web stuff pointed to by the seller. 

Copy link
Member

Choose a reason for hiding this comment

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

I'm still working on the documentation, but generally we've been trying to keep boards that have different manufacturers separate (see the recent ergodox split), even if they have the same pinout and everything. The layout stuff helps to let these keyboards still use the same keymaps though ;)

@jackhumbert
Copy link
Member

With #1784 (hopefully be able to merge it this week), we'll be able to structure projects in folders like converters without using the subproject stuff (which will be removed with it) - that would be preferred to splitting them up like this.

@jackhumbert
Copy link
Member

I think the restructuring is unneeded now, but it looks like there are some other changes here that would be nice to have - are you interested in rebasing things?

@BalzGuenat
Copy link
Contributor Author

Will look into it in the coming days.

@BlakeCLewis
Copy link
Contributor

BlakeCLewis commented Nov 2, 2017 via email

# Conflicts:
#	Makefile
#	keyboards/converter/usb_usb/README.md
#	keyboards/converter/usb_usb/rules.mk
Also attempt to get the BLE thing more properly integrated.
Also also fix led_set() to call led_set_kb().
@jackhumbert jackhumbert merged commit 3b5381d into qmk:master Nov 8, 2017
@jackhumbert
Copy link
Member

Awesome! Thanks :)

kgwong pushed a commit to kgwong/qmk_firmware that referenced this pull request Nov 26, 2017
* restructure converters

each converter is its own keyboard and different hardware variants are different subprojects.

remove (seemingly) old method of loading layouts from main Makefile

* call led_set_kb() from overridden led_set()

* put converter back into one folder

* revert some structure changes to bring in line with qmk#1784.

Also attempt to get the BLE thing more properly integrated.
Also also fix led_set() to call led_set_kb().
@BalzGuenat BalzGuenat deleted the converter-restructure branch December 15, 2017 21:37
akatrevorjay added a commit to akatrevorjay/qmk_firmware that referenced this pull request Apr 3, 2018
* 'master' of git://github.com/qmk/qmk_firmware: (291 commits)
  More "oscillope" keymap fixes. (qmk#1982)
  Improved README of yuuki and added RGB commands (qmk#1983)
  restructure converters (qmk#1825)
  qwerty_code_friendly: configurable left thumb
  Migrated most code from keymaps to userspace  (qmk#1980)
  Small ergodox config fix and update.
  Update and move around drashna keymaps (qmk#1976)
  make it easy to customize logo image
  add pgm_read_dword for Infinity ErgoDox
  333fred layout update (qmk#1971)
  Pete's 40th XD64 Layout
  Ergodox EZ and Atreus 42 key dvorak layout updates (qmk#1964)
  Correct the rules.mk documentation for auto shift.
  Fix RGBLIGHT startup color (qmk#1975)
  adds indication up to layer 7
  restore default mode/color if no 0 color
  adds per-layer rgb color option to ez
  update gh60 info
  add gh60 info.json
  several improvements for mitosis:datagrok (qmk#1960)
  ...
LovesTha pushed a commit to LovesTha/qmk_firmware that referenced this pull request Jul 24, 2018
* restructure converters

each converter is its own keyboard and different hardware variants are different subprojects.

remove (seemingly) old method of loading layouts from main Makefile

* call led_set_kb() from overridden led_set()

* put converter back into one folder

* revert some structure changes to bring in line with qmk#1784.

Also attempt to get the BLE thing more properly integrated.
Also also fix led_set() to call led_set_kb().
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.

3 participants