[Keyboard] Overhaul ploopyco devices #22967
Conversation
keyboards/ploopyco/ploopyco.c
Outdated
| #ifdef PLOOPY_DRAGSCROLL_INVERT | ||
| // Invert vertical scroll direction | ||
| mouse_report.v = -mouse_report.y; | ||
| mouse_report.v = -(int8_t)scroll_accumulated_v; | ||
| #else | ||
| mouse_report.v = mouse_report.y; | ||
| mouse_report.v = -(int8_t)scroll_accumulated_v; | ||
| #endif |
There was a problem hiding this comment.
-(int8_t)scroll_accumulated_v; is repeated twice here.
Trying this branch PLOOPY_DRAGSCROLL_INVERT has no effect due to this change.
Removing the negative modifier for the PLOOPY_DRAGSCROLL_INVERT ifdef branch seems to have the desired effect for me.
There was a problem hiding this comment.
This is perhaps personal preference, but I may expect both h and v values here to be inverted. It's unclear from the documentation if the PLOOPY_DRAGSCROLL_INVERT is only supposed to be for vertical scrolling. The reason I choose invert is to make the trackball scroll behavior to behave closer to a touch device when "moving" the plane around. The following gives me the behavior I would expect when turning on "invert"
#ifdef PLOOPY_DRAGSCROLL_INVERT
mouse_report.h = -(int8_t)scroll_accumulated_h;
mouse_report.v = (int8_t)scroll_accumulated_v;
#else
mouse_report.h = (int8_t)scroll_accumulated_h;
mouse_report.v = -(int8_t)scroll_accumulated_v;
#endifThere was a problem hiding this comment.
Other than the unneeded negative for the non-inverted, I'd rather keep it as is, as that is closer to the older behavior.
There was a problem hiding this comment.
Yup understood it was like that before, I only left the second comment because I still wasn't able to get the "invert" behavior to be quite right.
Please feel free to leave it was it was!
| # ifndef PLOOPY_DPI_DEFAULT | ||
| # define PLOOPY_DPI_DEFAULT 1 | ||
| # endif | ||
| #endif | ||
| #ifndef PLOOPY_DPI_DEFAULT | ||
| # define PLOOPY_DPI_DEFAULT 0 | ||
| #endif |
There was a problem hiding this comment.
It's unclear if this is intentional or not, but PLOOPY_DPI_DEFAULT is repeated here twice
I don't think it has any ill effects, since PLOOPY_DPI_DEFAULT 1 will always be defined, or the keymap config.h will win, but that does mean that it appears PLOOPY_DPI_DEFAULT 0 may not be doing anything?
There was a problem hiding this comment.
if the user specifies a dpi options array, the default isn't set. So to ensure that it has a default set.
keyboards/ploopyco/ploopyco.c
Outdated
| #ifndef PLOOPY_DRAGSCROLL_MOMENTARY | ||
| if (record->event.pressed) | ||
| #endif | ||
| { | ||
| is_drag_scroll = record->event.pressed; | ||
| #else | ||
| if (record->event.pressed) { | ||
| is_drag_scroll ^= 1; | ||
| } |
There was a problem hiding this comment.
I believe this condition to be inverted. When I #define PLOOPY_DRAGSCROLL_MOMENTARY in my keymap config DRAG_SCROLL behaves as as toggled.
When I don't define PLOOPY_DRAGSCROLL_MOMENTARY it behaves as momentary
|
Is it worth stripping out the user keymaps too, whilst you're at it? |
github-actions
bot
added
the
via
Description
Types of Changes
Checklist