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

Consistency check of MATRIX size against the number of defined pins #12151

Closed
wants to merge 1 commit into from

Conversation

elfmimi
Copy link
Contributor

@elfmimi elfmimi commented Mar 7, 2021

Description

This is the refined version of the consistency check patch which was included in #8160 .

It properly handles those keyboards doing things a bit off track.

  • keyboards/ai03/voyager60_alps

    MATRIX_COLS and MATRIX_COL_PINS contradicts. but it will be handled safely.

  • keyboards/choco60/rev2

    number of COL pins differ between left side and right side. but it will be handled safely.

Following is the only one which needs treatment.

  • keyboards/for_science

    need SPLIT_KEYBOARD = yes

I've checked this is not increasing binary size for any of the keyboards except choco60/rev2 .

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

@stale
Copy link

stale bot commented Jun 2, 2021

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@@ -29,10 +29,37 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
#define ROWS_PER_HAND (MATRIX_ROWS / 2)

#ifdef DIRECT_PINS
# ifdef DIRECT_PINS_RIGHT
static pin_t direct_pins[MATRIX_ROWS][MATRIX_COLS] = DIRECT_PINS;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this also be a const?

@stale stale bot removed the awaiting changes label Jun 7, 2021
@stale
Copy link

stale bot commented Jul 22, 2021

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@drashna
Copy link
Member

drashna commented Jul 23, 2021

Could you retarget this to develop, and rebase?

There are some changes in develop that will have conflicts with this change, though.

@drashna drashna self-requested a review July 23, 2021 02:08
@stale stale bot removed the awaiting changes label Jul 23, 2021
@stale
Copy link

stale bot commented Sep 7, 2021

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@stale
Copy link

stale bot commented Oct 11, 2021

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.

@stale stale bot closed this Oct 11, 2021
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.

2 participants