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

Remove custom matrix from PS2AVRGB boards #7396

Merged
merged 5 commits into from
Jan 2, 2020

Conversation

fauxpark
Copy link
Member

Description

All of this matrix code is unnecessary, as PS2AVRGB has a single common matrix pinout, and the core matrix.c can handle col2row.

Types of Changes

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

@fauxpark
Copy link
Member Author

All but one error seemed to be unrelated.

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.

LGTM.

My keymaps are erroring out due to the order that the mk files are being processed.... But that's 100% outside of the scope of this PR.

@yanfali
Copy link
Contributor

yanfali commented Nov 18, 2019

@coseyfannitutti can you verify the changes on your boards please?

@fauxpark
Copy link
Member Author

fauxpark commented Dec 11, 2019

@sidcarter DIODE_DIRECTION was missing from the Finger65 config.h, can you confirm it's COL2ROW? (and also how the heck it apparently worked without defining it) Seems to be the case for the rest of them but just to be sure.

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.

Looks good to me.

@sidcarter
Copy link
Contributor

@sidcarter DIODE_DIRECTION was missing from the Finger65 config.h, can you confirm it's COL2ROW? (and also how the heck it apparently worked without defining it) Seems to be the case for the rest of them but just to be sure.

whooops, I don't have that keeb no moah. I can ask the one who has the keeb to try it out, but I don't think he's well versed with the QMK code. Suggestions to proceed?

@fauxpark
Copy link
Member Author

Just need to know which way the diodes are pointing. Assuming this is the same PCB:

that looks like COL2ROW. The stripe on the diodes are pointed towards the switches, with the other end connected to vertical traces.

@sidcarter
Copy link
Contributor

@fauxpark https://txkeyboards.com/images/virtuemart/product/65per.jpg

Unfortunately, I don't have a more detailed pic. It looks as though as it's facing away from the switch?

I could try to get a new pic from the new owner, if that doesn't work.

@fauxpark
Copy link
Member Author

I'm fairly confident now - the matrix.c from here is switching row pins and reading column pins, which is what the QMK matrix.c does when configured for COL2ROW. It looks like all the L3 PCBs have the same pinout between them, much like PS2AVRGB. (Finger65 config.h is missing the C2 pin for row 0, probably since it has no F keys)

@fauxpark fauxpark merged commit 2557bc8 into qmk:master Jan 2, 2020
@fauxpark fauxpark deleted the ps2avrgb-no-custom-matrix branch January 2, 2020 06:45
HokieGeek pushed a commit to HokieGeek/qmk_firmware that referenced this pull request Feb 21, 2020
* Remove custom matrix from PS2AVRGB boards

* Add custom backlight.c to SRC for bminiex, for now

* Add missing DIODE_DIRECTIONs
kylekuj pushed a commit to kylekuj/qmk_firmware that referenced this pull request Apr 21, 2020
* Remove custom matrix from PS2AVRGB boards

* Add custom backlight.c to SRC for bminiex, for now

* Add missing DIODE_DIRECTIONs
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.

5 participants