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

Cirque trackpad features: circular scroll, inertial cursor #17482

Merged
merged 27 commits into from
Jul 13, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
26bbb93
cirque_pinnacle: Add timeout for potential loops
dkao Jun 26, 2022
20f677f
cirque_pinnacle: recalibrate during initialization
dkao Jun 26, 2022
421c2f9
cirque_pinnacle: Function to control smoothing
dkao Jun 26, 2022
0f2d232
cirque_pinnacle: Convert CPI according to diameter
dkao Jun 26, 2022
fbee3bb
cirque pointing driver: Make tap optional
dkao Jun 26, 2022
6e4a63a
cirque pointing driver: Circular scroll feature
dkao Jun 26, 2022
0c35461
cirque pointing driver: Inertial cursor (glide)
dkao Jun 26, 2022
f873816
cirque pointing driver: formatting
dkao Jun 26, 2022
7428dc1
cirque pointing driver: Adjust configuration functions
dkao Jun 26, 2022
43b2f99
cirque_pinnacle: Reword comment on calibration
dkao Jun 26, 2022
4d46e59
cirque_pinnacle: Import register definition header
dkao Jun 29, 2022
f85df63
cirque_pinnacle: Compile option for curved overlay
dkao Jun 29, 2022
8344eaa
cirque_pinnacle: Adjust calibration sequence
dkao Jun 29, 2022
97d1644
cirque pointing driver: Move fancy processing code
dkao Jun 30, 2022
de833e8
cirque pointing driver: Integer math for inch<->pixel conversion
dkao Jun 30, 2022
969e612
pointing_device_gestures: Adjust cursor glide types
dkao Jul 3, 2022
dbe15af
pointing_device_gestures: Q8 fixed point cursor glide
dkao Jul 3, 2022
516677a
cirque_pinnacle_gestures: Integer math circular scroll
dkao Jul 4, 2022
7158ed6
pointing device gestures: Update comments
dkao Jul 4, 2022
f08d5e7
pointing device gestures: div0 checks
dkao Jul 4, 2022
3ff32b0
pointing device gestures: Extract configurations to explicit struct
dkao Jul 4, 2022
60da778
circular scroll: Add left-handed mode
dkao Jul 5, 2022
7bf6c68
pointing device gestures: Conditional compilation
dkao Jul 9, 2022
064d9e4
cirque pointing driver: Add a default triggir_px for inertial cursor
dkao Jul 9, 2022
e48a3d1
pointing device gestures: Style
dkao Jul 9, 2022
18aa3a1
circular scroll: Use stdlib abs()
dkao Jul 9, 2022
8bd6c92
cirque_pinnacle: Minor change to read-modify-write variable names
dkao Jul 9, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/feature_pointing_device.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ This supports the Cirque Pinnacle 1CA027 Touch Controller, which is used in the
|`CIRQUE_PINNACLE_X_UPPER` | (Optional) The maximum reachable X value on the sensor. | `1919` |
|`CIRQUE_PINNACLE_Y_LOWER` | (Optional) The minimum reachable Y value on the sensor. | `63` |
|`CIRQUE_PINNACLE_Y_UPPER` | (Optional) The maximum reachable Y value on the sensor. | `1471` |
|`CIRQUE_PINNACLE_DIAMETER_MM` | (Optional) Diameter of the trackpad sensor in millimeters. | `40` |
|`CIRQUE_PINNACLE_ATTENUATION` | (Optional) Sets the attenuation of the sensor data. | `ADC_ATTENUATE_4X` |
|`CIRQUE_PINNACLE_TAPPING_TERM` | (Optional) Length of time that a touch can be to be considered a tap. | `TAPPING_TERM`/`200` |
|`CIRQUE_PINNACLE_TOUCH_DEBOUNCE` | (Optional) Length of time that a touch can be to be considered a tap. | `TAPPING_TERM`/`200` |
Expand Down
64 changes: 59 additions & 5 deletions drivers/sensors/cirque_pinnacle.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#define SYSCONFIG_1 0x03
#define FEEDCONFIG_1 0x04
#define FEEDCONFIG_2 0x05
#define FEEDCONFIG_3 0x06
#define CALIBRATION_CONFIG_1 0x07
#define PS2_AU_CONTROL 0x08
#define SAMPLE_RATE 0x09
Expand Down Expand Up @@ -49,7 +50,7 @@
// clang-format on

bool touchpad_init;
uint16_t scale_data = 1024;
uint16_t scale_data = CIRQUE_PINNACLE_DEFAULT_SCALE;

void cirque_pinnacle_clear_flags(void);
void cirque_pinnacle_enable_feed(bool feedEnable);
Expand Down Expand Up @@ -127,7 +128,8 @@ void cirque_pinnacle_enable_feed(bool feedEnable) {
// Reads <count> bytes from an extended register at <address> (16-bit address),
// stores values in <*data>
void ERA_ReadBytes(uint16_t address, uint8_t* data, uint16_t count) {
uint8_t ERAControlValue = 0xFF;
uint8_t ERAControlValue = 0xFF;
uint16_t timeout_timer;

cirque_pinnacle_enable_feed(false); // Disable feed

Expand All @@ -138,9 +140,10 @@ void ERA_ReadBytes(uint16_t address, uint8_t* data, uint16_t count) {
RAP_Write(ERA_CONTROL, 0x05); // Signal ERA-read (auto-increment) to Pinnacle

// Wait for status register 0x1E to clear
timeout_timer = timer_read();
do {
RAP_ReadBytes(ERA_CONTROL, &ERAControlValue, 1);
} while (ERAControlValue != 0x00);
} while ((ERAControlValue != 0x00) && (timer_elapsed(timeout_timer) <= CIRQUE_PINNACLE_TIMEOUT));
Copy link
Contributor

Choose a reason for hiding this comment

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

At least on I2C we could use && touchpad_init==true directly, as this will be set to false if there is any I2C error (or timeout). Not sure if SPI considers CIRQUE_PINNACLE_TIMEOUT at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

SPI doesn't have ack or check for any timeout. It will return whatever state MISO happens to be floating in, if the device is not connected. That could be either 0xFF or 0x00 (or something inbetween if there's noise on the line?).
Looking at AVR's spi_master.c for example: spi_start() always returns true, except if chip select pin is invalid but that's an error before ever interacting with the device. So touch_init check can't really apply for SPI.

Copy link
Member

Choose a reason for hiding this comment

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

This actually fixed a dead lock for me that happened with the code before. I started a small refactoring of the RAP_Read/Write functions. If you find it useful feel free to include it in this pr cirque_timeout.patch.txt

Copy link
Member

Choose a reason for hiding this comment

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

As a side note, if a single I2C transfer fails the whole driver will turn into an invalid state - which requires a reboot of the keyboard. I can think of mechanism on the pointing device driver level that will try to re-init a driver in such a case after a certain timeout. But this is not in the scope of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think handling touch_init and I2C transaction failures adds too much to this PR, let's handle that in a separate one.


RAP_ReadBytes(ERA_VALUE, data + i, 1);

Expand All @@ -150,7 +153,8 @@ void ERA_ReadBytes(uint16_t address, uint8_t* data, uint16_t count) {

// Writes a byte, <data>, to an extended register at <address> (16-bit address)
void ERA_WriteByte(uint16_t address, uint8_t data) {
uint8_t ERAControlValue = 0xFF;
uint8_t ERAControlValue = 0xFF;
uint16_t timeout_timer;

cirque_pinnacle_enable_feed(false); // Disable feed

Expand All @@ -162,9 +166,10 @@ void ERA_WriteByte(uint16_t address, uint8_t data) {
RAP_Write(ERA_CONTROL, 0x02); // Signal an ERA-write to Pinnacle

// Wait for status register 0x1E to clear
timeout_timer = timer_read();
do {
RAP_ReadBytes(ERA_CONTROL, &ERAControlValue, 1);
} while (ERAControlValue != 0x00);
} while ((ERAControlValue != 0x00) && (timer_elapsed(timeout_timer) <= CIRQUE_PINNACLE_TIMEOUT));

cirque_pinnacle_clear_flags();
}
Expand Down Expand Up @@ -192,6 +197,53 @@ void cirque_pinnacle_tune_edge_sensitivity(void) {
ERA_ReadBytes(0x0168, &temp, 1);
}

// Perform calibration
void cirque_pinnacle_calibrate(void) {
uint8_t calconfig;
uint16_t timeout_timer;

// CalConfig1 (Compensation)
// Bit 0: Calibrate, 1=calibrate, 0=complete
// Bit 1: Background Comp Enable, 1=enable, 0=disable
// Bit 2: NERD Comp Enable (No, I don't know what NERD means), 1=enable, 0=disable
// Bit 3: Track Error Comp Enable, 1=enable, 0=disable
// Bit 4: Tap Comp Enable, 1=enable, 0=disable
// Bit 6: Calibration Matrix Disable, 1=disabled, 0=enabled
RAP_ReadBytes(CALIBRATION_CONFIG_1, &calconfig, 1);
calconfig |= 0x01;
RAP_Write(CALIBRATION_CONFIG_1, calconfig);
KarlK90 marked this conversation as resolved.
Show resolved Hide resolved

// Calibration takes ~100ms according to GT-AN-090624, doubling the timeout just to be safe
timeout_timer = timer_read();
do {
RAP_ReadBytes(CALIBRATION_CONFIG_1, &calconfig, 1);
} while ((calconfig & 0x01) && (timer_elapsed(timeout_timer) <= 200));

cirque_pinnacle_clear_flags();
}

// Enable/disable cursor smoothing, smoothing is enabled by default
void cirque_pinnacle_cursor_smoothing(bool enable) {
uint8_t feedconfig3;

// FeedConfig3 (Advanced feature flags)
Copy link
Contributor

@Kriechi Kriechi Jun 26, 2022

Choose a reason for hiding this comment

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

Is this register available in all trackpad versions (TM040040/TM035035/TM023023) and Pinnacle firmwares?
GT-AN-090620 does not list it at all (presumably hidden under RESERVED).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FeedConfig3 register is listed in GT-AN-090625, which is stated as a companion document to GT-AN-090620. Same thing is stated for the ERA document (GT-AN-090623) and compensation document (GT-AN-090624).
Additionally, it is documented in Cirque's circle trackpad example code here. I don't see any indication it would not be present for a particular version of circle trackpad.
Empirically, I've tested this register on TM035035 and TM040040; the effect is subtle on 40mm but more prominent on 35mm.

// Bit 0: DualPoint Buttons, 1=enable, 0=disable
// Bit 1: Smoothing Disable, 1=disable, 0=enable
// Bit 2: Palm/NERD measurements Disable, 1=disable, 0=enable
// Bit 3: Noise Avoidance Disable, 1=disable, 0=enable
// Bit 4: WRAP lockout Disable
// Bit 5: Dynamic EMI adjust Disable
// Bit 6: HW EMI detection Disable
// Bit 7: SW EMI detection Disable
RAP_ReadBytes(FEEDCONFIG_3, &feedconfig3, 1);
KarlK90 marked this conversation as resolved.
Show resolved Hide resolved
if (enable) {
feedconfig3 &= ~0x02;
} else {
feedconfig3 |= 0x02;
}
RAP_Write(FEEDCONFIG_3, feedconfig3);
}

/* Pinnacle-based TM040040/TM035035/TM023023 Functions */
void cirque_pinnacle_init(void) {
#if defined(POINTING_DEVICE_DRIVER_cirque_pinnacle_spi)
Expand Down Expand Up @@ -241,6 +293,8 @@ void cirque_pinnacle_init(void) {
RAP_Write(Z_IDLE_COUNT, 5);

cirque_pinnacle_set_adc_attenuation(CIRQUE_PINNACLE_ATTENUATION);
// Perform manual calibration after setting ADC attenuation
dkao marked this conversation as resolved.
Show resolved Hide resolved
cirque_pinnacle_calibrate();

cirque_pinnacle_tune_edge_sensitivity();
cirque_pinnacle_enable_feed(true);
Expand Down
7 changes: 7 additions & 0 deletions drivers/sensors/cirque_pinnacle.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@
# define CIRQUE_PINNACLE_POSITION_MODE CIRQUE_PINNACLE_ABSOLUTE_MODE
#endif

#define CIRQUE_PINNACLE_DEFAULT_SCALE 1024
#ifndef CIRQUE_PINNACLE_DIAMETER_MM
# define CIRQUE_PINNACLE_DIAMETER_MM 40
#endif

// Coordinate scaling values
#ifndef CIRQUE_PINNACLE_X_LOWER
# define CIRQUE_PINNACLE_X_LOWER 127 // min "reachable" X value
Expand Down Expand Up @@ -84,6 +89,8 @@ typedef struct {
} pinnacle_data_t;

void cirque_pinnacle_init(void);
void cirque_pinnacle_calibrate(void);
void cirque_pinnacle_cursor_smoothing(bool enable);
pinnacle_data_t cirque_pinnacle_read_data(void);
void cirque_pinnacle_scale_data(pinnacle_data_t* coordinates, uint16_t xResolution, uint16_t yResolution);
uint16_t cirque_pinnacle_get_scale(void);
Expand Down
Loading