Invert pointing device motion pin for cirque touchpads

[preisi]

Description

Cirque touchpads 'data ready'-pin implements active-high logic in contrast to most other pointing devices which use an active-low logic. Therefore, this commit implements both a configurable inversion of the motion pin logic and sets the the default for the cirque touchpad driver accordingly.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

This issue came up yesterday in the #help-firmware discord channel and was pointed out by @daskygit after Kuromitsu#5783 tried to use the motion pin.

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[KarlK90]

LGTM! Thanks.

[zvecr]

Not a fan of the chosen name as it doesnt really align with any of the existing options.

Suggestions would be to align to the existing _ACTIVE_LOW (or at a push _ON_STATE?).

[preisi]

Not a fan of the chosen name as it doesnt really align with any of the existing options.

I aligned the name with the other defines in quantum/pointing_device which are named POINTING_DEVICE_INVERT_{X,Y,_X_RIGHT,_Y_RIGHT}. But granted, I'm not the biggest fan of the naming myself as it is not very explicit about the default behavior of the motion pin.

Suggestions would be to align to the existing _ACTIVE_LOW (or at a push _ON_STATE?).

To my understanding (and after grepping over parts of QMK), the _ACTIVE_LOW/_ACTIVE_HIGH seems to be mostly a ChibiOS convention. I hope the new choice _ON_STATE is better. Thank you for your feedback :)

[daskygit]

My issue with _ON_STATE is it doesn't line up with terminology (active low/high) that will be used by datasheets.

[preisi]

My issue with _ON_STATE is it doesn't line up with terminology (active low/high) that will be used by datasheets.

Mh, that is obviously true. So you're suggesting to rename the current _ON_STATE to _ACTIVE_LOW and assign it the according boolean value? Or is there any better way your envisioning? Creating 2 defines "_ACTIVE_LOW" and "_ACTIVE_HIGH" and assign them to a third define as some sort of "enum" seems overkill to me. Or do you have something else in mind? (Otherwise I'd just rename the existing define to "_ACTIVE_LOW".

[daskygit]

Nah, you'd only need to define _ACTIVE_LOW for it to switch behaviour with the undefined/default being active high which wouldn't need to be specified. I guess something like how HAPTIC_ENABLE_PIN_ACTIVE_LOW is handled.

[preisi]

Nah, you'd only need to define _ACTIVE_LOW for it to switch behaviour with the undefined/default being active high which wouldn't need to be specified. I guess something like how HAPTIC_ENABLE_PIN_ACTIVE_LOW is handled.

Alright, this should now be more to your liking :) I'm not completely happy with the result as _ACTIVE_LOW is basically be the default for all pointing devices except for the cirque touchpad, but it is how it is :) Then again, generally defining it and then specifically undefining it for the cirque touchpad would be possible, too (if that were to be preferred).