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.

Sign up for GitHub

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

Jump to bottom

Add hand swap config matrix for preonic rev 3 #7460

Merged
merged 2 commits into from
Nov 25, 2019
Merged

Add hand swap config matrix for preonic rev 3 #7460

merged 2 commits into from
Nov 25, 2019

Conversation

martinkauppinen
Copy link
Contributor

Description

The Swap Hands feature was broken on Preonic rev. 3 due to a change between revisions of the number of matrix rows and columns, leading to build errors if the feature was wanted. This fix adds a check for rev. 3 with the proper matrix.

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

@drashna
Copy link
Member

drashna commented Nov 24, 2019

I like the update, but I think it may be best to move the hand_swap code from the preonic.c to the rev#.c files for each board.

@noroadsleft any thoughts?

@drashna drashna requested a review from a team November 24, 2019 00:52
@noroadsleft
Copy link
Member

I like the update, but I think it may be best to move the hand_swap code from the preonic.c to the rev#.c files for each board.

@noroadsleft any thoughts?

I agree.

@drashna
Copy link
Member

drashna commented Nov 24, 2019 •
edited
Loading

Just thinking about it, could handle this the same way that the music_map is. By using LAYOUT_preonic_grid, which handles all this mess for you.

Nope, that wouldn't work, due to how the matrix works...

@martinkauppinen
Copy link
Contributor Author

I like the update, but I think it may be best to move the hand_swap code from the preonic.c to the rev#.c files for each board.

Good call, makes a lot more sense than what I did. I blame being awake and coding at 1am. The code has been moved accordingly.

@noroadsleft noroadsleft requested a review from drashna November 24, 2019 21:50
@drashna
Copy link
Member

drashna commented Nov 25, 2019

Well, what you did made sense, and it's what was there for the planck and preonic, in the past. However, it led to complications, so I wanted to avoid that, and keep things consistent with how the layout macros are handled. That should make things a lot simpler, if there are bug fixes needed.

drashna
drashna approved these changes Nov 25, 2019
View reviewed changes
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!

@noroadsleft noroadsleft merged commit 572d332 into qmk:master Nov 25, 2019
@noroadsleft
Copy link
Member

Thanks!

ripxorip pushed a commit to ripxorip/qmk_firmware that referenced this pull request Dec 3, 2019
@martinkauppinen
[Keyboard] Add hand swap config matrix for preonic rev 3 (qmk#7460)
36737e1 
* Add hand swap config matrix for preonic rev 3

* Refactor hand swap config matrix into revision source files
patrl pushed a commit to patrl/qmk_firmware that referenced this pull request Dec 29, 2019
@martinkauppinen @patrl
[Keyboard] Add hand swap config matrix for preonic rev 3 (qmk#7460)
0857c5f 
* Add hand swap config matrix for preonic rev 3

* Refactor hand swap config matrix into revision source files
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Feb 21, 2020
@martinkauppinen @HokieGeek
[Keyboard] Add hand swap config matrix for preonic rev 3 (qmk#7460)
74c1a2a 
* Add hand swap config matrix for preonic rev 3

* Refactor hand swap config matrix into revision source files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@noroadsleft noroadsleft noroadsleft approved these changes

@drashna drashna drashna approved these changes

Assignees
No one assigned
Labels
keyboard
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@martinkauppinen @drashna @noroadsleft