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

[Feature Request] Add Keychron Q3 #16812

Closed
i-am-logger opened this issue Apr 7, 2022 · 36 comments
Closed

[Feature Request] Add Keychron Q3 #16812

i-am-logger opened this issue Apr 7, 2022 · 36 comments

Comments

@i-am-logger
Copy link
Contributor

i-am-logger commented Apr 7, 2022

Support for KeyChron Q3 Keyboard

@i-am-logger i-am-logger changed the title Add Keychron Q1 [Feature Request] [Feature Request] Add Keychron Q1 Apr 7, 2022
@i-am-logger i-am-logger changed the title [Feature Request] Add Keychron Q1 [Feature Request] Add Keychron Q3 Apr 7, 2022
@yanfali yanfali transferred this issue from qmk/qmk_configurator Apr 7, 2022
@yanfali
Copy link
Contributor

yanfali commented Apr 7, 2022

As soon as it hits firmware it should work in configurator.

@IBNobody
Copy link
Contributor

Some of the delay of getting Q3 into Firmware might be because those of us with Q3 keyboards with encoders are reporting that the encoder is skipping events when the Q3 is loaded with compiled-from-source .bins. (It's not sampling the encoder fast enough to detect the required pulse edge transitions that indicate encoder turn direction.)

@i-am-logger
Copy link
Contributor Author

Yes, i'm aware of it. I am actually waiting to receive the knob version and will work towards a fix if one isn't present by the time i get it. the knob issue is across al Q knob keyboards

@IBNobody
Copy link
Contributor

That's good to know. I've troubleshot the problem down to the pin sampling level in encoder.c and witnessed the missing A and B line transition states, but I'm not sure how to increase the sampling rate. Is the culprit the overall matrix scan frequency of 188-191 for my board?

@i-am-logger
Copy link
Contributor Author

great, can you mark the function for me? will be easier to find when i get the keyboard. Thanks!

@IBNobody
Copy link
Contributor

encoder_read:

bool encoder_read(void) {

encoder_update called by encoder_read:

static bool encoder_update(uint8_t index, uint8_t state) {

This is the line in encoder_read that reads the pins.

encoder_state[i] |= (readPin(encoders_pad_a[i]) << 0) | (readPin(encoders_pad_b[i]) << 1);

And for this encoder, it needs to see 4 separate edges on the pulse on a counterclockwise turn:

I'm using the format of (pin A state, pin B state)

  • (1,1) to (1,0)
  • (1,0) to (0,0)
  • (0,0) to (0,1)
  • (0,1) to (1,1)

Sometimes it just misses the edge, usually the 3rd or 4th state where pins A and B are rising.

@IBNobody
Copy link
Contributor

@ibootstrapper The issue is clearly related to the matrix scan frequency. I played around with the ChibiOS settings CH_CFG_ST_FREQUENCY and was able to get the matrix scan frequency up from 191 to 600+ by turning off features and saw that the number of dropped encoder ticks was reduced but not eliminated. So we just need to get the scan frequency up above this.

The keyboard advertises a 1000Hz polling rate. One would think that this would mean that the keyboard scan rate could also achieve this level . . .

@IBNobody
Copy link
Contributor

So, the issue is indeed caused by the matrix scan rate. The low scan rate is caused by 3 factors.

  1. CKLED2001 RGB matrix driver - can this be optimized? It's significantly slowing down the scan rate.

  2. Custom Matrix (half the columns are using I/O, half using a '595 shift register) - There might be a better way to handle the shift register. Right now, the code shifts in a 0 to trigger a column and then shifts in all F's to clear the register. If instead of clearing the column it just shifted the 0 to the next column, a number of shifting operations could be eliminated and the keyboard matrix scan routine would run faster.

  3. CH_CFG_ST_FREQUENCY could be tweaked from 10k to 100k.

If I compile QMK or Keychron's fork with the RGB Matrix disabled, the custom matrix excluded, and CH_CFG_ST_FREQUENCY set to 10k, my matrix scan frequency jumps up to 2400Hz and that is well above the level needed for the matrix encodes to start working.

@lalalademaxiya1 FYI

@i-am-logger
Copy link
Contributor Author

i-am-logger commented Apr 19, 2022

@IBNobody i just got my Q3 with knob, are you on keychron's discord?

@IBNobody
Copy link
Contributor

There's a KC Discord? I'm on the QMK and SonixQMK ones. Throw me an invite. I'm also /u/IBNobody on Reddit.

@i-am-logger
Copy link
Contributor Author

https://discord.com/invite/HAYbRrTsjN, sent you a friend invite.

@mwyborski
Copy link

mwyborski commented Apr 25, 2022

Please check if this PR solves your problem:
#16932
You will have to define in config.h:

#define ENCODER_RESOLUTION 4
#define ENCODER_DEFAULT_POS 0x3

@lokher
Copy link
Contributor

lokher commented Apr 30, 2022

Please refer to PR #16954 to fix encoder issue.

@IBNobody
Copy link
Contributor

IBNobody commented May 3, 2022

Please refer to PR #16954 to fixed encoder issue.

Yes, I can confirm that this fixes the issue on Q3. Now I just need a repo branch that has both the encoder fix and the EEPROM/VIA fix.

@lokher
Copy link
Contributor

lokher commented May 3, 2022

It's already done in playground branch of keychron fork.

@damianesteban
Copy link

Excellent. This will be great to have.

@Cr8zyIvan
Copy link

I'm very new to mechanical keyboards and QMK. I acquired the Keychron Q3 with the hopes of getting into it.

  • Is this project piloted by the folks at Keychron, or is this purely fan based?
  • Is there an estimate when the firmware might available for the Q3?
    • My intention is not to pressure anyone. I know close to nothing about the process involved. This is simply a question of curiosity and eagerness to start coding with QMK!... and not knowing where else to post this question.

Cheers!

@mwyborski
Copy link

@Cr8zyIvan this project is not related to keychron. They make pull-requests and try to get their code into the project.
This being said, you should compile and flash the fork from keychron. I think the q3 should be available:
https://github.com/Keychron/qmk_firmware

In their fork they can commit what they see fitting and latest code code for the q series is there. It may take time for the code to be merged to this repository.

@mwyborski
Copy link

They have a q3 branch:
https://github.com/Keychron/qmk_firmware/tree/keychron-q3

@Cr8zyIvan
Copy link

Thank you!

@FWest98
Copy link

FWest98 commented Jun 5, 2022

Please refer to PR #16954 to fixed encoder issue.

Yes, I can confirm that this fixes the issue on Q3. Now I just need a repo branch that has both the encoder fix and the EEPROM/VIA fix.

I spent some time over the past few days to make such a branch: https://github.com/FWest98/OS-QMK_Firmware/tree/keychron/q3_fwest
It took some time, but it is based on the most recent QMK master with the Q3 definitions and the EEPROM and rotary encoder fixes.

@pandages
Copy link

Please refer to PR #16954 to fixed encoder issue.

Yes, I can confirm that this fixes the issue on Q3. Now I just need a repo branch that has both the encoder fix and the EEPROM/VIA fix.

I spent some time over the past few days to make such a branch: https://github.com/FWest98/OS-QMK_Firmware/tree/keychron/q3_fwest It took some time, but it is based on the most recent QMK master with the Q3 definitions and the EEPROM and rotary encoder fixes.

this looks good! I was thinking about trying to set per-key colors and per-layer effects by modifying QMK itself directly, rather than using VIA. but I don't know a lot about working with QMK, I'm new to it.

I've seen some really cool videos of effects people have achieved on other keyboards by modifying QMK.

@pandages
Copy link

I'm not familiar with compiling QMK; I don't have a build environment prepared yet. But I'd be happy to help review the merges and such. I'd like to see some of the intended QMK features working on the Keychron Q3. I'm sure many of the changes will also apply to the other Q-family boards, layout aside.

@mfin
Copy link

mfin commented Jul 20, 2022

Is there any progress here?

@i-am-logger
Copy link
Contributor Author

I worked on my own fork here and what i had now support the model with the knob

i-am-logger@909b1ea

Please let me know if you experinece any issues

@i-am-logger
Copy link
Contributor Author

@sharpenedblade
Copy link

The keychron fork used by default in the keyboard works properly now.

@ghost
Copy link

ghost commented Aug 14, 2022

@ibootstrapper if I’m not mistaken Q3 support is being added in this PR: #16912.

Maybe the work you’ve done on your fork would be useful there? Or as a separate PR?

@i-am-logger
Copy link
Contributor Author

thanks, i'll check it out

@tzarc
Copy link
Member

tzarc commented Aug 29, 2022

Up to Keychron to submit a PR. Until then we can't do anything about it.

@tzarc tzarc closed this as not planned Won't fix, can't repro, duplicate, stale Aug 29, 2022
@sharpenedblade
Copy link

A pr was just submitted, I think this issue should be reopened, or at least changed from not planned.

@quentin9696
Copy link

Hi Guys,

@tzarc Any idea on when the feature will be added? I see the PR is ready to merge.

Thank you for all your work !

@whyfuture
Copy link

whyfuture commented Oct 2, 2022

And why is this closed if it's not in the main repo?

@blalor
Copy link

blalor commented Oct 2, 2022

This is being implemented in #16912.

@whyfuture
Copy link

This is being implemented in #16912.

Sorry, missed the comment above. Thanks for pointing out.

@i-am-logger
Copy link
Contributor Author

i-am-logger commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests