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.

Sign up for GitHub

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

Jump to bottom

[Bug] Trackpoint work improperly in Interrupt Version (ARM chibios) PS/2 Mouse Support #22265

Open
d93921012 opened this issue Oct 13, 2023 · 1 comment · Fixed by #23694
Open

[Bug] Trackpoint work improperly in Interrupt Version (ARM chibios) PS/2 Mouse Support #22265

d93921012 opened this issue Oct 13, 2023 · 1 comment · Fixed by #23694
Labels
bug help wanted

Comments

@d93921012
Copy link

d93921012 commented Oct 13, 2023 •
edited

Describe the Bug

In the function ps2_mouse_task() in drivers/ps2/ps2_mouse.c, it should return when pbuf_has_data() retun false. If this function continues to execute the remaining codes, the status of the mouse will be updated with wrong data.

void ps2_mouse_task(void) {
    static uint8_t buttons_prev = 0;
    extern int     tp_buttons;

    /* receives packet from mouse */
#ifdef PS2_MOUSE_USE_REMOTE_MODE
    uint8_t rcv;
    rcv = ps2_host_send(PS2_MOUSE_READ_DATA);
    if (rcv == PS2_ACK) {
        mouse_report.buttons = ps2_host_recv_response();
        mouse_report.x       = ps2_host_recv_response();
        mouse_report.y       = ps2_host_recv_response();
#    ifdef PS2_MOUSE_ENABLE_SCROLLING
        mouse_report.v = -(ps2_host_recv_response() & PS2_MOUSE_SCROLL_MASK);
#    endif
    } else {
        if (debug_mouse) print("ps2_mouse: fail to get mouse packet\n");
    }
#else
    if (pbuf_has_data()) {
        mouse_report.buttons = ps2_host_recv_response();
        mouse_report.x       = ps2_host_recv_response();
        mouse_report.y       = ps2_host_recv_response();
#    ifdef PS2_MOUSE_ENABLE_SCROLLING
        mouse_report.v       = -(ps2_host_recv_response() & PS2_MOUSE_SCROLL_MASK);
#    endif
    } else {
        if (debug_mouse) print("ps2_mouse: fail to get mouse packet\n");
        // If there is no data the function should return to avoid to update the status.
        return;
    }
#endif

To reproduce the error, pressing and holding the left button of the mouse will cause the browser to cyclically trigger mouse down and mouse up events.
@strobo5
Copy link
Contributor

strobo5 commented Dec 20, 2023

I'm seeing the same issue, also with a Trackpoint and ChibiOS on an RP2040 and the PIO implementation for PS/2. Nice that you already found the cause!

So what I think what we want to avoid is this (I'm basically repeating what you wrote): when the main loop calls ps2_mouse_task() for the first time after the button down event, it finds no new data in the PS/2 receive buffer, but it tries to detect a button state change anyway. mouse_report was cleared on the previous loop iteration, but buttons_prev still holds the button down state and thus a wrong button up event is detected and reported. Click and hold, move the mouse, and each PS/2 packet sets the button state down again.

The suggested change (return when there is no new mouse data) works perfectly. mouse_report never gets changed in this case so there should be no need to reach ps2_mouse_clear_report() at the end of the function. Maybe create a merge request?

strobo5 added a commit to strobo5/qmk_firmware that referenced this issue May 11, 2024
@strobo5
Fix PS/2 Trackpoint mouse clicks (qmk#22265)
4a0c897 
Bug report and fix by d93921012, I'm just pushing the change.

Tested on ARM ChibiOS (RP2040) with a Trackpoint from a Thinkpad T61.
strobo5 added a commit to strobo5/qmk_firmware that referenced this issue May 11, 2024
@strobo5
Fix PS/2 Trackpoint mouse clicks (qmk#22265)
6ad766b 
Bug report and fix by d93921012, I'm just pushing the change.

Tested on ARM ChibiOS (RP2040) with a Trackpoint from a Thinkpad T61.
@strobo5 strobo5 mentioned this issue May 11, 2024
Merged
14 tasks
zvecr pushed a commit that referenced this issue May 20, 2024
@strobo5
Fix PS/2 Trackpoint mouse clicks (#22265) (#23694)
a850f7d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix PS/2 Trackpoint mouse clicks (#22265) strobo5/qmk_firmware
2 participants
@strobo5 @d93921012