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

fix: mousekey doesn't work with trackpoint #18474

Merged
merged 8 commits into from
Nov 13, 2022

Conversation

klesh
Copy link
Contributor

@klesh klesh commented Sep 25, 2022

Description

Related to #18277, I'm using cantor with trackpoint, so the mouse buttons are essential for, but they didn't seem to function correctly.

After some digging, I managed to solve the problem, and now the trackpoint and mousekeys are working perfectly for me. Might as well fix it for everyone.

Let me know if I missed anything. Thanks.

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 the core label Sep 25, 2022
quantum/mousekey.c Outdated Show resolved Hide resolved
quantum/mousekey.c Outdated Show resolved Hide resolved
@tzarc tzarc requested review from drashna and a team September 25, 2022 07:54
klesh and others added 3 commits September 25, 2022 21:10
Co-authored-by: Nick Brassel <nick@tzarc.org>
Co-authored-by: Nick Brassel <nick@tzarc.org>
@drashna
Copy link
Member

drashna commented Sep 25, 2022

This reverts the behavior that adding this function as is sought to fix, in the first place.

I'm guessing that this is for the ps2 trackpoint, correct?

@klesh
Copy link
Contributor Author

klesh commented Sep 26, 2022

@drashna Not really, I figured the should_mousekey_report_send function was meant to prevent sending unwanted reports to the host, however, the condition it was using detects only the movement, buttons state was omitted, so it should affect other use cases as well.

@klesh
Copy link
Contributor Author

klesh commented Sep 26, 2022

@drashna I've updated the code to handle the button state on top of existing logic. It should work for all cases, would you like to take a look when you have time, thanks

@drashna
Copy link
Member

drashna commented Sep 26, 2022

buttons are sent immediately usually, while motion isn't. Also, originally, the code was:
https://github.com/klesh/qmk_firmware/blob/5eb3fc255b0484062e720525202c7ba7d56779fe/quantum/mousekey.c#L254

@klesh
Copy link
Contributor Author

klesh commented Sep 27, 2022

@drashna Thanks for the quick response, I read the code, and found out:

  1. https://github.com/qmk/qmk_firmware/blob/master/quantum/action.c#L524 shows that for PS2_MOUSE, it flips the tp_buttons state without sending it out immediately. Is it expected?
  2. I failed to find the logic sending out the button events to the host in quantum/action.c, maybe this is the root cause of my symptom: click might work if I moved the cursor a little bit right before.

@drashna
Copy link
Member

drashna commented Sep 27, 2022

tp_buttons gets sent by ps2_mouse_task automatically (located in /drivers/ps2/ps2_mouse.c). And prior to the register_button function, it was handled in the same way. It was extracted to make it easier to handle.

Honestly, I'd like to integrate the ps2 code into the pointing device drivers, as this would reduce the number of edge cases like this.

@drashna
Copy link
Member

drashna commented Sep 27, 2022

Thinking about this, and some other code, I may have a more workable long term solution.

@klesh
Copy link
Contributor Author

klesh commented Sep 27, 2022

Thinking about this, and some other code, I may have a more workable long term solution.

This is great, I'm hesitating to work on a better solution since I don't know anything about the pointing device, nor do I have the device to test it out. Not to mention that I'm no expert on c language.

I would like to help if you need it, please let me know. You know, helping you to help myself 😃.

@klesh
Copy link
Contributor Author

klesh commented Oct 6, 2022

@drashna The error reported by the QMK CI Build doesn't seem to be caused by my PR? Would you mind taking a look at it? Thanks

@drashna
Copy link
Member

drashna commented Oct 7, 2022

QMK CI Build error is unrelated to the code change here. So you can ignore it.

@drashna drashna requested a review from a team October 7, 2022 01:46
@klesh
Copy link
Contributor Author

klesh commented Oct 8, 2022

Hi, @tzarc , would you mind taking another look at this PR? thanks.

@drashna drashna requested a review from tzarc October 17, 2022 03:37
@joh
Copy link
Contributor

joh commented Oct 24, 2022

I have the same problem with my trackpoint using PS2 streaming mode. I can confirm that the changes in this PR fixes the problem for me as well.

@tzarc tzarc merged commit 133fe1c into qmk:master Nov 13, 2022
dkruyt pushed a commit to dkruyt/qmk_firmware that referenced this pull request Nov 18, 2022
Co-authored-by: Nick Brassel <nick@tzarc.org>
thystips pushed a commit to thystips/qmk_firmware that referenced this pull request Nov 24, 2022
Co-authored-by: Nick Brassel <nick@tzarc.org>
ramonimbao pushed a commit to ramonimbao/qmk_firmware that referenced this pull request Nov 28, 2022
Co-authored-by: Nick Brassel <nick@tzarc.org>
elpekenin pushed a commit to elpekenin/qmk_firmware that referenced this pull request Dec 7, 2022
Co-authored-by: Nick Brassel <nick@tzarc.org>
crembz pushed a commit to crembz/qmk_firmware that referenced this pull request Dec 18, 2022
Co-authored-by: Nick Brassel <nick@tzarc.org>
nolanseaton pushed a commit to nolanseaton/qmk_firmware that referenced this pull request Jan 23, 2023
Co-authored-by: Nick Brassel <nick@tzarc.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants