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

olligranlund keymap updates for DZ60 and Keebio/Iris #10760

Closed
wants to merge 6 commits into from

Conversation

OlliGranlund
Copy link
Contributor

I've reworked my keymaps for both Iris and DZ60.

DZ60

I created a new keymap for my second DZ60 board which has a different layout than my first one. Added VIA support.

Keebio/Iris

I created a new keymap for the rebuild of my Iris board. This layout differs to some extent from the original and tries to be more inline with any normal 60% ISO keyboard. Added VIA support.

Description

The changes done are only keymap changes to keymaps which I've uploaded earlier.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

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).

@zvecr
Copy link
Member

zvecr commented Oct 25, 2020

In its current form, this PR cannot be accepted due to the submodule changes in lib/vusb.

@OlliGranlund
Copy link
Contributor Author

I guess I had an too old version of qmk fork, merged now from upstream latest master

@zvecr
Copy link
Member

zvecr commented Oct 25, 2020

drawing

Still no good. You want it so the files changed tab cant see a change, then the bot will remove the 'dependencies' label.

@OlliGranlund
Copy link
Contributor Author

Updated all the submodules with this command:

git submodule foreach git pull origin master

@zvecr
Copy link
Member

zvecr commented Oct 25, 2020

Now you have lib/googletest and lib/lufa changes

@OlliGranlund
Copy link
Contributor Author

Apparently updating the submodules turned out to be trickier than I expected... Where do we have stated which versions of the submodules should be used? I redownloaded QMK and replaced the lib-folder with the latest generated by qmk, but still running into the same issue...

Any hints on how I should proceed?

@OlliGranlund
Copy link
Contributor Author

Got some help from QMK discord to cleanup the repo, no changes to git submodules anymore :)


/* Select hand configuration */

// #define MASTER_LEFT
#define MASTER_LEFT
Copy link
Member

Choose a reason for hiding this comment

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

Technically, this isn't required, as it's the default behavior.

@drashna drashna requested a review from a team November 15, 2020 19:33
Copy link
Member

@noroadsleft noroadsleft left a comment

Choose a reason for hiding this comment

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

Actually, you need this:

Comment on lines +19 to +22
enum layers (
_QWERTY,
_LOWER,
_RAISE
Copy link
Member

Choose a reason for hiding this comment

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

Fixing two typos; this keymap doesn't compile without these fixes.

Suggested change
enum layers (
_QWERTY,
_LOWER,
_RAISE
enum layers {
_QWERTY,
_LOWER,
_RAISE,

ADJUST,
};

#define KC_ KC_TRNS
Copy link
Member

Choose a reason for hiding this comment

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

This should be removed, as it doesn't appear to be used.

Suggested change
#define KC_ KC_TRNS

#include "config_common.h"

/* USB Device descriptor parameter */
#define VENDOR_ID 0x2602
Copy link
Member

Choose a reason for hiding this comment

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

This vendor ID is already in use.
https://www.the-sz.com/products/usbid/index.php?v=0x2602&p=&n=

❯ echo -n "Oliver Granlund" | shasum | cut -c1-4 | tr '[:lower:]' '[:upper:]'   
B9F7
Suggested change
#define VENDOR_ID 0x2602
#define VENDOR_ID 0xB9F7

Or if you want to use ascii

Suggested change
#define VENDOR_ID 0x2602
#define VENDOR_ID 0x4F47 // "OG"

Comment on lines +100 to +105
case QWERTY:
if (record->event.pressed) {
set_single_persistent_default_layer(_QWERTY);
}
return false;
break;
Copy link
Member

Choose a reason for hiding this comment

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

This isn't needed, since you only have the one default layer.

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.

None yet

4 participants