Digitizer HID interface : absolute coordinates for mouse cursor #12851

a-chol · 2021-05-09T20:06:45Z

Digitizer HID interface : absolute coordinates for mouse cursor

Description

I've added a HID digitizer report enabled with DIGITIZER_ENABLE, which allows positioning the mouse cursor in absolute normalized screen coordinates.
This has been implemented for LUFA and Chibios backends, and tested on the pro micro as well as a blackpill and a proton c.

A few feature based on this could follow, such as mapping key coordinates to set the cursor position or a dichotomic positioning algorithm. Users can build mouse-based macros with this.

A handwired/onekey digitizer keymap has been added that rotates the cursor around the center of the screen.

Types of Changes

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

I'll get around to check codestyle if needed in the coming days.

a-chol · 2021-05-09T20:33:05Z

I tested it on windows but if someone could try on linux and macos, that would be great. Sometimes requirement for a functional report varies from one OS to another.

tzarc · 2021-05-09T23:27:56Z

I'll get around to check codestyle if needed in the coming days.

It's needed, along with all the copyright headers for new files, too.

drashna · 2021-05-10T01:30:20Z

Also, report information and HID endpoints need to be addded:
https://github.com/qmk/qmk_firmware/blob/3f854e16acc880b9c9ccab2244dc585705dfac1e/tmk_core/protocol/usb_descriptor.h

And V-USB support should be added.

a-chol · 2021-05-10T19:49:03Z

Also, report information and HID endpoints need to be addded:
https://github.com/qmk/qmk_firmware/blob/3f854e16acc880b9c9ccab2244dc585705dfac1e/tmk_core/protocol/usb_descriptor.h

And V-USB support should be added.

I was piggybacking on the shared report for the digitizer report descriptor as well as for the endpoint. Should I had a full dedicated endpoint for digitizer with the option to DIGITIZER_SHARED_EP to use the shared endpoint?

Regarding V-USB, it seems only the shared endpoint is an option. The joystick report was removed there pending some reorg of the endpoint management in V-USB (4226), and I'm not sure if this has happened yet. Do I had the digitizer reporting to the shared report of V-USB?

fauxpark · 2021-05-11T03:20:11Z

V-USB now has a shared endpoint (#11103) so you should be good to add this (and joystick) 👍.

docs/feature_digitizer.md

keyboards/handwired/onekey/keymaps/digitizer/config.h

keyboards/handwired/onekey/keymaps/digitizer/keymap.c

quantum/process_keycode/process_digitizer.c

tmk_core/protocol/usb_descriptor.c

tmk_core/common/host_driver.h

a-chol · 2021-05-13T16:49:23Z

So after a bit of a struggle I managed to get dedicated and shared endpoints working on lufa and chibios, and implemented the V-USB implementation.
I'll check code style all around for the coming commit.

a-chol · 2021-05-13T21:36:44Z

hmm, if I run qmk cformat on my branch, a lot of changes on files I didn't touch appear. This is not expected. Should I keep only changes on my files?

drashna · 2021-05-14T04:17:26Z

hmm, if I run qmk cformat on my branch, a lot of changes on files I didn't touch appear. This is not expected. Should I keep only changes on my files?

Yeah, just keep the changes on files you touched.

fauxpark · 2021-05-14T08:54:43Z

hmm, if I run qmk cformat on my branch, a lot of changes on files I didn't touch appear. This is not expected. Should I keep only changes on my files?

Use the Docker container to run clang-format, different versions seem to have different ideas of what "formatted" means.

tmk_core/protocol/vusb/vusb.c

keyboards/handwired/onekey/keymaps/digitizer/config.h

a-chol · 2021-05-14T12:47:14Z

I saw the PR #12819 as well. Should I preemptively apply the fix for the digitizer report as well?

a-chol · 2021-05-19T20:01:12Z

Just to sum up the outstanding issues :

the linter doesn't seem to find the same formatting issues in the docker container running on my computer than the CI one. Is it a problem?
the send_digitizer function pointer at the interface between tmk and the usb HW-dependent implementation adds a cost whether digitizer is enabled or not. Is Hiding it behind ifdef DIGITIZER_ENABLE while keeping it in the host_driver_t struct a good enough fix in the scope of this PR?
Do I apply the fix from [Core] ChibiOS fix O3 and LTO breakage of extra keys and joystick #12819 ?

fauxpark · 2021-05-19T22:25:13Z

the linter doesn't seem to find the same formatting issues in the docker container running on my computer than the CI one. Is it a problem?

It's a problem in the sense that clang-format's behaviour isn't consistent across versions. The solution for now is to always use the Docker container when running a format pass, otherwise when the PR is merged qmk-bot will open a subsequent PR to format it "properly".

the send_digitizer function pointer at the interface between tmk and the usb HW-dependent implementation adds a cost whether digitizer is enabled or not. Is Hiding it behind ifdef DIGITIZER_ENABLE while keeping it in the host_driver_t struct a good enough fix in the scope of this PR?

I'd prefer to have it implemented the way Joystick is, without touching the host_driver_t stuff at all.

Do I apply the fix from #12819 ?

If that PR goes in first, yes. Otherwise, we'd ask for it to be rebased to apply the changes to the Digitizer stuff as well.

a-chol · 2021-05-25T20:35:46Z

So I guess this PR is ready to be merged, at least feature-wise. The CI failed on something that was not related to the test and is failing to launch now, is there a way to relaunch it when it's not blocked by docker anymore?

keyboards/handwired/onekey/keymaps/digitizer/keymap.c

quantum/digitizer.c

docs/feature_digitizer.md

keyboards/handwired/onekey/keymaps/digitizer/keymap.c

quantum/digitizer.c

drashna

Just a couple of things, otherwise, looks good

absolute screen coordinates. Tested on Pro Micro, Proton C and Blackpill.

Co-authored-by: Ryan <fauxpark@gmail.com>

Add V-USB support

Fix formatting issues Move digitizer_task definition to the feature's base implementation file

pointer disappearing on Windows.

Co-authored-by: Ryan <fauxpark@gmail.com>

declaration being the interface to the implementation in each HW-specific usb implementation.

weak-linkage implementation for tests without usb implementation

a-chol · 2021-06-23T20:42:13Z

I updated the changes requested, everything looks good now.

…12851) * Add digitizer HID interface for setting the mouse cursor position at absolute screen coordinates. Tested on Pro Micro, Proton C and Blackpill. * Update docs/feature_digitizer.md Co-authored-by: Ryan <fauxpark@gmail.com> * Update tmk_core/protocol/usb_descriptor.c Co-authored-by: Ryan <fauxpark@gmail.com> * Add missing copyrights Add V-USB support * Add support for digitizer dedicated endpoint for lufa and chibios. Fix formatting issues Move digitizer_task definition to the feature's base implementation file * Run cformat on modified files * Change digitizer report usage to Digitizer instead of Pen to avoid pointer disappearing on Windows. * Update tmk_core/protocol/vusb/vusb.c Co-authored-by: Ryan <fauxpark@gmail.com> * Run cformat from docker image * Remove send_digitizer from host_driver_t and instead rely on the declaration being the interface to the implementation in each HW-specific usb implementation. * Fix build : send_digitizer shouldn't be static in vusb and add weak-linkage implementation for tests without usb implementation * Change digitizer user interface to match pointing device's * Update documentation with new API Co-authored-by: a-chol <nothing@none.com> Co-authored-by: Ryan <fauxpark@gmail.com>

github-actions bot added core documentation keymap labels May 9, 2021

a-chol changed the title ~~Add digitizer HID interface for setting the mouse cursor position at~~ Digitizer HID interface : absolute coordinates for mouse cursor May 9, 2021

tzarc requested a review from a team May 9, 2021 23:25

tzarc added the pr_checklist_pending Needs changes as per the PR checklist label May 9, 2021

fauxpark changed the base branch from master to develop May 9, 2021 23:30

fauxpark requested changes May 13, 2021

View reviewed changes

fauxpark reviewed May 13, 2021

View reviewed changes

tmk_core/common/host_driver.h Outdated Show resolved Hide resolved

a-chol force-pushed the feature/digitizer branch from 3164eb9 to 0521f7c Compare May 13, 2021 21:07

fauxpark requested changes May 14, 2021

View reviewed changes

tmk_core/protocol/vusb/vusb.c Outdated Show resolved Hide resolved

keyboards/handwired/onekey/keymaps/digitizer/config.h Outdated Show resolved Hide resolved

a-chol force-pushed the feature/digitizer branch 2 times, most recently from 3c07b73 to a881d71 Compare May 16, 2021 17:30

a-chol force-pushed the feature/digitizer branch 3 times, most recently from 6c2907c to e96d693 Compare May 25, 2021 20:30

drashna requested changes May 27, 2021

View reviewed changes

keyboards/handwired/onekey/keymaps/digitizer/keymap.c Show resolved Hide resolved

keyboards/handwired/onekey/keymaps/digitizer/keymap.c Outdated Show resolved Hide resolved

quantum/digitizer.c Outdated Show resolved Hide resolved

drashna reviewed May 30, 2021

View reviewed changes

docs/feature_digitizer.md Show resolved Hide resolved

drashna reviewed May 30, 2021

View reviewed changes

keyboards/handwired/onekey/keymaps/digitizer/keymap.c Show resolved Hide resolved

drashna reviewed May 30, 2021

View reviewed changes

keyboards/handwired/onekey/keymaps/digitizer/keymap.c Show resolved Hide resolved

drashna reviewed May 30, 2021

View reviewed changes

quantum/digitizer.c Show resolved Hide resolved

drashna requested changes May 30, 2021

View reviewed changes

a-chol and others added 13 commits June 23, 2021 22:16

Add digitizer HID interface for setting the mouse cursor position at

e61d19c

absolute screen coordinates. Tested on Pro Micro, Proton C and Blackpill.

Update docs/feature_digitizer.md

5092282

Co-authored-by: Ryan <fauxpark@gmail.com>

Update tmk_core/protocol/usb_descriptor.c

9f2f6bd

Co-authored-by: Ryan <fauxpark@gmail.com>

Add missing copyrights

1673f30

Add V-USB support

Add support for digitizer dedicated endpoint for lufa and chibios.

3a6e97b

Fix formatting issues Move digitizer_task definition to the feature's base implementation file

Run cformat on modified files

327aeec

Change digitizer report usage to Digitizer instead of Pen to avoid

a1f4fc9

pointer disappearing on Windows.

Update tmk_core/protocol/vusb/vusb.c

7c29bfb

Co-authored-by: Ryan <fauxpark@gmail.com>

Run cformat from docker image

092f81a

Remove send_digitizer from host_driver_t and instead rely on the

8426a48

declaration being the interface to the implementation in each HW-specific usb implementation.

Fix build : send_digitizer shouldn't be static in vusb and add

0de70e4

weak-linkage implementation for tests without usb implementation

Change digitizer user interface to match pointing device's

f7baaef

Update documentation with new API

f66fdd8

a-chol force-pushed the feature/digitizer branch from 5c46bbe to f66fdd8 Compare June 23, 2021 20:29

drashna approved these changes Jul 3, 2021

View reviewed changes

drashna requested a review from a team July 3, 2021 07:18

tzarc approved these changes Aug 17, 2021

View reviewed changes

tzarc merged commit 75b49af into qmk:develop Aug 17, 2021

rcrowell mentioned this pull request Jan 18, 2022

Expand digitizer feature. #15917

Closed

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Digitizer HID interface : absolute coordinates for mouse cursor #12851

Digitizer HID interface : absolute coordinates for mouse cursor #12851

a-chol commented May 9, 2021 •

edited

Loading

a-chol commented May 9, 2021

tzarc commented May 9, 2021

drashna commented May 10, 2021

a-chol commented May 10, 2021

fauxpark commented May 11, 2021

a-chol commented May 13, 2021

a-chol commented May 13, 2021

drashna commented May 14, 2021

fauxpark commented May 14, 2021

a-chol commented May 14, 2021

a-chol commented May 19, 2021 •

edited

Loading

fauxpark commented May 19, 2021

a-chol commented May 25, 2021

drashna left a comment

a-chol commented Jun 23, 2021

Digitizer HID interface : absolute coordinates for mouse cursor #12851

Digitizer HID interface : absolute coordinates for mouse cursor #12851

Conversation

a-chol commented May 9, 2021 • edited Loading

Description

Types of Changes

Checklist

a-chol commented May 9, 2021

tzarc commented May 9, 2021

drashna commented May 10, 2021

a-chol commented May 10, 2021

fauxpark commented May 11, 2021

a-chol commented May 13, 2021

a-chol commented May 13, 2021

drashna commented May 14, 2021

fauxpark commented May 14, 2021

a-chol commented May 14, 2021

a-chol commented May 19, 2021 • edited Loading

fauxpark commented May 19, 2021

a-chol commented May 25, 2021

drashna left a comment

Choose a reason for hiding this comment

a-chol commented Jun 23, 2021

a-chol commented May 9, 2021 •

edited

Loading

a-chol commented May 19, 2021 •

edited

Loading