-
Notifications
You must be signed in to change notification settings - Fork 127
Reimplement mouse mode #282
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
base: master
Are you sure you want to change the base?
Conversation
1bcec4b
to
632d509
Compare
67dbffd
to
6f3058f
Compare
6951ccd
to
9856f89
Compare
1f49e7c
to
77ecd72
Compare
217abf9
to
43919e5
Compare
43919e5
to
0dfa5bd
Compare
0dfa5bd
to
b7fac5d
Compare
b7fac5d
to
6b89563
Compare
6b89563
to
26a0ecc
Compare
a9a4633
to
0a2226b
Compare
0a2226b
to
118d57c
Compare
118d57c
to
1873fb9
Compare
To properly sync our multiple sub-devices, we need to synchronize the input frame in xpadneo instead of the generic HID handler, otherwise we may see input late or duplicated. See-also: atar-axis#460 See-also: atar-axis#282 Signed-off-by: Kai Krakow <[email protected]>
To properly sync our multiple sub-devices, we need to synchronize the input frame in xpadneo instead of the generic HID handler, otherwise we may see input late or duplicated. See-also: #460 See-also: #282 Signed-off-by: Kai Krakow <[email protected]>
f16f26f
to
d4539af
Compare
f9ffcf5
to
55715d0
Compare
55715d0
to
89b24ac
Compare
89b24ac
to
55466f5
Compare
55466f5
to
ffbb921
Compare
ffbb921
to
447f301
Compare
Co-authored-by: Jacob Essex <[email protected]> Co-authored-by: Florian Dollinger <[email protected]> Closes: atar-axis#99 Closes: atar-axis#105 Closes: atar-axis#160 Closes: atar-axis#511 Fixes: atar-axis#333 See-also: atar-axis#419 See-also: atar-axis#435 Signed-off-by: Kai Krakow <[email protected]>
447f301
to
c5620e8
Compare
Code Climate has analyzed commit c5620e8 and detected 0 issues on this pull request. View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR reimplements the mouse mode functionality for Xbox controllers, adding support for mouse buttons, scrolling, and basic keyboard events. The implementation includes timer-based mouse movement reporting and maps controller inputs to mouse and keyboard actions.
- Adds a new mouse.c file with comprehensive mouse emulation functionality
- Implements timer-based mouse movement and scrolling with error accumulation
- Maps controller inputs to mouse buttons, cursor keys, and special functions
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
File | Description |
---|---|
hid-xpadneo/src/xpadneo/mouse.c | New file implementing complete mouse emulation with analog stick movement, button mapping, and keyboard event handling |
hid-xpadneo/src/xpadneo/keyboard.c | Adds keyboard capabilities for mouse mode (arrow keys, Enter, Escape) |
hid-xpadneo/src/xpadneo/core.c | Adds mouse synchronization support and consumer control error handling |
hid-xpadneo/src/xpadneo/consumer.c | Enables on-screen keyboard capability for mouse mode |
hid-xpadneo/src/xpadneo.h | Adds mouse-related data structures, timer, and function declarations |
hid-xpadneo/src/hid-xpadneo.c | Integrates mouse functionality into main driver with timer setup and event routing |
hid-xpadneo/src/Makefile | Includes mouse.c in the build |
docs/README.md | Documents mouse mode usage and controls |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
xdata->profile_switched = true; | ||
} | ||
|
||
#define mouse_report_rel(a,v) if((v)!=0)input_report_rel(mouse,(a),(v)) |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The macro lacks spaces around operators and parameters, making it hard to read. Consider reformatting as #define mouse_report_rel(a, v) if ((v) != 0) input_report_rel(mouse, (a), (v))
#define mouse_report_rel(a,v) if((v)!=0)input_report_rel(mouse,(a),(v)) | |
#define mouse_report_rel(a, v) if ((v) != 0) input_report_rel(mouse, (a), (v)) |
Copilot uses AI. Check for mistakes.
} | ||
|
||
#define rescale_axis(v,d) (((v)<(d)&&(v)>-(d))?0:(32768*((v)>0?(v)-(d):(v)+(d))/(32768-(d)))) | ||
#define digipad(v,v1,v2,v3) (((v==(v1))||(v==(v2))||(v==(v3)))?1:0) |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These macros contain complex logic with magic numbers (3072, 32768) and lack spacing around operators. Consider replacing with properly formatted inline functions with descriptive parameter names and documented constants.
#define digipad(v,v1,v2,v3) (((v==(v1))||(v==(v2))||(v==(v3)))?1:0) | |
/** | |
* digipad - Check if value matches any of three possible values. | |
* @value: The value to check. | |
* @v1: First possible value. | |
* @v2: Second possible value. | |
* @v3: Third possible value. | |
* | |
* Returns 1 if value equals v1, v2, or v3; otherwise returns 0. | |
*/ | |
static inline int digipad(int value, int v1, int v2, int v3) | |
{ | |
return (value == v1) || (value == v2) || (value == v3); | |
} |
Copilot uses AI. Check for mistakes.
input_report_key(keyboard, KEY_UP, digipad(value, 8, 1, 2)); | ||
input_report_key(keyboard, KEY_RIGHT, digipad(value, 2, 3, 4)); | ||
input_report_key(keyboard, KEY_DOWN, digipad(value, 4, 5, 6)); | ||
input_report_key(keyboard, KEY_LEFT, digipad(value, 6, 7, 8)); |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The magic numbers (1, 2, 3, 4, 5, 6, 7, 8) representing directional values should be defined as named constants to improve code readability and maintainability.
Copilot uses AI. Check for mistakes.
xdata->mouse_state.wheel_y = rescale_axis(value - 32768, 3072); | ||
return 1; | ||
case ABS_RZ: | ||
if (xdata->mouse_state.analog_button.left && value < 384) { |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The threshold values 384 and 640 are magic numbers that should be defined as named constants to clarify their purpose and make them easier to maintain.
Copilot uses AI. Check for mistakes.
xdata->mouse_state.analog_button.left = false; | ||
input_report_key(mouse, BTN_LEFT, 0); | ||
xdata->mouse_sync = true; | ||
} else if (!xdata->mouse_state.analog_button.left && value > 640) { |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The threshold values 384 and 640 are magic numbers that should be defined as named constants to clarify their purpose and make them easier to maintain.
Copilot uses AI. Check for mistakes.
} | ||
return 1; | ||
case ABS_Z: | ||
if (xdata->mouse_state.analog_button.right && value < 384) { |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The threshold values 384 and 640 are magic numbers that should be defined as named constants to clarify their purpose and make them easier to maintain.
Copilot uses AI. Check for mistakes.
xdata->mouse_state.analog_button.right = false; | ||
input_report_key(mouse, BTN_RIGHT, 0); | ||
xdata->mouse_sync = true; | ||
} else if (!xdata->mouse_state.analog_button.right && value > 640) { |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The threshold values 384 and 640 are magic numbers that should be defined as named constants to clarify their purpose and make them easier to maintain.
Copilot uses AI. Check for mistakes.
* Supports customization through profiles (work in progress) | ||
* Optional high-precision mode for Wine/Proton users (disables dead zones so games don't apply an additional one) | ||
* Share button support on supported controllers | ||
* Works as a mouse if you're are in couch-mode (press <key>Guide</key>+<key>Select</key>) |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a grammatical error with double 'are'. It should be 'if you're in couch-mode' or 'if you are in couch-mode'.
* Works as a mouse if you're are in couch-mode (press <key>Guide</key>+<key>Select</key>) | |
* Works as a mouse if you're in couch-mode (press <key>Guide</key>+<key>Select</key>) |
Copilot uses AI. Check for mistakes.
Todo: