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

[core][new feature]Pointing device modes #21426

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

Conversation

Alabastard
Copy link

@Alabastard Alabastard commented Jul 2, 2023

Okay everyone this PR is finally back!

I have re based to current develop and tested that it does in fact compile let me know if there are any issues and I will address them as soon as I am able (also let me know if keycodes need to be bumped up another version I put it in 0.0.4 for now).

Also as it has been a while let me know if upcoming changes to pointing devices would require changes here.

Look in the feature_pointing_device.md doc under the new section: Pointing Device Modes for how this feature works

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).

@Alabastard
Copy link
Author

is the boards that are going over the memory limit expected?
the code this adds to base without enabling anything is almost nothing...

@sigprof
Copy link
Contributor

sigprof commented Jul 2, 2023

On NixOS with avr-gcc (GCC) 8.5.0 I get this on develop:

jones/v1:via        * The firmware is too large! 29126/28672 (454 bytes over)
keebio/bamfk1:via   * The firmware is too large! 28692/28672 (20 bytes over)

jones/v1:via is oversized even on master (40 bytes over); keebio/bamfk1:via on master barely fits (10 bytes free). So these boards are already broken on develop even without your changes, and are probably broken even on master with some different compiler versions.

@drashna drashna requested a review from a team July 27, 2023 19:45
@Alabastard
Copy link
Author

Let me know if there is anything I can do to help the process along

docs/feature_pointing_device.md Outdated Show resolved Hide resolved
quantum/pointing_device/pointing_device_modes.h Outdated Show resolved Hide resolved
quantum/pointing_device/pointing_device_modes.c Outdated Show resolved Hide resolved
@github-actions github-actions bot added keyboard keymap via Adds via keymap and/or updates keyboard for via support CI dd Data Driven Changes labels Sep 9, 2023
@Alabastard Alabastard force-pushed the feature_pointing_device_modes branch from cf16784 to 77ca7e6 Compare September 9, 2023 00:29
@github-actions github-actions bot removed keyboard keymap via Adds via keymap and/or updates keyboard for via support CI labels Sep 9, 2023
@Alabastard
Copy link
Author

Okay so a bit of a major update to how pointing mode maps are working.

Pointing mode maps now use action_exec to process key presses (basically the same code as for encoders) which enables the ability to use more than basic keycodes (such as QMK keycodes etc.). Not much changes at the keymap level other than the new POINTING_NUM_DIRECTIONS instead of hard coding 4 to avoid issues and now POINTING_MODE_MAP_COUNT is handled automatically and now only a POINTING_MODE_MAP_ENABLE = yes is needed in rules.mk).

Documentation has not been fully updated yet.

Still needs proper testing but I have been able to compile it without issue.

@Alabastard Alabastard force-pushed the feature_pointing_device_modes branch 2 times, most recently from 1bca1f0 to be07d56 Compare September 14, 2023 20:01
@Alabastard
Copy link
Author

Alabastard commented Sep 14, 2023

Okay should be cleaned up now. Sorry for anyone who picked it up while it was a bit of a mess for a bit there.
need to be better about maintaining my git exclusions and keeping my local repo up to date and based on latest develop.

@Alabastard
Copy link
Author

Okay there was a bit more than documentation to fix up, but should be working as intended now.
POINTING_MODE_MAP_COUNT is now properly handled automatically at compile time (i.e. not intended for users to modify or define) pointing_mode_map_count(void) is meant for modifying map count if needed as a hidden feature similarly to encoders.

@Alabastard
Copy link
Author

Argh another keyboard is going oversize again is this expected? (the code should not add much size without enabling things unless I missed something)
Also what is keeping this code from going into develop? is there anything that still needs to be addressed?

@Alabastard Alabastard requested a review from drashna October 24, 2023 03:35
@burkfers
Copy link
Contributor

burkfers commented Nov 20, 2023

I have some feedback:

First off, thank you very much, alabastard, this is great!

I tested the changes and have been running them on my productive keyboard for a combined 35 hours without trouble, though I did not try pointing mode maps yet.
It does what is advertised and does it well. Thanks to the expansive documentation, it was easy to integrate. I found no side effects that the docs didn't warn me about.

However, I found one small issue (or two tiny related ones, depending on how you look at it). The down and left directions of caret scrolling, volume, and history modes are swapped, and the order of the parameters for the relevant pointing_tap_codes are not in vimkeys order as documented.

I've submitted a PR to the source fork that hopes to address this minor issue.
Alabastard#1

@Alabastard
Copy link
Author

First off, thank you very much, alabastard, this is great!

You are very welcome! I am just happy when someone uses my code :)

I tested the changes and have been running them on my productive keyboard for a combined 35 hours without trouble, though I did not try pointing mode maps yet. It does what is advertised and does it well. Thanks to the expansive documentation, it was easy to integrate. I found no side effects that the docs didn't warn me about.

This is great! glad it is working out there in the wild and that the docs were for the most part pretty clear/thorough!

However, I found one small issue (or two tiny related ones, depending on how you look at it). The down and left directions of caret scrolling, volume, and history modes are swapped, and the order of the parameters for the relevant pointing_tap_codes are not in vimkeys order as documented.

I've submitted a PR to the source fork that hopes to address this minor issue. Alabastard#1

Thanks for bringing this to my attention!
Also thanks for taking the effort to submit the PR!
However there is an issue with the PR you submitted in that you introduce a bug but you caught some bugs which thanks for that (I'll give more details in response to your PR).

@keyboard-magpie
Copy link
Contributor

Relabelled for q2 pending further work by alabastard. I've tested what's in place so far and works well but appreciate the feedback in the chain above.

@Alabastard
Copy link
Author

Alabastard commented Feb 21, 2024

Thank you for moving this to q2 sorry I've been slow haven't had as much time as I would like to work on this each day but progress is being made.

Unfortunately the changes will require major updates to any code that uses this. A lot of function/define names have changed and many functions have been removed as they are no longer necessary.

This will hopefully simplify interaction with pointing modes and improve functionality as well. I will push an update as soon as I can compile and test.

Copy link

github-actions bot commented Apr 7, 2024

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 Apr 7, 2024
@keyboard-magpie keyboard-magpie added in progress and removed stale Issues or pull requests that have become inactive without resolution. labels Apr 7, 2024
@Alabastard
Copy link
Author

Alabastard commented Apr 29, 2024

Okay finally testing the latest changes and updating the documentation should push the changes really soon
Currently I have implemented the requested features along with the major rework:

  • Give consistent naming convention to all functions
  • Change precision to a per device setting that can be adjusted rather than a separate mode
  • General Cleanup of the code to remove some of the functions which are no longer needed
  • Reorganize primary data struct so all per device data is in the same struct
  • Allow for pointing mode types which currently allow for switching between tapping and holding type modes as well as 4 way vs D-pad
    • Holding modes currently hold a key down until the next movement of the pointing device (this works with both the 4-way and D-pad mode types) (in D-pad modes horizontal and vertical presses are tracked independently)
    • Also still adding in catches for this to prevent orphaned held keys (timers and cleanup on layer switching etc.)
    • D-pad mode will send both horizontal and vertical key presses (as taps or key holds depending on the mode type) when the pointing device reports a diagonal movement (a diagonal movement has a rather abstract definition based on what I think could be efficiently calculated will be shown in the documentation)
    • 4-way is the default and will just send a key press when movement is primarily in one of the four directions
  • Focus the feature on using mode maps for all modes that involve key presses

Currently Requested features that were not included are:

  • Allow for the syncing of mode maps to keyboard layers if desired (similarly to how encoders work) (Planned)
  • Allow for making 8-way maps to allow for sending unique keys for diagonal movements (Not planned at the moment)

@Alabastard
Copy link
Author

Alabastard commented May 27, 2024

Apologies for the delay should have the update this soon
Was having issues with the holding behavior being a bit janky... I was considering removing it due to the bloat it can add (two extra 16 bit variables per device) but I think I'll just get it working as intended with cleanup on mode change and timeout so someone with a track pad can test and see if they like it (perhaps I should have this as an option?)

@Alabastard
Copy link
Author

Okay I'm back from my health crisis/the dead to finish this up I have resolved the conflicts just need to move keycode stuff appropriately and then I should be good to push the update.

@KarlK90
Copy link
Member

KarlK90 commented Oct 6, 2024

@Alabastard welcome back and good to hear that you are doing better. Leave a comment when you have updated the PR - I intent to give it a review once the merge conflicts are resolved.

@Alabastard
Copy link
Author

Still working on this should have update soon.


# Pointing Device Modes :id=pointing-device-modes

Inspired by the work of previous trackball users that added features such as drag scroll, caret scroll, and sniping modes to their keyboards, this framework allows for easy setup and inclusion of different pointing device modes that when active will change the behaviour of a pointing device by taking it's x/y outputs and changing them into something else such as h/v for drag scrolling, key presses such as arrow keys for caret scrolling, and even just adjusting the x/y values before output. When a pointing device mode is active it accumulates x and y outputs from a pointing device and stores it into internal x & y values, halting normal mouse x and y output (_modes can re-enable and/or modify mouse output_), these internally stored x and y values are then divided by a defined divisor resulting the modified output (_key taps, h/v, modified mouse x/y etc._). The dividing factors can be used to control sensitivity in each mode as adjusting cpi may not always be desired/possible.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a minor spelling/grammar nitpick, the fourth line should be ..."by taking its x/y"... rather than "it's".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this I have re written the intro a bit and cleaned up the grammar hopefully.

@Alabastard
Copy link
Author

Alabastard commented Oct 29, 2024

Okay I am actually pretty happy with the state of the code right now just going through the docs to adjust to the changes while I am testing the code. Will push an update as soon as docs are updated.

Currently there are still two requested features missing:

  1. Option to force syncing of pointing modes to layer state
    -Not sure how to do this in a way that is not annoying when there are multiple devices
    -could force it for single device only
  2. Allow for 8-way directional support for keys with unique keycodes on diagonal movements
    -Diagonals is supported only for D-PAD type modes that will press both horizontal and vertical keys on diagonal movement
    -Additionally this will reduce the maximum number of maps to 32 as it only is using one row of the matrix

@daskygit
Copy link
Member

Currently there are still two requested features missing:

I'd vote for getting it merged as is, the extra features can always be sorted later and those will be a smaller (easier) pull request to review.

@Alabastard
Copy link
Author

Alabastard commented Nov 14, 2024 via email

@tzarc tzarc added breaking_change_2025q1 Targeting breaking changes Q1 2025 and removed develop-fast-track Intended to be merged early in the next develop cycle. breaking_change_2024q4 labels Nov 21, 2024
@Alabastard
Copy link
Author

Alabastard commented Dec 1, 2024

Okay finally doing a full update and just running into some issues with keycodes as soon as I can set a new range I'll get this up

edit:
New range selected but I need to make some adjustments to account for the extended wheel reports (should be able to use my old code if it is handled the same way as my old implementation)

Really close here I'm hoping to get all this done over the holidays

@Alabastard
Copy link
Author

Alabastard commented Dec 25, 2024

Update:
New Code is compiling finally after a greater than expected number of changes to how the json needs to be setup and cleaning up the keycodes (currently I have added them as 0.07 let me know if I need to change that to 0.06).
Once I get it tested on a keyboard I will push the update, hopefully tomorrow.

Testing has been a lot more of a slog than I expected especially with getting the data driven config done correctly but progress is being made.

@Alabastard
Copy link
Author

Alabastard commented Jan 2, 2025

Okay running tally of everything that has been tested, fixed, and working:

  • New Precision setting and Divisor code
  • 4 way Maps ( allows for using QMK keycodes)
  • 4 way tapping workaround function (legacy only standard keycodes allowed)
  • D-PAD Mode type (Fixed!)
  • Hold key modes (Working but differences to tapping mode are very subtle)
  • 8 Way Maps (Working!)
  • 2 Way Maps
  • inverting setting per mode (part of mode type)
  • allowing adjusting settings (Divisor, and type) on default modes (Drag/regular pointer)
  • Finish updating Documentation with all changes (Now finally fixing up this)
    • Update 4-way mode maps documentation, divisor setting, and precision
    • Add 8-way mode map documentation
    • Update all function names to new naming scheme
    • Add section for adjusting mode types and options with examples
    • Update advanced mode type definitions to include new functions for accessing residuals etc.
    • Update any device switching code to reflect changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking_change_2025q1 Targeting breaking changes Q1 2025 core dd Data Driven Changes documentation in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants