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

Add ps2_mouse pointing device #22532

Open
wants to merge 29 commits into
base: develop
Choose a base branch
from
Open

Conversation

joh
Copy link
Contributor

@joh joh commented Nov 22, 2023

Description

Migrate ps2_mouse to the pointing device framework.

  • Migrate ps2_mouse to pointing device (POINTING_DEVICE_DRIVER=ps2_mouse). This effectively removes PS2_MOUSE_ENABLE. Many PS2_MOUSE features have been replaced by pointing device equivalents (see table below).
  • The ps2_mouse code is moved to drivers/sensors/ps2_mouse.[ch], together with the rest of the pointing device code.
  • The ps2_mouse_pointing_device_{get,set}_cpi() implementations use counts/mm instead of counts/inch, since this is the unit employed by the PS/2 mouse protocol.

The code has been tested on RP2040 with PS2_DRIVER=vendor and ATmega32U4 with PS2_DRIVER=usart using a SK8707 trackpoint.

To be completed

  • Refactor ps2_mouse code
  • Port ps2_mouse keyboards over to pointing device
  • Update documentation
  • Test on the other PS/2 implementations(?)
  • Compile a list of keyboards using PS2_MOUSE and what features are in use.
  • Compile a list of which PS2_MOUSE features have pointing device replacements, and which features need to be retained.

Keyboards using PS2_MOUSE feature

From discussions on Discord, there seems to be a consensus to move PS2_MOUSE over to pointing device, since the code is very much overlapping. A first step is then to compile a list of keyboards currently using PS2_MOUSE, to get an overview over where and how the feature is used.

A simple git grep PS2_MOUSE_ENABLE keyboards/ reveals that there are only a handful of keyboards in the qmk codebase using the feature. Below I have compiled a list of them, and noted which features are in use and why.

buzzard/rev1:

  PS2_DRIVER = interrupt
  PS2_MOUSE_ROTATE 270
  keymap default,crehmann:
    PS2_MOUSE_SCROLL_BTN_MASK (1<<PS2_MOUSE_BTN_RIGHT)
    PS2_MOUSE_SCROLL_BTN_SEND 500

evyd13/gh80_3700:

  keymap ps2:
    PS2_DRIVER = usart
    PS2_MOUSE_ENABLE_SCROLLING
    PS2_MOUSE_INIT_DELAY 1000
    PS2_MOUSE_X_MULTIPLIER 2
    PS2_MOUSE_Y_MULTIPLIER 2
    PS2_MOUSE_V_MULTIPLIER 1
    PS2_MOUSE_SCROLL_BTN_MASK 0
    PS2_MOUSE_USE_REMOTE_MODE

frobiac/blackbowl:

  PS2_DRIVER = usart
  PS2_MOUSE_USE_REMOTE_MODE
  PS2_MOUSE_INIT_DELAY 1000

handwired/108key_trackpoint:
PS2_DRIVER = usart

handwired/promethium:

  PS2_DRIVER = interrupt
  PS2_MOUSE_INIT_DELAY 2000
  keymap default,priyadi:
    ps2_mouse_init_user() to set trackpoint settings
    PS2_MOUSE_SEND()
    ps2_host_send()
    ps2_host_recv_response()

handwired/trackpoint:
PS2_DRIVER = usart

pierce:

  PS2_DRIVER = usart
  keymap durken1:
    ps2_mouse_moved_user() for implementing AUTO_BUTTONS (auto mouse layer)

PS2_MOUSE features and pointing device equivalents:

A lot of the PS2_MOUSE features listed above have pointing device equivalents, but not all.

PS2_MOUSE pointing device
PS2_MOUSE_SCROLL_MASK none
PS2_MOUSE_ENABLE_SCROLLING none
PS2_MOUSE_USE_2_1_SCALING none
PS2_MOUSE_INIT_DELAY pointing_device_init_user()?
PS2_MOUSE_[XYV]_MULTIPLIER pointing_device_set_cpi()?
PS2_MOUSE_SCROLL_DIVISOR_[HV] Only used for PS2_MOUSE_SCROLL_BTN_*
PS2_MOUSE_INVERT_BUTTONS none?
PS2_MOUSE_INVERT_[XY] POINTING_DEVICE_INVERT_[XY]
PS2_MOUSE_INVERT_[HV] Only used for PS2_MOUSE_SCROLL_BTN_*
PS2_MOUSE_ROTATE POINTING_DEVICE_ROTATION_*
PS2_MOUSE_SCROLL_BTN_* pointing_device_task_user(), example in docs
PS2_MOUSE_SEND none
PS2_MOUSE_USE_REMOTE_MODE none
PS2_MOUSE_DEBUG_{RAW,HID} POINTING_DEVICE_DEBUG
ps2_mouse_init_user() pointing_device_init_user()
ps2_mouse_moved_user() pointing_device_task_user()
ps2_mouse_set_resolution() pointing_device_set_cpi()
ps2_mouse_{enable,disable}_data_reporting() none
ps2_mouse_set_{remote,stream}_mode() none
ps2_mouse_set_scaling_{1_1,2_1}() none
ps2_mouse_set_sample_rate() none?

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

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 Nov 22, 2023
drivers/ps2/ps2_mouse.c Outdated Show resolved Hide resolved
@drashna drashna requested review from daskygit and a team November 28, 2023 04:48
drivers/ps2/ps2_mouse.c Outdated Show resolved Hide resolved
@morganvenable
Copy link

I'm so excited to see this PR. Thanks for pushing this along @joh!
Svalboard isn't in the QMK codebase yet, but uses PS2 for trackpoint with similar feature utilization as buzzard, and I'd love to be able to handle left/right installs or dual installs easily. Anything that gets it out of orphan status and into pointing devices would be a mitzvah in my eyes even with the loss of some specialized PS/2 feature-fu.

From my experience with Trackpoint, the ability to change the sensitivity registers seems like the most critical tuning parameter I'd like to somehow retain, whatever the method -- since the native scaling in the trackpoint itself operates before too much other gain is applied.

@joh joh force-pushed the pointing_device_ps2_mouse branch from 6af7bd3 to 30e6986 Compare December 13, 2023 21:55
@github-actions github-actions bot added dependencies dd Data Driven Changes labels Dec 13, 2023
Copy link

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 Jan 28, 2024
@joh
Copy link
Contributor Author

joh commented Jan 28, 2024

Status: waiting for review :)

@github-actions github-actions bot removed the stale Issues or pull requests that have become inactive without resolution. label Jan 29, 2024
lib/chibios-contrib Outdated Show resolved Hide resolved
@joh joh requested a review from drashna February 6, 2024 20:45
Copy link

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 Mar 23, 2024
@joh
Copy link
Contributor Author

joh commented Apr 1, 2024

Status: waiting for review

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.

A couple of issues I found while trying to compile.

Also, as mentioned on discord, it may be best to simply rip out the entire ps2 implementation wholesale, and migrate all of the code over

builddefs/common_features.mk Show resolved Hide resolved
builddefs/common_features.mk Show resolved Hide resolved
@joh
Copy link
Contributor Author

joh commented Aug 23, 2024

The code migration should now be complete. PR Lint complains about missing licenses for some keyboards that were ported over to pointing device.

I've been using this as my daily driver for some time, no issues on my hardware (RP2040 + SK8707 trackpoint module) . The remaining work is documentation and perhaps testing on additional hardware. It would be great to get the code reviewed before proceeding with the docs :)

@joh
Copy link
Contributor Author

joh commented Dec 21, 2024

I have now migrated and reworked the documentation, so this should be good to go for review. Also tested successfully on ATmega32U4 with USART.

@joh joh requested a review from drashna December 21, 2024 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review core dd Data Driven Changes documentation keyboard keymap stale Issues or pull requests that have become inactive without resolution.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants