Factor out just the mouse key emulation from #778 #1430

DelusionalLogic · 2022-08-17T21:48:18Z

This PR includes ONLY enough mouse emulation to enable mouse keys. It's a trimmed down version of #778.

I've left this as 2 commits, the first one is a squashed version of #778, the second removes everything that is not required for mouse keys.

It has been tested to work on a corne in usb mode. bindings on both the left and the right side work.

I would recommend squashing before merging, as having a commit that adds a bunch of stuff and then one that removes it again is sort odd, but it probably makes the review easier for people familiar with #778 already.

A squashed version of the original changeset from zmkfirmware#778 rebased on main

I've stripped out everything not strictly required for mouse button HID support. This include dropping the worker thread completely, shaving down the USB HID descriptor. Flattening the mouse/ source directory structure and removing a bunch of event handling. I have kept the mouse event handling separate from the other HID event handling since I figured that was a pretty neat split. If that's a bad idea, do tell. I've also added a test case for mouse button emulation, since that was untested before. The changes have been tested on a corne (split) in usb mode. Bindings on both the left and the right side works (with the left side as master).

DelusionalLogic · 2022-08-22T20:21:21Z

I've been looking though the code, and I'm maybe not entirely convinced the mouse_button_state_changed_event does anything for me. It doesn't really seem to fit with the rest of ZMK to raise events on behaviors.

Does anyone have any opinions on that?

codeandr3w · 2022-09-04T16:26:14Z

I tried this and it doesn't work for me: Windows shows the mouse HID device not loading correctly when attached by USB and I get nothing when connected via BLE. I tested with a nice!nano. The mousepr does work for me.

DelusionalLogic · 2022-09-08T17:12:55Z

@codeandr3w That interesting. Do you get any error message or anything in the Windows EventLog? I'm running an expanded version of this PR on my crkbd and it's working in both windows and linux over USB with no issues.

petejohanson

A few minor items. Thanks for working on this!

petejohanson · 2022-09-26T04:04:43Z

app/include/dt-bindings/zmk/mouse.h

+
+/* Mouse press behavior */
+/* Left click */
+#define MB1 (0x01)


Why not alias proper HID usage defines here, like we do for keyboard page?

petejohanson · 2022-09-26T04:05:03Z

app/include/dt-bindings/zmk/mouse.h

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2020 The ZMK Contributors


Please update the copyright dates.

petejohanson · 2022-09-26T04:05:57Z

app/include/zmk/endpoints.h

@@ -13,3 +13,4 @@ int zmk_endpoints_toggle();
 enum zmk_endpoint zmk_endpoints_selected();

 int zmk_endpoints_send_report(uint16_t usage_page);
+int zmk_endpoints_send_mouse_report();


Why not just use the existing API and pass the button page id?

We could do that. I think it's maybe a little odd since we're not telling the api which device we want to send a report for (in case you also had a button subusage inside the keyboard), but there's no technical problem.

Hmmm... Let me ponder that a bit more.

I'm having a hard time thinking of why we would have that subusage any time soon. If you can give me a compelling use case, I'm open to changing toward your proposed approach, otherwise I'd say we stick with the existing API/approach.

petejohanson · 2022-09-26T04:07:49Z

app/include/zmk/hid.h

+    HID_COLLECTION(HID_COLLECTION_PHYSICAL),
+    HID_USAGE_PAGE(HID_USAGE_BUTTON),
+    HID_USAGE_MIN8(0x1),
+    HID_USAGE_MAX8(0x10),


Is 0x10 hard coded here arbitrary, or based on some spec?

It's sorta arbitrary. It's the number of buttons, and 8 was chosen to fill up a word. There's no real rhyme or reason, and i suspect most systems won't even support all 8 buttons.

That's fine, can we pull out the number of supported buttons into a define value to name it?

petejohanson · 2022-09-26T04:09:36Z

app/src/behaviors/behavior_mouse_key_press.c

+static const struct behavior_driver_api behavior_mouse_key_press_driver_api = {
+    .binding_pressed = on_keymap_binding_pressed, .binding_released = on_keymap_binding_released};
+
+#define KP_INST(n)                                                                                 \


Suggested change

#define KP_INST(n) \

#define MKP_INST(n) \

aherriott · 2022-10-18T05:03:02Z

Also was unable to get this pr working on windows 11. nice!nano_v2 connected over USB. Haven't tried the original PR yet, that will be next.

Here's the details of the error from device manager:

Error:
This device cannot start. (Code 10)

Driver:
Microsoft HID-Compliant-Mouse 10.0.22000.653

Event:
Device HID\VID_1D50&PID_615E&Col03\8&222c9c9c&1&0002 had a problem starting.

Driver Name: msmouse.inf
Class Guid: {4d36e96f-e325-11ce-bfc1-08002be10318}
Service: mouhid
Lower Filters:
Upper Filters:
Problem: 0xA
Problem Status: 0xC0110004

Update: the same keyboard works just fine on Ubuntu 18. Just has driver errors on Windows. Let me know if you need more data. I'm not super familiar with HID but I'm happy to help where I can.

zmkfirmware/zmk@main...DelusionalLogic:zmk:mouse0buttons

DelusionalLogic · 2022-10-29T17:23:22Z

Firstly, Thanks for the review. I'm sorry it too so long to get back to this. I've asked some questions I think we should discuss before merging this, but I'll make the rest of the changes soon.

As for @aherriott. I'm not super familiar with how to debug this on windows, so I'll have to see if I can replicate. I daily a keyboard running this patch on windows without issue, but that keyboard also has additional unreleased changes. I'll try and revert to just this branch and see if i can replicate your issue.

If anyone has any idea of how to debug this, please do share. I'll happily fix it if I know what the issue is.

bartlomiejmucha · 2022-12-06T14:44:58Z

app/dts/behaviors/mouse_key_press.dtsi

@@ -0,0 +1,9 @@
+/ {
+	behaviors {
+		/omit-if-no-ref/ mkp: behavior_mouse_key_press {


Why not name it &mbp - mouse button press or &mbc - mouse button click? I think the mouse button is a more common name than the mouse key.

Good point.. I'm more inclined to go with the press version though, since we keep the press/release terminology still.

petejohanson · 2023-03-27T06:19:03Z

So, one thing I'm noticing here, is that the HID report/descriptor here includes the buttons, but no X/Y axis report information. I'm wondering if perhaps this trips up Windows' HID driver which expects a collection in this usage page to include those.

The original lump sum PR had those items in the report descriptor, so perhaps worth adding that and seeing how the Windows testing goes.

petejohanson · 2023-11-07T00:43:50Z

I've just opened #1991 as a starting point so folks can test the fixes for Windows there, and will be iterating on that to try to address all the review feedback and get this in. Please test there!

petejohanson · 2023-11-24T06:08:43Z

Closing now that #1991 is merged.

krikun98 and others added 2 commits August 17, 2022 23:38

The original changes from zmkfirmware#778

b1deea5

A squashed version of the original changeset from zmkfirmware#778 rebased on main

petejohanson requested changes Sep 26, 2022

View reviewed changes

petejohanson self-assigned this Oct 8, 2022

zemackdaddy pushed a commit to zemackdaddy/zmk-refil that referenced this pull request Oct 23, 2022

just the mouse key emulation from zmkfirmware#778 zmkfirmware#1430

52950f6

zmkfirmware/zmk@main...DelusionalLogic:zmk:mouse0buttons

bartlomiejmucha reviewed Dec 6, 2022

View reviewed changes

petejohanson mentioned this pull request Nov 7, 2023

Pointer Core: HID, mouse keys behavior, etc. #1991

Merged

petejohanson closed this Nov 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Factor out just the mouse key emulation from #778 #1430

Factor out just the mouse key emulation from #778 #1430

DelusionalLogic commented Aug 17, 2022

DelusionalLogic commented Aug 22, 2022

codeandr3w commented Sep 4, 2022

DelusionalLogic commented Sep 8, 2022

petejohanson left a comment

petejohanson Sep 26, 2022

petejohanson Sep 26, 2022

petejohanson Sep 26, 2022

DelusionalLogic Oct 29, 2022

petejohanson Nov 4, 2022

petejohanson Nov 26, 2022

petejohanson Sep 26, 2022

DelusionalLogic Oct 29, 2022

petejohanson Nov 4, 2022

petejohanson Sep 26, 2022

aherriott commented Oct 18, 2022 •

edited

Loading

DelusionalLogic commented Oct 29, 2022

bartlomiejmucha Dec 6, 2022

petejohanson Jan 6, 2023

petejohanson commented Mar 27, 2023

petejohanson commented Nov 7, 2023

petejohanson commented Nov 24, 2023

Factor out just the mouse key emulation from #778 #1430

Factor out just the mouse key emulation from #778 #1430

Conversation

DelusionalLogic commented Aug 17, 2022

DelusionalLogic commented Aug 22, 2022

codeandr3w commented Sep 4, 2022

DelusionalLogic commented Sep 8, 2022

petejohanson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aherriott commented Oct 18, 2022 • edited Loading

DelusionalLogic commented Oct 29, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petejohanson commented Mar 27, 2023

petejohanson commented Nov 7, 2023

petejohanson commented Nov 24, 2023

aherriott commented Oct 18, 2022 •

edited

Loading