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

[Keyboard] Add nuphy air75 v2 keyboard #22751

Open
wants to merge 36 commits into
base: master
Choose a base branch
from

Conversation

nuphy-src
Copy link

@nuphy-src nuphy-src commented Dec 25, 2023

Description

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

@github-actions github-actions bot added keyboard keymap via Adds via keymap and/or updates keyboard for via support labels Dec 25, 2023
Copy link
Contributor

@dunk2k dunk2k left a comment

Choose a reason for hiding this comment

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

Add Community Layout support

keyboards/nuphy/air75_v2/ansi/info.json Outdated Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/keymaps/default/keymap.c Outdated Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/keymaps/via/keymap.c Outdated Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/ansi.c Outdated Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/ansi.c Outdated Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/ansi.c Outdated Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/ansi.c Outdated Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/ansi.c Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/side_driver.c Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/config.h Outdated Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/info.json Outdated Show resolved Hide resolved
@rdhar
Copy link

rdhar commented Jan 2, 2024

Hi @nuphy-src, if you wouldn't mind, could you please resolve/comment on @adophoxia, @dunk2k and @drashna's (code) suggestions so it's easier to follow progress on this PR?

@nuphy-src
Copy link
Author

Hello everyone, @dunk2k @rdhar @drashna @adophoxia ,I have adjusted the code according to your suggestions, please kindly approve the modified content, thank you

@nuphy-src
Copy link
Author

Hi @nuphy-src, if you wouldn't mind, could you please resolve/comment on @adophoxia, @dunk2k and @drashna's (code) suggestions so it's easier to follow progress on this PR?

Hi rdhar, I have adjusted the code according to your suggestions. please kindly approve the modified content, thank you

@jincao1
Copy link

jincao1 commented Jan 7, 2024

@nuphy-src In your latest commit the key codes for KC_LGUI, KC_LALT and KC_RGUI are in the wrong place in the windows layer. It was previously correct but looks like you made them match the Mac layer. The Mac layer is corrected with the wrong key placements before but in the process you broke the Windows layer. FYI.

@nuphy-src
Copy link
Author

@nuphy-src In your latest commit the key codes for KC_LGUI, KC_LALT and KC_RGUI are in the wrong place in the windows layer. It was previously correct but looks like you made them match the Mac layer. The Mac layer is corrected with the wrong key placements before but in the process you broke the Windows layer. FYI.

Thanks for your reminding, I will correct this problem immediately

@DP19
Copy link

DP19 commented Jan 7, 2024

@nuphy-src FYI a few others and I have built and run this version of the firmware and the media controls don't work in wireless modes in its current form.

We've made various changes to get it working but none of us are super well versed in C /qmk so there may be better ways to fix it

@nuphy-src
Copy link
Author

@nuphy-src FYI a few others and I have built and run this version of the firmware and the media controls don't work in wireless modes in its current form.

We've made various changes to get it working but none of us are super well versed in C /qmk so there may be better ways to fix it

May I ask which key does not function? KC_MCTL?

@DP19
Copy link

DP19 commented Jan 7, 2024

@nuphy-src FYI a few others and I have built and run this version of the firmware and the media controls don't work in wireless modes in its current form.

We've made various changes to get it working but none of us are super well versed in C /qmk so there may be better ways to fix it

May I ask which key does not function? KC_MCTL?

IIRC it's everything in the function row that begin with KC_

At least on MAC. @jincao1 can probably speak to Windows

@nuphy-src
Copy link
Author

@nuphy-src FYI a few others and I have built and run this version of the firmware and the media controls don't work in wireless modes in its current form.

We've made various changes to get it working but none of us are super well versed in C /qmk so there may be better ways to fix it

May I ask which key does not function? KC_MCTL?

IIRC it's everything in the function row that begin with KC_

At least on MAC. @jincao1 can probably speak to Windows

About the problem: The multimedia button does not function in mac system. I'll test the side again later. Thank you

@rdhar
Copy link

rdhar commented Jan 7, 2024

As @DP19 mentioned, here's collection of ongoing QMK firmware forks for Nuphy keyboards maintained by @zhogov.

It might be worth accepting input at this early stage from positively-active contributors like @jincao1 and @edykim. Really embracing the open-source culture to share the best of community development.

These contributors and more are already present on your Discord's Custom firmware on Air75v2 channel, if you'd like to continue the conversation there.

@nuphy-src
Copy link
Author

As @DP19 mentioned, here's collection of ongoing QMK firmware forks for Nuphy keyboards maintained by @zhogov.

It might be worth accepting input at this early stage from positively-active contributors like @jincao1 and @edykim. Really embracing the open-source culture to share the best of community development.

These contributors and more are already present on your Discord's Custom firmware on Air75v2 channel, if you'd like to continue the conversation there.

Yes, I am also happy to share my code here, and welcome your suggestions, Thanks

@jincao1
Copy link

jincao1 commented Jan 7, 2024

I believe the issue with the F modifiers are that none of them work in RF because the current implementation with core QMK does not send the system and consumer reports when in RF mode.

I wrote a simple RF driver to handle that within the QMK lifecycle. Keep in mind I'm not a C developer.

Feel free to look at these commits I made in the diff below relating to the RF driver and attempts at addressing keys that may get stuck/lost in RF mode.

jincao1/qmk_firmware@2553cb6...faff038

You're welcome to build my branch to test and to cherry pick my changes in. I won't be submitting any PRs as the commits contain customization's I want which your users may not want.

@nuphy-src
Copy link
Author

I believe the issue with the F modifiers are that none of them work in RF because the current implementation with core QMK does not send the system and consumer reports when in RF mode.

I wrote a simple RF driver to handle that within the QMK lifecycle. Keep in mind I'm not a C developer.

Feel free to look at these commits I made in the diff below relating to the RF driver and attempts at addressing keys that may get stuck/lost in RF mode.

jincao1/qmk_firmware@2553cb6...faff038

You're welcome to build my branch to test and to cherry pick my changes in. I won't be submitting any PRs as the commits contain customization's I want which your users may not want.

Hi Thank you very much for your suggestions and code, your code is of great help to me, I will refer to it to modify my code.

@nuphy-src
Copy link
Author

Thank you very much for your suggestion @DP19 @jincao1 @rdhar ,and I have modified my code with reference to your code, @jincao1 which is very helpful for me to solve this problem. Thanks

update

update

update

reset submodule
keyboards/nuphy/air75_v2/ansi/readme.md Outdated Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/info.json Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Oct 8, 2024

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 bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

@github-actions github-actions bot added the stale Issues or pull requests that have become inactive without resolution. label Oct 8, 2024
@jwoodard80
Copy link

Can someone please look at this rather than let it go stale? I've been reading over this request and it seems like @nuphy-src has tried in vain for almost a year to get this committed. Nuphy could just as easily continue to maintain their own code repos but they have attempted to add their code which we all appreciate.

If there is something outstanding for them to resolve please let them know, otherwise I would be interested to know what the delay is.

Thank you to everyone.

@DP19 @adophoxia @rdhar @ar53n @drashna @dunk2k

@github-actions github-actions bot removed the stale Issues or pull requests that have become inactive without resolution. label Oct 9, 2024
@tzarc
Copy link
Member

tzarc commented Oct 10, 2024

Can someone please look at this rather than let it go stale? I've been reading over this request and it seems like @nuphy-src has tried in vain for almost a year to get this committed. Nuphy could just as easily continue to maintain their own code repos but they have attempted to add their code which we all appreciate.

If there is something outstanding for them to resolve please let them know, otherwise I would be interested to know what the delay is.

This is one of those PRs that has enough wrong with it that reviewers are almost nonexistent -- the PR checklist published by QMK outlines a significant number of things that need addressing before anything would normally proceed, and reviewers generally leave PRs alone if things in the checklist aren't handled. Stuff like introducing new lighting systems and duplicating QMK code effectively render this PR as "unmergeable" in its current form, yet you've seemingly got the expectation that we should ignore our requirements just for your favourite board.

The PR checklist is there so we, as maintainers, don't need to continually outline the same problems people keep making in minute detail. It's not up to us to inform PR submitters which clauses they don't comply with.

Additionally there's discussions going on with other manufacturers regarding how to integrate wireless -- doing things as they're done in this PR has no future within QMK. Manufacturers have been told to maintain their own forks in the meantime.

@titusjaka
Copy link

Hi @nuphy-src,
Thank you for your efforts on this PR 🙏

Could you please provide update for this PR? Are you planning to continue working on it?
Thank you!

@jwoodard80
Copy link

Can someone please look at this rather than let it go stale? I've been reading over this request and it seems like @nuphy-src has tried in vain for almost a year to get this committed. Nuphy could just as easily continue to maintain their own code repos but they have attempted to add their code which we all appreciate.
If there is something outstanding for them to resolve please let them know, otherwise I would be interested to know what the delay is.

This is one of those PRs that has enough wrong with it that reviewers are almost nonexistent -- the PR checklist published by QMK outlines a significant number of things that need addressing before anything would normally proceed, and reviewers generally leave PRs alone if things in the checklist aren't handled. Stuff like introducing new lighting systems and duplicating QMK code effectively render this PR as "unmergeable" in its current form, yet you've seemingly got the expectation that we should ignore our requirements just for your favourite board.

The PR checklist is there so we, as maintainers, don't need to continually outline the same problems people keep making in minute detail. It's not up to us to inform PR submitters which clauses they don't comply with.

Additionally there's discussions going on with other manufacturers regarding how to integrate wireless -- doing things as they're done in this PR has no future within QMK. Manufacturers have been told to maintain their own forks in the meantime.

You seem a bit pointed in your comment. I honestly hold no expectations at all. However, as someone reading this from the outside (and judging by the likes, I don't seem to be alone) it very much appears as though the submitter has asked for feedback multiple times in the PR and at least attempted to resolve the issues. By no means does that guarantee his code to be mergeable. Requirements are there to maintain the standard for everyone and I wholeheartedly support them.

I will be the first to admit I am a novice. That said, it just seems that the same time you gave to me (which I genuinely appreciate) to explain why it is stalled might have been better served earlier in the process.

I very much appreciate the project and thank you all for your clarification in this. :-)

@nuphy-src
Copy link
Author

Can someone please look at this rather than let it go stale? I've been reading over this request and it seems like @nuphy-src has tried in vain for almost a year to get this committed. Nuphy could just as easily continue to maintain their own code repos but they have attempted to add their code which we all appreciate.
If there is something outstanding for them to resolve please let them know, otherwise I would be interested to know what the delay is.

This is one of those PRs that has enough wrong with it that reviewers are almost nonexistent -- the PR checklist published by QMK outlines a significant number of things that need addressing before anything would normally proceed, and reviewers generally leave PRs alone if things in the checklist aren't handled. Stuff like introducing new lighting systems and duplicating QMK code effectively render this PR as "unmergeable" in its current form, yet you've seemingly got the expectation that we should ignore our requirements just for your favourite board.

The PR checklist is there so we, as maintainers, don't need to continually outline the same problems people keep making in minute detail. It's not up to us to inform PR submitters which clauses they don't comply with.

Additionally there's discussions going on with other manufacturers regarding how to integrate wireless -- doing things as they're done in this PR has no future within QMK. Manufacturers have been told to maintain their own forks in the meantime.

Hi Nick,

Thank you for your suggestions. As a participant in QMK, Nuphy has put a lot of effort into this, and we hope that our code can be officially recognized by QMK. We have been working towards this goal, continuously optimizing our code based on your recommendations over the past year. As of now, the issues raised in this PR have all been resolved. The wireless functionality is a unique feature of Nuphy keyboards and a key reason many customers choose us. We hope that more QMK enthusiasts can see our complete code. We are also pleased to hear that QMK will be integrating wireless communication code, and we are very willing to assist QMK in completing this work. Thank you!

@nuphy-src
Copy link
Author

Hi @nuphy-src, Thank you for your efforts on this PR 🙏

Could you please provide update for this PR? Are you planning to continue working on it? Thank you!

Hello Denis, We have been paying attention to this PR, and have made a lot of modifications, but now we do not know what to do, do you have any suggestions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting changes in progress keyboard keymap pr_checklist_pending Needs changes as per the PR checklist via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.