Implement relative mode for Cirque trackpad

[drzony]

Implement relative mode for Cirque trackpad

Description

Cirque can work in absolute or relative mode, they differ in features. Give user a choice to use one of them. Also relative mode saves a lot of space.

Types of Changes

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

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).

[dkao]

Does not work with #define MOUSE_EXTENDED_REPORT.

[dkao]

Do side scroll and secondary tap work on circular trackpads? I wasn't able to trigger these.

[drzony]

Yes, they should both work. Secondary tap is a bit awkward, since you need to exactly hit the "sweet spot"

[dkao]

Any way these can be made runtime configurable? Use case maybe one hand relative mode, one hand absolute mode.

[drzony]

This would require a full rework of the trackpad code + it would discard the "FW size" benefit for relative mode. In the case you provided I think it's better to use absolute on both halves.

[dkao]

Does this line up with the

Resolution 245 x 1/2 cpi

mentioned under PS/2 mode? Or is CPI intended to be an arbitrary scale for relative mode?

I was thinking cirque_pinnacle_get_cpi() and cirque_pinnacle_set_cpi() need to be overridden for relative mode.

[drzony]

Scale is arbitrary, there is no mention of how this is scaled in datasheet. The scale is probably the same as in absolute mode, however the range is not. One idea is to add some conversion in cirque_pinnacle_*_cpi() to something like:

if (some_value < cpi < some_other_value) {
  scale = 1;
} else if (...) {
  scale = 2;
}

And then in scale_data just do xTemp = coordinates->xDelta * scale

[dkao]

I was messing around with that 245 x 1/2 number from the PS/2 portion of the spec and can't get it to make sense....
It's not the same as absolute mode either from what I can tell. I give up.

Do you want to just do something like this in pointing_device_drivers.c:

#    if CIRQUE_PINNACLE_POSITION_MODE
uint16_t cirque_pinnacle_get_cpi(void) {
    return CIRQUE_PINNACLE_PX_TO_INCH(cirque_pinnacle_get_scale());
}
void cirque_pinnacle_set_cpi(uint16_t cpi) {
    cirque_pinnacle_set_scale(CIRQUE_PINNACLE_INCH_TO_PX(cpi));
}
#    endif

// clang-format off
const pointing_device_driver_t pointing_device_driver = {
    .init       = cirque_pinnacle_init,
    .get_report = cirque_pinnacle_get_report,
#    if CIRQUE_PINNACLE_POSITION_MODE
    .set_cpi    = cirque_pinnacle_set_cpi,
    .get_cpi    = cirque_pinnacle_get_cpi,
#    else
    .set_cpi    = cirque_pinnacle_set_scale,
    .get_cpi    = cirque_pinnacle_get_scale
#    endif
};

Empirically it gives sensitivity in a similar ballpark to the absolute mode scaling, and saves a bit more space.
Maybe split the relative mode and absolute mode driver into two sections divided by one #if CIRQUE_PINNACLE_POSITION_MODE, with a separate much more concise definition of cirque_pinnacle_get_report() for relative mode?

[drzony]

On PS/2 mode trackpad has a Set Resolution and Set Scaling commands. I think the 245 x 1/2 cpi is somehow relative to the values set there. Maybe they are available in extended registers, but there is zero documentation for them.

[drzony]

Squashed all the fixups

[dkao]

Something happened here to the mouse_report update for absolute mode.
Please restore and rebase, thanks!

[drashna]

This has some merge conflicts.

[drzony]

Rebased

[dkao]

Thanks! Looks good.
I still can't get secondary tap and side scroll to work at all on my TM040040. Wondering if it's only supported on certain models or possibly certain revisions. Might be a good thing to mention in the docs?
Also it could be nice to implement functions like cirque_pinnacle_enable_tap(), cirque_pinnacle_enable_secondary_tap(), cirque_pinnacle_enable_side_scroll() if someone wants to disable the features only on certain layers.

[drzony]

It's working on my TM040040. My config:

#define SPLIT_POINTING_ENABLE
#define POINTING_DEVICE_RIGHT
#define CIRQUE_PINNACLE_POSITION_MODE CIRQUE_PINNACLE_RELATIVE_MODE
#define CIRQUE_PINNACLE_TAP_ENABLE
#define CIRQUE_PINNACLE_SECONDARY_TAP_ENABLE
#define CIRQUE_PINNACLE_SIDE_SCROLL_ENABLE

Right click is on the top-right edge (half of my finger needs to be outside of the trackpad)
The scroll starts about 0.5cm from the right edge (up-down movement).

As for the functions I would leave them for another PR

[drashna]

The side scroll feels like it should be a generic option.

And the right click does too, to a lesser degree.

And I can confirm that the code appears to work in both modes, and with both features.

[drashna]

Right click is on the top-right edge (half of my finger needs to be outside of the trackpad)

I found this to be the case too. Would be worth noting in the docs.

[drzony]

@drashna
Something like:

#define POINTING_DEVICE_LEFT_CLICK_ENABLE // instead of CIRQUE_PINNACLE_TAP_ENABLE, if not defined disable button reporting for pimoroni also
#define POINTING_DEVICE_RIGHT_CLICK_ENABLE // instead of CIRQUE_PINNACLE_SECONDARY_TAP_ENABLE
#define POINTING_DEVICE_SCROLL_ENABLE // This would enable circular scroll on absolute and side scroll on relative

?

[drzony]

Updated docs with secondary tap info and flash difference amount

[drashna]

Yeah. Though, POINTING_DEVICE_GESTURES_* would probably be better, since we already have one define doing that.
https://qmk.github.io/qmk_docs_devel/#/feature_pointing_device?id=common-configuration

Though, I do mean something more in line with:
https://github.com/qmk/qmk_firmware/blob/develop/quantum/pointing_device/pointing_device_gestures.c

Namely, if it can be handled in a generic fashion, it should be. That way, as other sensors are added, they can take advantage of the features, right away.

If that can't be done, then no worries. It looks like the secondary tap is hardware dependent. Though, the generic define for the scrolling may be the best way to go. Not 100% sure, though.

[drzony]

Added POINTING_DEVICE_GESTURES_SCROLL_ENABLE