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

drivers/stmpe811: changes for interrupt-driven touch handling and gesture recognition #19885

Merged
merged 4 commits into from
Aug 31, 2023

Conversation

gschorcht
Copy link
Contributor

Contribution description

This PR contains changes needed for the purely interrupt-driven handling of the touch position as touch_dev device, which is a prerequisite for the realization of gesture recognition. (PR #19884).

The interrupt-driven approach of touch_dev devices (PR #19882) and the gesture recognition (PR #19884) need continuous reporting of the touch position as long as there is at least one touch. Since the driver so far only uses the Touch Detect interrupt, only the position at the beginning of a touch is available. All further positions must be polled.

Therefore, the changes in this PR additionally enable the FIFO Threshold interrupt when the touch_dev module is enabled. However, since the Touch Detect interrupt does not work reliably when the FIFO Threshold interrupt is enabled and the FIFO Threshold is 1, the FIFO Threshold is set to 2 by default when the touch_dev module is enabled.

Furthermore, the FIFO queue has to be reset after reading one touch position. Otherwise new touch positions are processed with a delay if the rate of calling the function to read the FIFO is slower than the rate at which the FIFO is filled. The reason for this is that with each call of this function only the oldest touch position is read value by value from the FIFO. Gestures can't be implemented with such a behavior.

Testing procedure

  1. tests/drivers/stmpe811 should work as before (only a single position is shown):
    BOARD=stm32f429i-disco make -j8 -C tests/drivers/stmpe811 flash
    
    main(): This is RIOT! (Version: 2023.10-devel-120-g848d1- 
    drivers/stmpe811_touch_dev_gestures)
    STMPE811 test application
    +------------Initializing------------+
    Initialization successful
    Pressed!
    X: 135, Y:131
    Released!
    
  2. tests/drivers/stmpe811 should work on top of PR tests/drivers/touch_dev: allow to test a touch device in polling mode #19882 with continuous outputs of touch positions:
    BOARD=stm32f429i-disco make -j8 -C tests/drivers/touch_dev flash
    
    main(): This is RIOT! (Version: 2023.10-devel-121-g38d3db-drivers/stmpe811_touch_dev_gestures)
    Event: pressed!
    X: 131, Y:145
    X: 133, Y:141
    X: 135, Y:138
    X: 138, Y:133
    X: 141, Y:128
    X: 146, Y:122
    X: 151, Y:117
    Event: released!
    
  3. tests/driver/touch_dev_gestures of PR drivers/touch_dev_gestures: add gesture recognition for touch devices #19884 should work on top of this PR:
    BOARD=stm32f429i-disco make -j8 -C tests/drivers/touch_dev_gestures flash
    
    main(): This is RIOT! (Version: 2023.10-devel-128-g05f690-drivers/touch_dev_gestures_work)
    Swipe right
    Swipe left
    Swipe down
    Swipe up
    Single Tap X: 246, Y:148
    Single Tap X: 256, Y:139
    Double Tap X: 247, Y:136
    Pressed    X: 246, Y:131
    Moving     X: 248, Y:132
    Moving     X: 250, Y:133
    Moving     X: 258, Y:135
    Moving     X: 270, Y:136
    Moving     X: 285, Y:132
    Moving     X: 300, Y:126
    Moving     X: 309, Y:122
    Moving     X: 310, Y:119
    Released   X: 310, Y:119
    

Issues/PRs references

@github-actions github-actions bot added the Area: drivers Area: Device drivers label Aug 14, 2023
@gschorcht gschorcht added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 14, 2023
@gschorcht gschorcht requested a review from aabadie August 14, 2023 21:03
@riot-ci
Copy link

riot-ci commented Aug 14, 2023

Murdock results

✔️ PASSED

f95b225 makefiles: fix typo in driver_with_touch_dev.mk

Success Failures Total Runtime
7937 0 7937 13m:48s

Artifacts

Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Found some typos in the comments.

drivers/stmpe811/stmpe811.c Outdated Show resolved Hide resolved
drivers/stmpe811/stmpe811.c Outdated Show resolved Hide resolved
drivers/stmpe811/stmpe811.c Outdated Show resolved Hide resolved
drivers/stmpe811/stmpe811.c Outdated Show resolved Hide resolved
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

please squash!

The FIFO queue has to be reset after reading one touch position. Otherwise new touch positions are processed with a delay if the rate of calling the function to read the FIFO is slower than the rate at which the FIFO is filled. The reason for this is that with each call of this function always only the oldest touch position is read value by value from the FIFO. Gestures, for example, can't be implemented with such a behavior
For continuous reporting of the touch position the FIFO threshold interrupt is used. This is necessary for example to implement gestures in interrupt mode. To activate the FIFO threshold interrupt and thus continuous reporting, the macro 'STMPE811_FIFO_THRESHOLD_ENABLED' must be set to 1. However, the driver only works reliably with FIFO threshold interrupts if the FIFO threshold value is at least 2. Otherwise the Touch Detect interrupt will not work correctly when a touch is released. Therefore, the FIFO threshold is set to 2 by default when `STMPE811_FIFO_THRESHOLD_ENABLED` is set. This default value can be overwritten by `STMPE811_FIFO_THRESHOLD`.
@gschorcht
Copy link
Contributor Author

please squash!

Done

@gschorcht gschorcht force-pushed the drivers/stmpe811_touch_dev_gestures branch from a1790f4 to 0382e10 Compare August 30, 2023 14:53
@aabadie aabadie added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 30, 2023
@github-actions github-actions bot added the Area: build system Area: Build system label Aug 30, 2023
@aabadie
Copy link
Contributor

aabadie commented Aug 30, 2023

bors merge

bors bot added a commit that referenced this pull request Aug 30, 2023
19885: drivers/stmpe811: changes for interrupt-driven touch handling and gesture recognition r=aabadie a=gschorcht

### Contribution description

This PR contains changes needed for the purely interrupt-driven handling of the touch position as `touch_dev` device, which is a prerequisite for the realization of gesture recognition. (PR #19884).

The interrupt-driven approach of `touch_dev` devices (PR #19882) and the gesture recognition (PR #19884) need continuous reporting of the touch position as long as there is at least one touch. Since the driver so far only uses the _Touch Detect_ interrupt, only the position at the beginning of a touch is available. All further positions must be polled.

Therefore, the changes in this PR additionally enable the _FIFO Threshold_ interrupt when the `touch_dev` module is enabled. However, since the _Touch Detect_ interrupt does not work reliably when the _FIFO Threshold_ interrupt is enabled and the FIFO Threshold is 1, the FIFO Threshold is set to 2 by default when the `touch_dev` module is enabled.

Furthermore, the FIFO queue has to be reset after reading one touch position. Otherwise new touch positions are processed with a delay if the rate of calling the function to read the FIFO is slower than the rate at which the FIFO is filled. The reason for this is that with each call of this function only the oldest touch position is read value by value from the FIFO. Gestures can't be implemented with such a behavior.

### Testing procedure

1. `tests/drivers/stmpe811` should work as before (only a single position is shown):
   ```
   BOARD=stm32f429i-disco make -j8 -C tests/drivers/stmpe811 flash
   ```
   ```
   main(): This is RIOT! (Version: 2023.10-devel-120-g848d1- 
   drivers/stmpe811_touch_dev_gestures)
   STMPE811 test application
   +------------Initializing------------+
   Initialization successful
   Pressed!
   X: 135, Y:131
   Released!
   ```
2. `tests/drivers/stmpe811` should work on top of PR #19882 with continuous outputs of touch positions:
   ```
   BOARD=stm32f429i-disco make -j8 -C tests/drivers/touch_dev flash
   ```
   ```
   main(): This is RIOT! (Version: 2023.10-devel-121-g38d3db-drivers/stmpe811_touch_dev_gestures)
   Event: pressed!
   X: 131, Y:145
   X: 133, Y:141
   X: 135, Y:138
   X: 138, Y:133
   X: 141, Y:128
   X: 146, Y:122
   X: 151, Y:117
   Event: released!
   ```
3. `tests/driver/touch_dev_gestures` of PR #19884 should work on top of this PR:
   ```
   BOARD=stm32f429i-disco make -j8 -C tests/drivers/touch_dev_gestures flash
   ```
   ```
   main(): This is RIOT! (Version: 2023.10-devel-128-g05f690-drivers/touch_dev_gestures_work)
   Swipe right
   Swipe left
   Swipe down
   Swipe up
   Single Tap X: 246, Y:148
   Single Tap X: 256, Y:139
   Double Tap X: 247, Y:136
   Pressed    X: 246, Y:131
   Moving     X: 248, Y:132
   Moving     X: 250, Y:133
   Moving     X: 258, Y:135
   Moving     X: 270, Y:136
   Moving     X: 285, Y:132
   Moving     X: 300, Y:126
   Moving     X: 309, Y:122
   Moving     X: 310, Y:119
   Released   X: 310, Y:119
   ```

### Issues/PRs references


Co-authored-by: Gunar Schorcht <[email protected]>
@bors
Copy link
Contributor

bors bot commented Aug 31, 2023

Build failed:

@aabadie
Copy link
Contributor

aabadie commented Aug 31, 2023

bors merge

@bors
Copy link
Contributor

bors bot commented Aug 31, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 8017249 into RIOT-OS:master Aug 31, 2023
23 checks passed
@gschorcht
Copy link
Contributor Author

@aabadie Thanks for reviewing and merging 😄

@gschorcht gschorcht deleted the drivers/stmpe811_touch_dev_gestures branch August 31, 2023 15:20
bors bot added a commit that referenced this pull request Sep 1, 2023
19884: drivers/touch_dev_gestures: add gesture recognition for touch devices r=aabadie a=gschorcht

### Contribution description

This PR adds simple gesture recognition for touch devices accessed via the generic Touch Device API. It can be used in conjunction with device drivers that use either interrupts or polling mode. It supports up to two touches and the following gestures:
- Single and double tap at given position
- Long press and release given position
- Moving while pressed with current position
- Swipe left, right, up and down
- Zoom in (spread) and out (pinch)

Gesture recognition has been tested with:
- [x] `stm32f746g-disco` (works out of the box)
- [x] `stm32f723e-disco` (works out of the box)
- [x] `stm32f429i-disc1` (works on top of PR #19885)
- [x] `stm32l496g-disco` (works with my local LCD display changes waiting for PR #19825, not yet provided)
- [x] `esp32s3-wt32-sc01-plus` (new board, not yet provided)

### Testing procedure

Flash `tests/drivers/touch_dev_gestures` to a board with touch pane, for example:
```
BOARD=stm32f746g-disco make -j8 -C tests/drivers/touch_dev_gestures/ flash
```
PR #19885 is required for the `stm32f429i-disc1` board.

The output should look like this:
```
main(): This is RIOT! (Version: 2023.10-devel-121-g81c5c-drivers/touch_dev_gestures)
Single Tap X: 255, Y:154
Single Tap X: 253, Y:153
Double Tap X: 253, Y:149
Swipe right
Swipe down
Swipe left
Swipe up
Pressed    X: 257, Y:155
Moving     X: 257, Y:155
Moving     X: 257, Y:155
Moving     X: 259, Y:156
Moving     X: 262, Y:157
Moving     X: 266, Y:158
Moving     X: 269, Y:160
Moving     X: 273, Y:162
Moving     X: 276, Y:165
Moving     X: 278, Y:167
Moving     X: 278, Y:169
Moving     X: 278, Y:169
Released   X: 279, Y:172
```

### Issues/PRs references

Co-authored-by: Gunar Schorcht <[email protected]>
bors bot added a commit that referenced this pull request Sep 1, 2023
19884: drivers/touch_dev_gestures: add gesture recognition for touch devices r=aabadie a=gschorcht

### Contribution description

This PR adds simple gesture recognition for touch devices accessed via the generic Touch Device API. It can be used in conjunction with device drivers that use either interrupts or polling mode. It supports up to two touches and the following gestures:
- Single and double tap at given position
- Long press and release given position
- Moving while pressed with current position
- Swipe left, right, up and down
- Zoom in (spread) and out (pinch)

Gesture recognition has been tested with:
- [x] `stm32f746g-disco` (works out of the box)
- [x] `stm32f723e-disco` (works out of the box)
- [x] `stm32f429i-disc1` (works on top of PR #19885)
- [x] `stm32l496g-disco` (works with my local LCD display changes waiting for PR #19825, not yet provided)
- [x] `esp32s3-wt32-sc01-plus` (new board, not yet provided)

### Testing procedure

Flash `tests/drivers/touch_dev_gestures` to a board with touch pane, for example:
```
BOARD=stm32f746g-disco make -j8 -C tests/drivers/touch_dev_gestures/ flash
```
PR #19885 is required for the `stm32f429i-disc1` board.

The output should look like this:
```
main(): This is RIOT! (Version: 2023.10-devel-121-g81c5c-drivers/touch_dev_gestures)
Single Tap X: 255, Y:154
Single Tap X: 253, Y:153
Double Tap X: 253, Y:149
Swipe right
Swipe down
Swipe left
Swipe up
Pressed    X: 257, Y:155
Moving     X: 257, Y:155
Moving     X: 257, Y:155
Moving     X: 259, Y:156
Moving     X: 262, Y:157
Moving     X: 266, Y:158
Moving     X: 269, Y:160
Moving     X: 273, Y:162
Moving     X: 276, Y:165
Moving     X: 278, Y:167
Moving     X: 278, Y:169
Moving     X: 278, Y:169
Released   X: 279, Y:172
```

### Issues/PRs references

Co-authored-by: Gunar Schorcht <[email protected]>
bors bot added a commit that referenced this pull request Sep 1, 2023
19884: drivers/touch_dev_gestures: add gesture recognition for touch devices r=aabadie a=gschorcht

### Contribution description

This PR adds simple gesture recognition for touch devices accessed via the generic Touch Device API. It can be used in conjunction with device drivers that use either interrupts or polling mode. It supports up to two touches and the following gestures:
- Single and double tap at given position
- Long press and release given position
- Moving while pressed with current position
- Swipe left, right, up and down
- Zoom in (spread) and out (pinch)

Gesture recognition has been tested with:
- [x] `stm32f746g-disco` (works out of the box)
- [x] `stm32f723e-disco` (works out of the box)
- [x] `stm32f429i-disc1` (works on top of PR #19885)
- [x] `stm32l496g-disco` (works with my local LCD display changes waiting for PR #19825, not yet provided)
- [x] `esp32s3-wt32-sc01-plus` (new board, not yet provided)

### Testing procedure

Flash `tests/drivers/touch_dev_gestures` to a board with touch pane, for example:
```
BOARD=stm32f746g-disco make -j8 -C tests/drivers/touch_dev_gestures/ flash
```
PR #19885 is required for the `stm32f429i-disc1` board.

The output should look like this:
```
main(): This is RIOT! (Version: 2023.10-devel-121-g81c5c-drivers/touch_dev_gestures)
Single Tap X: 255, Y:154
Single Tap X: 253, Y:153
Double Tap X: 253, Y:149
Swipe right
Swipe down
Swipe left
Swipe up
Pressed    X: 257, Y:155
Moving     X: 257, Y:155
Moving     X: 257, Y:155
Moving     X: 259, Y:156
Moving     X: 262, Y:157
Moving     X: 266, Y:158
Moving     X: 269, Y:160
Moving     X: 273, Y:162
Moving     X: 276, Y:165
Moving     X: 278, Y:167
Moving     X: 278, Y:169
Moving     X: 278, Y:169
Released   X: 279, Y:172
```

### Issues/PRs references

Co-authored-by: Gunar Schorcht <[email protected]>
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.10 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants