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 #21949

Closed
wants to merge 10 commits into from
Closed

[Keyboard] Add NuPhy Air75 V2 keyboard #21949

wants to merge 10 commits into from

Conversation

Persama
Copy link

@Persama Persama commented Sep 8, 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 Sep 8, 2023
@Persama Persama changed the title [Keyboard] Add NuPhy Air75 keyboard [Keyboard] Add NuPhy Air75 V2 keyboard Sep 8, 2023
keyboards/nuphy/air75_v2/ansi/ansi.c Outdated Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/info.json Outdated Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/info.json Outdated Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/readme.md Outdated Show resolved Hide resolved
Persama and others added 4 commits September 18, 2023 09:37
Co-authored-by: jack <0x6a73@protonmail.com>
Co-authored-by: jack <0x6a73@protonmail.com>
Co-authored-by: jack <0x6a73@protonmail.com>
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.

If you could run a format apps on the keymap.c files, and on the ansi.c/ansi.h files? (code formatting and tab->space)

keyboards/nuphy/air75_v2/ansi/ansi.c Outdated Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/info.json Show resolved Hide resolved
keyboards/nuphy/air75_v2/ansi/readme.md Outdated Show resolved Hide resolved
Co-authored-by: Drashna Jaelre <drashna@live.com>
Co-authored-by: Drashna Jaelre <drashna@live.com>
@RealGeo113
Copy link

@Persama Just wanted you to know that after building firmware from Persama:nuphy and flashing the keyboard sidelights are unusable.

@adophoxia
Copy link
Contributor

You must stop closing/opening new PRs for this same keyboard. It fragments the review process, also making it take longer.
Review suggestions and any other updates should just be pushed to the same branch.

People have been warned before that opening and closing new PRs for new changes will get them banned from the repo. If you want to have this PR, specifically #21949, to be merged, I suggest you stick to it, and not open any more PRs. Better to not fragment the review process.

@Persama
Copy link
Author

Persama commented Oct 15, 2023

@adophoxia Okay, I will continue to use the current PR #21949

@Persama Persama reopened this Oct 15, 2023
@Persama
Copy link
Author

Persama commented Oct 16, 2023

@Persama Just wanted you to know that after building firmware from Persama:nuphy and flashing the keyboard sidelights are unusable.

@RealGeo113 Thank you for your reminder. We are aware of this issue as it is how we designed/planned it.
The submitted code only needs to include basic functions, making it convenient for users to conduct secondary development on this basis, without being affected by the logic of the original private code. At the same time, we will also prepare a complete set of mass-produced firmware hex, which includes all the functions of the keyboard, making it convenient for users to restore the factory's functions;

@max3-2
Copy link

max3-2 commented Oct 16, 2023

@Persama Just wanted you to know that after building firmware from Persama:nuphy and flashing the keyboard sidelights are unusable.

@RealGeo113 Thank you for your reminder. We are aware of this issue as it is how we designed/planned it. The submitted code only needs to include basic functions, making it convenient for users to conduct secondary development on this basis, without being affected by the logic of the original private code. At the same time, we will also prepare a complete set of mass-produced firmware hex, which includes all the functions of the keyboard, making it convenient for users to restore the factory's functions;

Can you then supply documentation on how to address the additional features like the sidelights, e.g. pinout and control logic? Else this will be very hard to use

@RealGeo113
Copy link

@Persama Just wanted you to know that after building firmware from Persama:nuphy and flashing the keyboard sidelights are unusable.

@RealGeo113 Thank you for your reminder. We are aware of this issue as it is how we designed/planned it. The submitted code only needs to include basic functions, making it convenient for users to conduct secondary development on this basis, without being affected by the logic of the original private code. At the same time, we will also prepare a complete set of mass-produced firmware hex, which includes all the functions of the keyboard, making it convenient for users to restore the factory's functions;

Will the source code that is going to be used to build the "mass-produced-firmware" available somewhere then?

@Persama Persama requested a review from drashna October 19, 2023 15:25
waffle87
waffle87 previously approved these changes Oct 23, 2023
@lokher
Copy link
Contributor

lokher commented Oct 24, 2023

This keyboard is tri-mode keyboard NuPhy Air7, but I don't see any bluetooth/2.4g related code in the PR.
Maybe I am wrong, but it seems like that they committed only the USB code to meet the requicment for VIA json to be merged.

@Persama
Copy link
Author

Persama commented Oct 24, 2023

This keyboard is tri-mode keyboard NuPhy Air7, but I don't see any bluetooth/2.4g related code in the PR. Maybe I am wrong, but it seems like that they committed only the USB code to meet the requicment for VIA json to be merged.

@lokher Indeed, as you mentioned, this submission did not include any code for wireless functionality. We are concerned that making arbitrary modifications after opening it up may cause fatal issues with wireless functionality. Therefore, we will provide a wireless programming guide document on Nuphy's official website for those interested in making modifications. Partners who only want to use wireless functions can simply use the VIA settings keys.

@lokher
Copy link
Contributor

lokher commented Oct 25, 2023

This keyboard is tri-mode keyboard NuPhy Air7, but I don't see any bluetooth/2.4g related code in the PR. Maybe I am wrong, but it seems like that they committed only the USB code to meet the requicment for VIA json to be merged.

@lokher Indeed, as you mentioned, this submission did not include any code for wireless functionality. We are concerned that making arbitrary modifications after opening it up may cause fatal issues with wireless functionality. Therefore, we will provide a wireless programming guide document on Nuphy's official website for those interested in making modifications. Partners who only want to use wireless functions can simply use the VIA settings keys.

If you choose QMK, you need/have to open full functionality source code because it use GPL license, this is common sense, the fatal issues with wireless functionality is another topic.

naitoshedo added a commit to SRGBmods/QMK-Binaries that referenced this pull request Oct 29, 2023
Add support for nuphy_air75_vs (this does break all wireless modes as NuPhy did not provide source code support for those modes... USB connected only!) qmk/qmk_firmware#21949
@MarioRicalde
Copy link

This whole situation disappoints me greatly. It is so bad. The products that have been released under "V2" by NuPhy feel like a scam once you discover the fact that you can't compile the firmware without letting go of core functionality (wireless).

The submitted code only needs to include basic functions, making it convenient for users to conduct secondary development on this basis, without being affected by the logic of the original private code. At the same time, we will also prepare a complete set of mass-produced firmware hex, which includes all the functions of the keyboard, making it convenient for users to restore the factory's functions;

I've been gawking at the keyboards from NuPhy for a while, they look nice, but I was turned off by the firmware. Saw the recent launch of V2 and was excited with the front and center placement of QMK/VIA. Sent messages to friends and colleagues with excitement. I should've waited.

This is a deal breaker and from the sounds it against the license itself. Had I known this from the get-go I would've labeled this as a scam.

Please fix this situation immediately so that the advertisement is not as misleading as it currently is.

image

image

image

image

@adophoxia
Copy link
Contributor

This whole situation disappoints me greatly. It is so bad. The products that have been released under "V2" by NuPhy feel like a scam once you discover the fact that you can't compile the firmware without letting go of core functionality (wireless).

QMK at its core isn't really suited for wireless as whole since its support for BT is terrible. Considering that none of the code in this PR has anything to do with BT as a whole should've been obvious.

The submitted code only needs to include basic functions, making it convenient for users to conduct secondary development on this basis, without being affected by the logic of the original private code. At the same time, we will also prepare a complete set of mass-produced firmware hex, which includes all the functions of the keyboard, making it convenient for users to restore the factory's functions;

This is a deal breaker and from the sounds it against the license itself. Had I known this from the get-go I would've labeled this as a scam.

GPL, in the case of QMK, states that if a manufacturer/vendor releases the binaries for the firmware of their board, then they are obliged to provide the source code for the firmware of said boards.

If releasing the source code for the firmware for this board (albeit the lack of BT functionality) is somehow "against the license itself", then by that logic, you could say the same for every board in this repo whose source code is released and you may as well say that they're against the license. Ignoring such abstracted logic, they're already abiding the license, which is already good enough.

Please fix this situation immediately so that the advertisement is not as misleading as it currently is.

What can they do? Release an r1 of this board, and then decide in the r2 variant, slap on a cheap 8051, ESP32 or nrf MCU to make BT/2.4GHz possible, making this PR redundant once Nuphy stops making variants of these boards that use an STM32 MCU?

@MarioRicalde
Copy link

MarioRicalde commented Nov 2, 2023

I am going to limit myself on commenting on the other points since I was quoting the person above me.

What can they do?

Not slap QMK front and center on the product given potential buyers the perception that they will get AT LEAST a similar experience to that of, let's say Keychron, which is not perfect and as it lives in its own branch somewhere in their fork.

In my humble opinion it's quite misleading. It should mention on the site that you won't be able to use QMK without using VIA. Or you will just have to stick to wired.

Not all consumers will take the time to look for this particular PR, what you point out as obvious is not as obvious as it should be for consumers. QMK is known enough where it influences sales. And I am not having any of that.

@narze
Copy link
Contributor

narze commented Nov 2, 2023

I almost pre-ordered Air60v2, luckily I changed my mind.
Maybe another way out is to make a fork [like Keychron did]?(https://github.com/Keychron/qmk_firmware/tree/bluetooth_playground)

@Azarien
Copy link

Azarien commented Nov 2, 2023

What can they do? Release an r1 of this board, and then decide in the r2 variant, slap on a cheap 8051, ESP32 or nrf MCU to make BT/2.4GHz possible, making this PR redundant once Nuphy stops making variants of these boards that use an STM32 MCU?

They already had a version of this keyboard based on an 8051 type chip. This is V2, with QMK/VIA support being one of the advertised improvements.

@sehrgut
Copy link

sehrgut commented Nov 2, 2023

I agree that this should not be merged until NuPhy releases full source code. This is a violation of the GPL, which their legal department could have told them. Even if, as some have said, the BT code would not be mergeable into upstream QMK, the full source release MUST occur for NuPhy to be in compliance.

This is trivial and any IP lawyer would have told them the same thing.

@codebymiles
Copy link

codebymiles commented Nov 6, 2023

Adding my own weight to this. I purchased the keyboard solely due to the claimed QMK support for all functionality. Wireless and lighting is 100% included in that expectation. If I use QMK on the board I don't expect to lose any functionality. The product page specifically leads me to believe that this is the intended path NuPhy took, so to only claim that wired functionality with no lighting is the intended capability is a joke.

Regarding GPL, if NuPhy / @Persama has taken the QMK code and made changes to it so that it runs on the board with full functionality, they / @Persama MUST release the full code that includes that work under the GPLv2 license: https://www.gnu.org/licenses/gpl-faq.html#GPLRequireSourcePostedPublic

That includes all the lighting and wireless code, since the project includes the QMK GPLv2 code.

If they built a full codebase themselves without using QMK and that forms the firmware then they don't have to, but then it's not QMK either.

@Persama which one is it? Do you support QMK (in which case you need to release the full code that includes QMK), or are you releasing partial support, in which case the product page is incorrect?

@MarioRicalde
Copy link

Adding my own weight to this. I purchased the keyboard solely due to the claimed QMK support for all functionality. Wireless and lighting is 100% included in that expectation. If I use QMK on the board I don't expect to lose any functionality. The product page specifically leads me to believe that this is the intended path NuPhy took, so to only claim that wired functionality with no lighting is the intended capability is a joke.

Regarding GPL, if NuPhy / @Persama has taken the QMK code and made changes to it so that it runs on the board with full functionality, they / @Persama MUST release the full code that includes that work under the GPLv2 license: https://www.gnu.org/licenses/gpl-faq.html#GPLRequireSourcePostedPublic

That includes all the lighting and wireless code, since the project includes the QMK GPLv2 code.

If they built a full codebase themselves without using QMK and that forms the firmware then they don't have to, but then it's not QMK either.

@Persama which one is it? Do you support QMK (in which case you need to release the full code that includes QMK), or are you releasing partial support, in which case the product page is incorrect?

This reaction from a customer should not come as a surprise.

Sucks we are getting so much radio silence.

Sorry for everyone who fell for it. Be sure to read the Nuphy Return Policy.

@tzarc tzarc marked this pull request as draft November 8, 2023 08:11
@tzarc
Copy link
Member

tzarc commented Nov 8, 2023

I've converted this PR to draft, pending release of missing wireless and lighting code as per GPL requirements.
Thankyou to the people who have highlighted the lack of GPL compliance on the part of the vendor -- this will need to be rectified.

Any further complaints will be marked off-topic. This is a pull request, not a support request. Use proper channels with the vendor if you have specific grievances.

EDIT: swapped "Bluetooth" to "missing wireless and lighting".

@codebymiles
Copy link

Thanks @tzarc. It should be noted that the keyboard has both bluetooth and 2.4GHz wireless modes, so it would be expected that code for both is included (not just bluetooth code). Additionally, the current code doesn't appear to allow for addressing of the side lights on the keyboard, as per @RealGeo113's comment, so that would also be expected to be included.

I have also reached out to the vendor via their support channel to formally request the full source for both QMK and VIA code.

@waffle87 waffle87 dismissed their stale review November 8, 2023 17:37

Requires further changes

@codebymiles
Copy link

As an update, I received the following reply from NuPhy support on Nov 10th:

Hi,

The code that works with the wireless as well as the sidelight is currently being cleaned up and will be released in a short while.

‌ The pull request submitted to QMK is the minimal codebase that is necessary to have the keyboard recognized by QMK and therefore pave the way for VIA auto-detection, not the complete codebase that will be made public. This is done both to accelerate the enabling of official VIA support, and due to the fact that currently QMK lacks support for a wide range of wireless/bluetooth hardwares and features, and any code that needs to work with them will have to introduce core changes in QMK and extremely unlikely to be accepted.

‌ A common approach we adopt is to release the rest of the code elsewhere in public, probably in our own repo, which is fully GPL-compliant. It could be made public in the next two weeks. And when this happens we will make sure to inform users like you.

‌ Hope this helps and if you need any further clarification we can always be reached. Have a good day!

‌ Best Wishes,
‌ NuPhy Team!

Seemingly they will release the full code in the near future, however it may not be via this PR.

@tzarc
Copy link
Member

tzarc commented Nov 14, 2023

Seemingly they will release the full code in the near future, however it may not be via this PR.

That's fine, for GPL compliance.

@codebymiles
Copy link

Seemingly they will release the full code in the near future, however it may not be via this PR.

That's fine, for GPL compliance.

Agreed.

@BinaryShrub
Copy link

@Persama any update? Seems support said things will be releasing soon per @codebymiles.

@Persama Persama closed this by deleting the head repository Nov 30, 2023
@codebymiles
Copy link

codebymiles commented Nov 30, 2023

Yesterday I received an email from Nuphy stating that the full code had been released at the following location: https://github.com/nuphy-src/qmk_firmware/tree/nuphy-keyboards

I've yet to confirm that it does indeed contain everything, but smarter minds than mine are fully encouraged to go check it out and confirm.

@AlexRFox
Copy link

AlexRFox commented Dec 8, 2023

I also got an email from Nuphy late last night by my time (just before 1am EST) pointing me to PRs to this repo and saying that if I search for "Air60 V2" in the PRs, the code would come up. That brought me here and sadly I can't find any Air60 V2 code yet, I've reached out to them to clarify. Hopefully it means they're about to put in a PR with the Air60 V2 code, and that reply just slipped out to me before they had a change to submit it!

@AlexRFox
Copy link

I got another reply! It also seems they set up an autoresponder apologizing for slower response times since my first email, I'm hoping this is all symptoms of a more successful product launch than they expected. They apologized for telling me the wrong thing in the first email, and said they checked with the tech and "he said [the air60v2 qmk code] should be released within a month"

@terrehbyte
Copy link

Looks like commits landed in NuPhy's fork for the other two V2 series just days before Christmas:

I'll have to pick one up and try it out myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keyboard keymap via Adds via keymap and/or updates keyboard for via support
Projects
None yet
Development

Successfully merging this pull request may close these issues.