Keychron / qmk_firmware

Open-source keyboard firmware for Atmel AVR and Arm USB families
https://qmk.fm
GNU General Public License v2.0
758 stars 1.02k forks source link

When will the `bluetooth_playground` branch be merged into master and in the main QMK repo? #145

Open edianannink opened 1 year ago

edianannink commented 1 year ago

Issue Description

Hi all, I'm a very happy K2 Pro user. Some time ago, I was searching for the firmware of this keyboard in the qmk/qmk_firmware repository. However, I couldn't find it and noticed that this repo existed. The firmware of this keyboard is located in the bluetooth_playground branch, and it took me some time to find it. It builds without any problems, which is nice. However, I must say that I'm a little bit disappointed that the firmware of the K series, and more specifically the bluetooth_playground branch, hasn't been merged yet into the master of this repo and finally into the main qmk/qmk_firmware repository. Are there any plans to do this? It would really help to gain some traction for the K series keyboards in the QMK community.

uzemelteteshj commented 1 year ago

Try this run qmk msys qmk setup -b bluetooth_playground Keychron/qmk_firmware after qmk list-keyboards and you will see k2 pro

svenjacobs commented 1 year ago

Try this run qmk msys qmk setup -b bluetooth_playground Keychron/qmk_firmware after qmk list-keyboards~ and you will see k2 pro

Unfortunately this doesn't answer the OP's question 🙁 I'm an owner of a K3 Pro and would also like to know when the bluetooth_playground is merged to master and then to QMK upstream to benefit from new QMK functionality and bugfixes?

ilmagico commented 1 year ago

I just bought a K3 pro (awesome keyboard btw, typing this with it) and, for a keyboard that advertises as one of its selling point "customizable with QMK" I was really surprised to see that the official QMK repo didn't support it. I found the bluetooth_playground branch by accident while troubleshooting an issue with via. This really deserves to be in the main QMK repo, especially for a keyboard explicitly sold as "QMK programmable".

16kRAMpack commented 1 year ago

(Keychron user, not a developer, but I have hacked around on my own fork of the QMK firmware).

I think there's two issues here:

  1. Why doesn't bluetooth_playground get merged into this repo's master?
  2. Why isn't the upstream repo (qmk/qmk_firmware) more up-to-date?

Regarding issue 1, QMK tell developers (like Keychron):

It is highly recommended for QMK development, regardless of what is being done or where, to keep your master branch updated, but never commit to it. Instead, do all your changes in a development branch and issue pull requests from your branches when you’re developing.

So keychron seem to be doing the correct thing here, developing in a separate branch.

Issue 2 seems to me to be with upstream (QMK). It looks to me like Keychron are submitting pull requests, and these are being actioned by QMK devs - but not as rapidly as we might want. Take a look at this actioned pull request for the C2 Pro - it's taken nearly three months, and includes comments from keychron-dev like "hi, please check this, it has been so long".

My own keyboard, a K8 Pro, is still in bluetooth_playground. It's unfortunate, but QMK is an open source project and is heavily reliant on volunteers. I want a QMK keyboard (I lean heavily on personal hacks I do through QMK), so my choice is either accept the issues that open source development upstream brings, or volunteer with QMK.

tl;dr - I don't think bluetooth_playground will ever (or should ever) be merged into this repo's master. And merges to the QMK repo seem to me to be issues with upstream.

svenjacobs commented 1 year ago

I don't think bluetooth_playground will ever (or should ever) be merged into this repo's master

Why shouldn't bluetooth_playground at least be merged into this repo's master? I mean these are actual products out there, sold to customers. Having this in master should simplify fetching changes from upstream?

ilmagico commented 1 year ago

@svenjacobs I think @16kRAMpack is right here. Different projects have different processes they follow to get external contributions done, and if QMK would rather you do PR's from different branches, and never commit to master, that should be followed. QMK sets the rules here, and it's good Keychron is following them.

Instead, I think the main effort should go into merging this into QMK's main repo, and I don't know who's to blame here, but if a product is sold as "QMK supported" or whatever, I'd expect it to be from the official repo.

Maybe the only thing that could be done here is update the README to point users to branches of interest for specific keyboard models, until those get merged upstream. This could be done in a dedicated branch (to follow QMK's rule) and that branch be set as the default one.

svenjacobs commented 1 year ago

and if QMK would rather you do PR's from different branches, and never commit to master, that should be followed

I misunderstood their recommendation. I thought they meant their master not the master branch of the fork.

So it would be nice if Keychron could get their bluetooth_playground merged into QMK's master. I believe this is in the interest of everyone?

edianannink commented 1 year ago

Thanks everyone for clarifying and discussing the issue. I think we should create a PR to merge the bluetooth_playground into QMK's master then if I understand correctly?

16kRAMpack commented 1 year ago

Thanks everyone for clarifying and discussing the issue. I think we should create a PR to merge the bluetooth_playground into QMK's master then if I understand correctly?

Not exactly. bluetooth_playground is a development branch, so it's always going to be changing, everytime Keychron create a new BT keyboard. The process looks to me to be that PRs are created for individual keyboards. (Here's the list of open PRs for Keychron boards I posted earlier: is:pr is:open keychron - 13 open as I write this comment).

A PR for the C2 Pro was merged a few days ago, so PRs are happening. As I mentioned in my previous comment, based on the list of open PRs and that comment from a Keychron dev ("hi, please check this, it has been so long"), it looks to me like the issue is simply that upstream (QMK) aren't able to review and approve PRs all that quickly. It makes sense - they're dealing with PRs from more than just Keychron, and are themselves volunteers on an open source project.

Syphdias commented 1 year ago

Maybe I missed it but the Q1 Pro (ISO layout) was released before the C1/C2 Pro and there is no merge request open or closed for it. If you search for "keychron q1 pro" you will find one PR where I asked if this was for Q1 Pro, and it wasn't.

Maybe it was forgotten or they plan to do a combined merge for multiple keyboard or there is little ISO love or they only want a few open PRs to upstream at a time. As far as I can tell there are only two people working on getting all Keychron keyboards to work and I hugely appreciate them. They are probably under pressure to prioritise the latest releases but it feels bad to be left out here.

Sadly, I currently lack the skill and time to look into it myself but I am looking forward to the day Q1 Pro ISO gets a PR to upstream and gets merged into upstream eventually.

ghost commented 1 year ago

Did anyone try to merge locally master into bluetooth_playground and build a firmware to see if it works?

Edit: I just created a branch from bluetooth_playground and merged master into it. Lots of conflicts. Many might be solvable.

Edit2: been fiddling with the files a little and found this image Looks like qmk will soon be merging keebs agian

mutageneral commented 1 year ago

I am a new owner of a K2 Pro ANSI RGB and new user of QMK. When using the online VIA tool, I saw this message in the macro section:

Upgrade firmware to use delays

Does the delay feature currently exist in the bluetooth_playground firmware and the keyboard came with an older version, or is the delay feature still missing? People here have mentioned the bluetooth_playground firmware missing newer features.

On the download page on the Keychron website, it says that the version of the firmware for K2 Pro ANSI RGB is "version 1.00 updated December 20, 2022". Would this have the delay features? Or maybe the delay feature was added to bluetooth_playground, but a new release was not made so compiling from source would work?

Sorry if this is off-topic.

Draxy518 commented 1 year ago

I'm fairly certain that we're never going to get the bluetooth_playground branch merged into the master qmk repo and it's all got to do with the licensing of the bluetooth chip SDKs. The comments on this reddit post discuss the issue: https://www.reddit.com/r/olkb/comments/10ckhi6/2023_and_no_mass_market_bluetooth_olkb/

If that is the real issue then I think we should work on getting Keychron to merge the changes from the qmk master into their bluetooth_playground branch so we get the new features.

hedgss commented 1 year ago

I just bought K11 PRO. This seems to be an unfair advertisement to mislead buyers. Buying a keyboard that declares QMK support, the user expects to have access to all QMK features, including the latest updates and bugfixes. It looks pretty disappointing.

SmetDenis commented 1 year ago

I have K 15 Pro and the same issue... :( https://www.reddit.com/r/Keychron/comments/16w2nt6/why_isnt_bluetooth_playground_merged_with_the/

svenjacobs commented 1 year ago

I'm fairly certain that we're never going to get the bluetooth_playground branch merged into the master qmk repo and it's all got to do with the licensing of the bluetooth chip SDKs.

I don't understand. If the bluetooth_playground branch contains code that is not compatible with GPLv2, it shouldn't even be allowed to be there because the project is licensed under GPLv2 according to the LICENSE file in the root folder of this project.

Anyway, if the merge to upstream is not possible for some reason, Keychron should at least keep the branch in sync with QMK's main branch.

I also bought my keyboard mainly because of QMK support and was hoping for regular updates to benefit from latest features and fixes.

adophoxia commented 1 year ago

Seeing as there's some... confusion around this topic, I'd hope to bring some clarification into this.

This was never an issue with incompatible licenses. The issue with this whole ordeal is because the code that Keychron used to make BT work for their K Pro and Q Pro boards is too custom to where even in its current state, it's not mergable.

Both, KC and and the QMK team are still trying to work out a solution towards this. It may be possible when the QMK team merge RIOT OS support into the repo, which is needed for their XAP project (XAP is their variant of VIA, but without relying on a third party for them to have reason to maintain support.)

So until/if this happens, people who want custom firmware for these kinds of boards will have to rely on Keychron's fork.

NovaViper commented 1 year ago

@adophoxia thank you for the clarification! Hopefully the teams over at Keychron and QMK manage to get something together and merge the changes.

MarioRicalde commented 1 year ago

Since we have to rely on Keychron's fork, and based on the marketing material, it sounds only fair that the company puts work into getting the fork up to date with all those bug fixes from upstream.

Right?

adophoxia commented 1 year ago

From what I've been told, the branch itself is based on Breaking Changes Q4 2022.

svenjacobs commented 1 year ago

I contacted Keychron support with the following message

Some months ago I bought a K3 Pro mainly because of the programmability via QMK. Unfortunately I noticed afterwards that the keyboard is not supported by the main QMK repository. Instead I have to use the bluetooth_playground branch of your own fork.

What is even more unfortunate is that this branch is very outdated. It's several thousand commits behind the master branch. I kindly ask you to keep this branch in sync with master or merge it back into master. I would like to benefit from latest features and bug fixes of QMK.

As I said I mainly bought this keyboard because of QMK support. Since it is advertised as such, I expect to be able to use the latest QMK version. I'm a bit disappointed that this is not the case. There is also an issue about this. It would be nice if some Keychron dev could respond there.

Thank you very much!

and got the following answer on Nov 2:

We will pass it to our team and will take that into consideration. Have a nice day!

gericho commented 1 year ago

I agree on every point, I will send an email regarding this to them too.

I contacted Keychron support with the following message

Some months ago I bought a K3 Pro mainly because of the programmability via QMK. Unfortunately I noticed afterwards that the keyboard is not supported by the main QMK repository. Instead I have to use the bluetooth_playground branch of your own fork. What is even more unfortunate is that this branch is very outdated. It's several thousand commits behind the master branch. I kindly ask you to keep this branch in sync with master or merge it back into master. I would like to benefit from latest features and bug fixes of QMK. As I said I mainly bought this keyboard because of QMK support. Since it is advertised as such, I expect to be able to use the latest QMK version. I'm a bit disappointed that this is not the case. There is also an issue about this. It would be nice if some Keychron dev could respond there. Thank you very much!

and got the following answer on Nov 2:

We will pass it to our team and will take that into consideration. Have a nice day!

tmas commented 10 months ago

Since the issue with getting these changes merged seems to be solely based on Bluetooth license issues, maybe it could be worth creating non-bluetooth implementations of each keyboard? I bought a Pro board because of the keycaps/switches it came with and don't care about Bluetooth, so I'd be happy to run regular QMK without Bluetooth support. If I wanted to use the Bluetooth functionality later, I could always flash back to the stock firmware or build QMK myself based on Keychron's fork.

Unfortunately while I am a software developer, I mostly work on web projects and have very limited knowledge of how QMK works. At some point I might attempt this myself, but whatever I come up with might not be worth releasing to the world.

Rokin05 commented 8 months ago

Just a short message to say that I'm in the same boat (K3 Pro ISO) and that i'm shedding a little tear as i see that nothing is really moving forward.

svenjacobs commented 8 months ago

I 100% agree! It is really sad that no Keychron representative has commented on this issue since it was created in July 2023 and that we're left alone with our concerns.

This was my first and last Keychron product 😡

gericho commented 8 months ago

I agree with everything, It will be the last Keychron product for me as well. (K15 PRO)

adophoxia commented 8 months ago

Said it before and I'll say it again. Their code is too custom to be merged into upstream QMK. The wireless_playground branch is more up to date with upstream, so that should leave little to talk about how it's old as heck to deal with.

svenjacobs commented 8 months ago

Related issue

github-actions[bot] commented 5 months ago

This issue has been automatically marked as stale because it has not had activity in the last 90 days. It will be closed in the next 30 days unless it is tagged properly or other activity occurs. For maintainers: Please label with bug, in progress, on hold, discussion or to do to prevent the issue from being re-flagged.

NovaViper commented 5 months ago

Go away, Stalebot. This issue is very much so still important and is a pressing one.

nickshanks347 commented 2 months ago

Sorry if this is off-topic but is there an easy way to pull the latest QMK updates from the main QMK repo into a locally-cloned Keychron QMK repo?

adophoxia commented 2 months ago

Sorry if this is off-topic but is there an easy way to pull the latest QMK updates from the main QMK repo into a locally-cloned Keychron QMK repo?

No because it doesn't work like that. In order for Keychron to make wireless work on QMK for these boards, they made both, code changes of the repo and used code that the QMK team would not accept as it is too custom for them to do so.

nickshanks347 commented 2 months ago

Sorry if this is off-topic but is there an easy way to pull the latest QMK updates from the main QMK repo into a locally-cloned Keychron QMK repo?

No because it doesn't work like that. In order for Keychron to make wireless work on QMK for these boards, they made both, code changes of the repo and used code that the QMK team would not accept as it is too custom for them to do so.

Gotcha ok. Shame to be honest. Like others I expected full QMK compatibility as advertised by Keychron.

adophoxia commented 2 months ago

Sorry if this is off-topic but is there an easy way to pull the latest QMK updates from the main QMK repo into a locally-cloned Keychron QMK repo?

No because it doesn't work like that. In order for Keychron to make wireless work on QMK for these boards, they made both, code changes of the repo and used code that the QMK team would not accept as it is too custom for them to do so.

Gotcha ok. Shame to be honest. Like others I expected full QMK compatibility as advertised by Keychron.

What do you mean by "full QMK compatibility"? The boards do use actual QMK firmware. Just not like as if it came from the main QMK repo.

svenjacobs commented 2 months ago

Just not like as if it came from the main QMK repo.

Right, but that is what customers expect when they read "QMK" and see the QMK logo on Keychron's product websites and packaging. My main concern is that I cannot just pull the newest version of QMK from the original repo and flash it onto my keyboard 😢

adophoxia commented 2 months ago

For what it's worth, I've actually been trying to get the k6_pro code to compile on a branch based off of upstream master, but using the changes Keychron made for wireless to work. So far, only 1 error is stopping me from compiling successfully.

42Craft commented 4 weeks ago

Sad to see that Keychron is not fixing this. This is not what you expect when you buy a QMK keyboard.