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

Disable NKRO on V-USB controllers #9054

Merged
merged 2 commits into from
Jul 22, 2020
Merged

Disable NKRO on V-USB controllers #9054

merged 2 commits into from
Jul 22, 2020

Conversation

drashna
Copy link
Member

@drashna drashna commented May 9, 2020

Description

NKRO isn't available on on V-USB. Instead of erroring out and failing to compile, just forcibly disable NKRO on V-USB controllers.

Types of Changes

  • Core
  • Enhancement/optimization

Checklist

  • My code follows the code style of this project.
  • 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 drashna added the core label May 9, 2020
@drashna drashna requested a review from a team May 9, 2020 04:53
@fauxpark
Copy link
Member

fauxpark commented May 9, 2020

I wouldn't say NKRO is not supported... it just hasn't been implemented yet. Mainly due to the endpoint mess, but given we can theoretically merge them together with SHARED_EP_ENABLE it shouldn't be too much of an issue.

fauxpark
fauxpark reviewed May 9, 2020
View reviewed changes
tmk_core/common.mk Outdated Show resolved Hide resolved
@stale
Copy link

stale bot commented Jun 23, 2020

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.

@Erovia Erovia requested a review from a team June 23, 2020 08:33
@stale stale bot removed the awaiting changes label Jun 23, 2020
@drashna drashna marked this pull request as ready for review July 2, 2020 02:57
@drashna drashna mentioned this pull request Jul 5, 2020
Closed
13 tasks
@drashna drashna changed the base branch from master to develop July 8, 2020 13:18
@tzarc
Copy link
Member

tzarc commented Jul 16, 2020

So.... what's the direction that we're planning to take? We're okay with disabling, with a note?

@drashna
Copy link
Member Author

drashna commented Jul 18, 2020

So.... what's the direction that we're planning to take? We're okay with disabling, with a note?

I think hard disabling it for now is the best option, until somebody wants to put in the work to add support for it (and more endpoints).

As for this PR vs #9498, I think this is a much cleaner implementation, as it's a much lighter touch (it doesn't moving things around anywhere else). However, I am biased, since this is my PR.

@tzarc tzarc merged commit 12c1c35 into qmk:develop Jul 22, 2020
noroadsleft pushed a commit that referenced this pull request Jul 26, 2020
@drashna @fauxpark @noroadsleft
Disable NKRO on V-USB controllers (#9054)
f06baa1 
* Disable NKRO on V-USB controllers

* not _currently_ supported text

Co-authored-by: Ryan <[email protected]>

Co-authored-by: Ryan <[email protected]>
@drashna drashna deleted the fix/nkro_on_vusb branch July 27, 2020 03:42
noroadsleft pushed a commit that referenced this pull request Jul 31, 2020
@drashna @fauxpark @noroadsleft
Disable NKRO on V-USB controllers (#9054)
47d0c01 
* Disable NKRO on V-USB controllers

* not _currently_ supported text

Co-authored-by: Ryan <[email protected]>

Co-authored-by: Ryan <[email protected]>
noroadsleft pushed a commit that referenced this pull request Aug 11, 2020
@drashna @fauxpark @noroadsleft
Disable NKRO on V-USB controllers (#9054)
de57f33 
* Disable NKRO on V-USB controllers

* not _currently_ supported text

Co-authored-by: Ryan <[email protected]>

Co-authored-by: Ryan <[email protected]>
noroadsleft pushed a commit that referenced this pull request Aug 27, 2020
@drashna @fauxpark @noroadsleft
Disable NKRO on V-USB controllers (#9054)
3f30480 
* Disable NKRO on V-USB controllers

* not _currently_ supported text

Co-authored-by: Ryan <[email protected]>

Co-authored-by: Ryan <[email protected]>
noroadsleft pushed a commit that referenced this pull request Aug 29, 2020
@drashna @fauxpark @noroadsleft
Disable NKRO on V-USB controllers (#9054)
3c74edb 
* Disable NKRO on V-USB controllers

* not _currently_ supported text

Co-authored-by: Ryan <[email protected]>

Co-authored-by: Ryan <[email protected]>
nicocesar pushed a commit to nicocesar/qmk_firmware that referenced this pull request Sep 6, 2020
@drashna @fauxpark @nicocesar
Disable NKRO on V-USB controllers (qmk#9054)
fb01c42 
* Disable NKRO on V-USB controllers

* not _currently_ supported text

Co-authored-by: Ryan <[email protected]>

Co-authored-by: Ryan <[email protected]>
drashna added a commit to zsa/qmk_firmware that referenced this pull request Sep 30, 2020
@drashna @fauxpark
Disable NKRO on V-USB controllers (qmk#9054)
92eeea4 
* Disable NKRO on V-USB controllers

* not _currently_ supported text

Co-authored-by: Ryan <[email protected]>

Co-authored-by: Ryan <[email protected]>
kjganz pushed a commit to kjganz/qmk_firmware that referenced this pull request Oct 28, 2020
@drashna @fauxpark @noroadsleft
Disable NKRO on V-USB controllers (qmk#9054)
335335f 
* Disable NKRO on V-USB controllers

* not _currently_ supported text

Co-authored-by: Ryan <[email protected]>

Co-authored-by: Ryan <[email protected]>
@stephen-huan stephen-huan mentioned this pull request Apr 5, 2022
Closed
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
@drashna @fauxpark @noroadsleft
Disable NKRO on V-USB controllers (qmk#9054)
fa0b11c 
* Disable NKRO on V-USB controllers

* not _currently_ supported text

Co-authored-by: Ryan <[email protected]>

Co-authored-by: Ryan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tzarc tzarc tzarc approved these changes

@fauxpark fauxpark fauxpark approved these changes

Assignees
No one assigned
Labels
awaiting review core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@drashna @fauxpark @tzarc @noroadsleft