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.

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

Preserve signedness from opt_encoder_handler for scroll data on Ploopy devices #12223

Merged
merged 1 commit into from
Mar 25, 2021

Conversation

sbuller
Copy link

@sbuller sbuller commented Mar 13, 2021

This is a bugfix for Ploopy devices (trackball and mouse).

Ploopy devices use an optical encoder to produce scroll information. The opt_encoder_handler returns a value of 1, -1 or 0 depending on direction of scroll. This was being coerced into a uint8_t, discarding the sign. The problem was masked by the coercion happening when the value is put into the mouse descriptor's int8_t.

It's not clear to me why process_wheel_user uses int16_t for its h and v parameters, when the mouse_report values are only int8_t, and dir only takes values of 1, -1 and 0. Perhaps int8_t is more appropriate for the dir variable? Maybe even opt_encoder_handler? I matched the dir type to the function which produces it, and changed nothing that didn't strictly need to change.

@drashna drashna requested a review from a team March 24, 2021 03:17
Copy link
Member

@fauxpark fauxpark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opt_encoder_handler() should ideally return an int8_t to save space (and really we should not be using int etc. for clarity), otherwise looks good.

@drashna
Copy link
Member

drashna commented Mar 25, 2021

opt_encoder_handler() should ideally return an int8_t to save space (and really we should not be using int etc. for clarity), otherwise looks good.

That's probably because a lot of the code came from arduino originally. And it's something that I've been meaning to clean up.

@drashna drashna merged commit 97a7363 into qmk:master Mar 25, 2021
@sbuller sbuller deleted the ploopy_scroll_fix branch March 25, 2021 04:20
mrlinuxfish pushed a commit to mrlinuxfish/qmk_firmware that referenced this pull request Mar 28, 2021
d4mation added a commit to d4mation/qmk_firmware that referenced this pull request Mar 30, 2021
mrtnee pushed a commit to mrtnee/qmk_firmware that referenced this pull request Nov 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants