qmk_firmware
qmk_firmware copied to clipboard
[core][new feature]Pointing device modes
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
- [x] Core
- [ ] Bugfix
- [x] New feature
- [ ] Enhancement/optimization
- [ ] Keyboard (addition or update)
- [ ] Keymap/layout/userspace (addition or update)
- [x] Documentation
Issues Fixed or Closed by This PR
Checklist
- [x] My code follows the code style of this project: C, Python
- [x] I have read the PR Checklist document and have made the appropriate changes.
- [x] My change requires a change to the documentation.
- [x] I have updated the documentation accordingly.
- [x] I have read the CONTRIBUTING document.
- [ ] I have added tests to cover my changes.
- [x] I have tested the changes and verified that they work and don't break anything (as well as I can manage).
is the boards that are going over the memory limit expected? the code this adds to base without enabling anything is almost nothing...
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.
Let me know if there is anything I can do to help the process along
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.
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.
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.
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?
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. https://github.com/Alabastard/qmk_firmware/pull/1
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).
keycode issue due to more recent code. regen.sh needs a new run.
Okay just pushed a conflict resolution patch due to the dip switch changes.
"playing" with this PRs code, to see if&how a dpad feature would fit in here.
while doing so, some refactoring/typo fixes/.../suggestions came together = https://github.com/Alabastard/qmk_firmware/pull/2 feel free to cherry-pick commits from there (-:
"playing" with this PRs code, to see if&how a dpad feature would fit in here.
while doing so, some refactoring/typo fixes/.../suggestions came together = Alabastard#2 feel free to cherry-pick commits from there (-:
Thanks for this I see there is a decent amount of cleanup of my code included there but I'm wary about the changes to the cirque code as that seems a bit out of scope. I'll try and add in the cleanup right away and I'll have a think about the other changes for the moment.
~@zvecr I thought I should ask as you mentioned in #22770 that there are a few things in this PR that you think could use a revamp and I was hoping you could give a quick run down/summary so I could try and address them (sorry if you have done this in the past as I have lost comment history due to issues with my old github account) feel free to be as harsh as you would like as I'm always looking to improve :wink: and I think it would help get this PR ready to be merged.~ Nvm after seeing the feedback and PR from @JohSchneider I will be doing a large refactor and I'll just send out a request for review once that is done. Hopefully this addresses the issues and brings this code up to standard.
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.
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.
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.
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)
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?)