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 Prime_L V2 and Prime_EXL Plus #8111

Merged
merged 19 commits into from
Feb 8, 2020
Merged

Add Prime_L V2 and Prime_EXL Plus #8111

merged 19 commits into from
Feb 8, 2020

Conversation

holtenc
Copy link
Contributor

@holtenc holtenc commented Feb 7, 2020

Description

Adding firmware for two new PCBs. Prime_L V2 will be produced. Prime_EXL Plus is a personal project, hence why it's in /handwired.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • [ x] 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.
  • 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).

@holtenc
Copy link
Contributor Author

holtenc commented Feb 7, 2020

not sure what the fail is all about. It builds just fine locally.

@fauxpark
Copy link
Member

fauxpark commented Feb 7, 2020

The keyboard name needs to be lowercase.

@fauxpark fauxpark requested a review from a team February 7, 2020 05:51
@holtenc
Copy link
Contributor Author

holtenc commented Feb 7, 2020

Decided to drop adding Prime_EXL Plus from QMK as it will never be used by anyone other than myself. Made all other changes requested under Prime_L V2. But never did find where you mentioned the keyboard name needs to be lower case...

@fauxpark
Copy link
Member

fauxpark commented Feb 7, 2020

I'm referring to the folder name:
image

@holtenc
Copy link
Contributor Author

holtenc commented Feb 7, 2020

folder name fixed. it was fixed locally a while ago which is why I wasn't seeing what you were talkinga bout. but I guess that change doesn't get pushed to github?? either way. it's done.

@drashna
Copy link
Member

drashna commented Feb 8, 2020

Yeah, the casing is ... troublesome, which is why we generally want all lower case. It can cause issues on some OS's... As you've demonstrated. :)

Copy link
Member

@drashna drashna left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Member

@fauxpark fauxpark left a comment

Choose a reason for hiding this comment

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

Thanks!

@fauxpark fauxpark merged commit 8fe29f2 into qmk:master Feb 8, 2020
Shinichi-Ohki added a commit to Shinichi-Ohki/qmk_firmware that referenced this pull request Feb 8, 2020
* 'master' of https://github.com/qmk/qmk_firmware: (24 commits)
  CI: Add workflow for CLI testing (qmk#7357)
  Add Prime_L V2 and Prime_EXL Plus (qmk#8111)
  [Keyboard] adding keyboard: neuron (qmk#7980)
  [Keyboard] adding keyboard: pain27 (qmk#7977)
  [Keyboard] adding keyboard: sl40 (qmk#7978)
  [Keyboard] adding keyboard: houndstooth (qmk#7981)
  [Keyboard] adding keyboard: kodachi50 (qmk#7976)
  [Keyboard] adding keyboard: gothic70 (qmk#7982)
  [Keyboard] adding keyboard: gothic50 (qmk#7983)
  [Keyboard] YMDK NP21: matrix and keymap refactor (qmk#8112)
  [Keymap] Added keymap for keebio/nyquist (qmk#8108)
  [Keyboard] Port SPLIT_USB_DETECT to lily58 (qmk#8107)
  [Keymap] Phoebe/Maxr1998 keymap: enable space cadet with curly braces (qmk#8096)
  [Keymap] add lily58 yshrsmz keymap (qmk#8095)
  [Keyboard] Remove i2c write command when reading columns on Ergodox EZ (qmk#8092)
  [Keyboard] Misc tidyups for Chidori (qmk#8091)
  [Keymap] Add users/alfrdmalr and switch to layouts (qmk#8030)
  XD84: Configurator bugfix for ISO layout (qmk#8117)
  ZJ68: complete Configurator layout support (qmk#8116)
  Add QMK Compile Context Sensitivity (qmk#6884)
  ...
@noroadsleft
Copy link
Member

So... this looks like an updated version of the Prime_L. Any reason why this wasn't added as a subfolder (keyboards/primekb/prime_l/v2/)?

@holtenc
Copy link
Contributor Author

holtenc commented Feb 8, 2020

@noroadsleft no reason other than it didn't occur to me and I'm not familiar with the process/file/folder structure. If you insist on it I'll play with it and submit a new PR

@noroadsleft
Copy link
Member

Let me work on it and I'll send you a commit or PR. Might be tomorrow though.

@holtenc
Copy link
Contributor Author

holtenc commented Feb 9, 2020

I have no problem figuring it out myself. as I noted in the qmk_configurator PR the two different PCB versions need a differing info.json. I'm just not familiar enough with how _configurator works with _firmware to know if it's possible to put an additional info.json in a subfolder like you can with rules.mk

@noroadsleft
Copy link
Member

This is what I get for not checking my GitHub notifications.

https://github.com/noroadsleft/qmk_firmware/tree/rf/prime_l/keyboards/primekb/prime_l

Anyway, each revision can have its own info.json file. The idea is to only duplicate the parts that are different between revisions, and have everything else in common between the revisions hosted at the keyboard level.

Use all of the changes, or none. I really only intended this to be a sample.

Important Notes:

noroadsleft:rf/prime_l, keyboards/primekb/prime_l/rules.mk#L35

... makes it so calling make primekb/prime_l calls make primekb/prime_l/v1, because moving the folders means the make command needed changes. I did not create a similar configuration for the V2, and in truth, many of us within QMK want to deprecate and later eliminate DEFAULT_FOLDER completely.

noroadsleft:rf/prime_l, keyboards/primekb/prime_l/v2/rules.mk#L1

This line isn't actually required (it's the same as in keyboards/primekb/prime_l/rules.mk), but the file itself is. QMK requires a rules.mk be present in order to treat the directory as that of a buildable keyboard.

keyboards/primekb/prime_l/keymaps/

This implementation only works when all revisions share a similar physical layout. As both revisions use a four/two/four configuration on the bottom row for the alpha area, the change in keysizes between revisions isn't relevant. Each revision uses the appropriate layout macro, which is set up between keyboards/primekb/prime_l/prime_l.h, keyboards/primekb/prime_l/v1/v1.h, and keyboards/primekb/prime_l/v2/v2.h.

@holtenc
Copy link
Contributor Author

holtenc commented Feb 17, 2020

@noroadsleft sorry it's taken me so long to respond. Thank so much for setting that up and explaining it. I've looked at examples from other keyboards that have multiple revisions (Iris etc) but it's a lot easier for me to follow what's going on when I'm already familiar with the keyboard firmware, such as this. It all looks fine to me. I haven't done a test build with these changes yet but should be able to get to that this week. I'll get it all tested and submit a new PR ASAP. Should I just close the _configurator PR?

@noroadsleft
Copy link
Member

@noroadsleft sorry it's taken me so long to respond. Thank so much for setting that up and explaining it. I've looked at examples from other keyboards that have multiple revisions (Iris etc) but it's a lot easier for me to follow what's going on when I'm already familiar with the keyboard firmware, such as this. It all looks fine to me. I haven't done a test build with these changes yet but should be able to get to that this week. I'll get it all tested and submit a new PR ASAP. Should I just close the _configurator PR?

Go ahead and leave the Configurator PR open; I can mark it on hold and later walk you through updating it.

HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Feb 21, 2020
* correct indicator light states.

function of indicator lights was inverted. these changes correct that.

* flesh out keymaps pre production

* Enable extrakey in rules

* Prime_BLE initial commit

* Initial commit for Prime_L V2

* Update info.json

correct key spacing.

* update copyright

* Update readme.md

* Inital commit

* updates before PR into QMK master

* Drop Prime_EXL Plus from PR. Make requested changes to Prime_L V2

* Rename keyboards/primekb/Prime_l_v2/config.h to keyboards/primekb/prime_l_v2/config.h

* Rename keyboards/primekb/prime_l_v2/config.h to keyboards/primekb/Prime_l_v2/config.h

* remove directory Prime_l_v2

* re-submit with proper folder name.
c0psrul3 pushed a commit to c0psrul3/qmk_firmware that referenced this pull request Mar 23, 2020
* correct indicator light states.

function of indicator lights was inverted. these changes correct that.

* flesh out keymaps pre production

* Enable extrakey in rules

* Prime_BLE initial commit

* Initial commit for Prime_L V2

* Update info.json

correct key spacing.

* update copyright

* Update readme.md

* Inital commit

* updates before PR into QMK master

* Drop Prime_EXL Plus from PR. Make requested changes to Prime_L V2

* Rename keyboards/primekb/Prime_l_v2/config.h to keyboards/primekb/prime_l_v2/config.h

* Rename keyboards/primekb/prime_l_v2/config.h to keyboards/primekb/Prime_l_v2/config.h

* remove directory Prime_l_v2

* re-submit with proper folder name.
kylekuj pushed a commit to kylekuj/qmk_firmware that referenced this pull request Apr 21, 2020
* correct indicator light states.

function of indicator lights was inverted. these changes correct that.

* flesh out keymaps pre production

* Enable extrakey in rules

* Prime_BLE initial commit

* Initial commit for Prime_L V2

* Update info.json

correct key spacing.

* update copyright

* Update readme.md

* Inital commit

* updates before PR into QMK master

* Drop Prime_EXL Plus from PR. Make requested changes to Prime_L V2

* Rename keyboards/primekb/Prime_l_v2/config.h to keyboards/primekb/prime_l_v2/config.h

* Rename keyboards/primekb/prime_l_v2/config.h to keyboards/primekb/Prime_l_v2/config.h

* remove directory Prime_l_v2

* re-submit with proper folder name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants