esphome icon indicating copy to clipboard operation
esphome copied to clipboard

Touchscreen component and driver fixes

Open nielsnl68 opened this issue 1 year ago • 22 comments

What does this implement/fix?

  • Fixed ft6336 strange behavier reported by @nagyrobi.

  • In some cases the touchscreen calibration was not setup correctly when display rotation was set. This introduces 2 fixes.

    1. It will take the native dimensions from the display.
    2. The option to set the calibration variables in the YAML file.
  • The Display has now native width and height public methods. and the private internal width and height is moved to the display class.

  • Making sure that per touchscreen driver the predefined calibration settings are not overriding the manual set values

  • remove the need to call touchscreen::get_width_(); and touchscreen::get_heigth_(); and introducing two properties instead which are loaded on setup.

  • Fixing issue where the on_release event's was repeatedly was fired after touch release. thanks to @clydebarrow

  • making sure that the on_update did give the right information about the state of the touches

    • state Bit 0 is set when there is a touch detect
    • state Bit 1 is set when touch is moving on the screen.
    • state Bit 2 is set when a touch release is detected
    • state Bit 7 is set when the calibration has not correctly set.
    • state returns 0 when there is no touch detected and the info is invalid.
  • implanted most of the changes that @clydebarrow has shown in #6049 (thanks)

  • making sure the display is running before touchscreen is setup

  • The XPT2024 will break with this PR. The calibration_*_min and calibration_*_max are now moved under calibration:

  • added a test special for touchscreen drivers.

touchscreen:
  - platform: XPT2024
    calibration:
      x_min: 280
      x_max: 3860
      y_min: 340
      y_max: 3860

Types of changes

  • [x] Bugfix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [x] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [x] Other

Related issue or feature (if applicable): fixes: https://github.com/esphome/issues/issues/5251 https://github.com/esphome/issues/issues/5265 https://github.com/esphome/issues/issues/5249 https://github.com/esphome/issues/issues/5441

Pull request in esphome-docs with documentation (if applicable): esphome/esphome-docs#3487

Test Environment

  • [x] ESP32
  • [ ] ESP32 IDF
  • [ ] ESP8266
  • [ ] RP2040
  • [ ] BK72xx
  • [ ] RTL87xx

Example entry for config.yaml:

touchscreen:
  - platform: XPT2024
    calibration:
      x_min: 280
      x_max: 3860
      y_min: 340
      y_max: 3860

Checklist:

  • [ ] The code change is tested and works locally.
  • [ ] Tests have been added to verify that the new code works (under tests/ folder).

If user exposed functionality or configuration variables are added/changed:

nielsnl68 avatar Dec 22 '23 17:12 nielsnl68

Hey there @gpambrozio, mind taking a look at this pull request as it has been labeled with an integration (ft63x6) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

probot-esphome[bot] avatar Dec 22 '23 17:12 probot-esphome[bot]

Hey there @clydebarrow, mind taking a look at this pull request as it has been labeled with an integration (ft5x06) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

probot-esphome[bot] avatar Dec 22 '23 17:12 probot-esphome[bot]

Hey there @jesserockz, mind taking a look at this pull request as it has been labeled with an integration (media_player) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

probot-esphome[bot] avatar Dec 22 '23 17:12 probot-esphome[bot]

Hey there @clydebarrow, @jesserockz, mind taking a look at this pull request as it has been labeled with an integration (gt911) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

probot-esphome[bot] avatar Dec 22 '23 17:12 probot-esphome[bot]

Hey there @kroimon, mind taking a look at this pull request as it has been labeled with an integration (tt21100) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

probot-esphome[bot] avatar Dec 22 '23 17:12 probot-esphome[bot]

Hey there @jesserockz, mind taking a look at this pull request as it has been labeled with an integration (touchscreen) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

probot-esphome[bot] avatar Dec 22 '23 17:12 probot-esphome[bot]

Looks like #5999 and this are doing the same thing.

clydebarrow avatar Dec 22 '23 22:12 clydebarrow

Looks like #5999 and this are doing the same thing.

There should be a difference, @guillempages made an attempt to allow to use the transform settings from the display and use that for the touchscreen.

And used this code as starting point, before i fixed a lot of issues. Which you are reporting.

nielsnl68 avatar Dec 23 '23 00:12 nielsnl68

Split the tt21100 fix into a separate PR and I can approve it straight away.

clydebarrow avatar Dec 23 '23 19:12 clydebarrow

Trying on a Lanbon L8. Intention is to use with LVGL (migrate from OpenHASP).

  - source: github://pr#6049
    refresh: 1440min
    components: [ touchscreen ]
  - source: github://pr#5997
    refresh: 1440min
    components: [ touchscreen, ft63x6 ]
  - source: github://pr#6055
    refresh: 1440min
    components: [ ft63x6 ]

touchscreen:
  - platform: ft63x6
    id: tft_touch
    display: tft_display
    calibration:
#      x_min: 0
      x_max: 240
#      y_min: 0
      y_max: 320
    on_touch:
      - lambda: |-
            ESP_LOGI("cal", "x=%d, y=%d, x_raw=%d, y_raw=%0d",
                touch.x,
                touch.y,
                touch.x_raw,
                touch.y_raw
                );

Results in:

src/esphome/components/touchscreen/touchscreen.cpp: In member function 'virtual void esphome::touchscreen::Touchscreen::call_setup()':
src/esphome/components/touchscreen/touchscreen.cpp:20:44: error: 'class esphome::display::Display' has no member named 'get_native_width'
   20 |     this->display_width_ = this->display_->get_native_width();
      |                                            ^~~~~~~~~~~~~~~~
src/esphome/components/touchscreen/touchscreen.cpp:21:45: error: 'class esphome::display::Display' has no member named 'get_native_height'
   21 |     this->display_height_ = this->display_->get_native_height();
      |                                             ^~~~~~~~~~~~~~~~~

Otherwise the device works fine, if I ignore the PRs related to ft63x6, remove calibration config and change straight in its code

  this->x_raw_max_ = 240;
  this->y_raw_max_ = 320;

compiles and works correctly.

nagyrobi avatar Jan 07 '24 16:01 nagyrobi

@nagyrobi to fix your issue you need to change the external_component:

external_component:
  - source: github://pr#5997
    refresh: 1440min
    components: [ display, touchscreen, ft63x6 ]

nielsnl68 avatar Jan 08 '24 00:01 nielsnl68

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:

Learn more about our pull request process.

esphome[bot] avatar Jan 08 '24 04:01 esphome[bot]

Hey there @kroimon, mind taking a look at this pull request as it has been labeled with an integration (tt21100) you are listed as a code owner for? Thanks! Hey there @gpambrozio, mind taking a look at this pull request as it has been labeled with an integration (ft63x6) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

probot-esphome[bot] avatar Jan 09 '24 00:01 probot-esphome[bot]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 53.81%. Comparing base (4d8b5ed) to head (750f101). Report is 31 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #5997      +/-   ##
==========================================
+ Coverage   53.70%   53.81%   +0.10%     
==========================================
  Files          50       50              
  Lines        9408     9438      +30     
  Branches     1654     1660       +6     
==========================================
+ Hits         5053     5079      +26     
+ Misses       4056     4052       -4     
- Partials      299      307       +8     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov-commenter avatar Jan 15 '24 01:01 codecov-commenter

Hey there @kroimon, mind taking a look at this pull request as it has been labeled with an integration (tt21100) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

probot-esphome[bot] avatar Jan 18 '24 12:01 probot-esphome[bot]

  • The red flag is because i made this PR useable for current version ESPHome users. You should be able to fix most of the issues with you touchscreen with this PR. Add the following part in your YAML file.
external_components:
  - source: github://pr#5997
    components: [ display, touchscreen, YourTouchScreen]
    refresh: 0sec

Replace "YourTouchScreen" with the name of the used touchscreen.

nielsnl68 avatar Jan 28 '24 10:01 nielsnl68

Hey there @gpambrozio, mind taking a look at this pull request as it has been labeled with an integration (ft63x6) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

probot-esphome[bot] avatar Feb 10 '24 21:02 probot-esphome[bot]

This PR works nicely for me on Lanbon L8 with ft63x6.

nagyrobi avatar Feb 11 '24 21:02 nagyrobi

The changes to display relating to anti-aliased fonts should be removed from this PR and left for #6198.

I have remove those.

nielsnl68 avatar Feb 13 '24 15:02 nielsnl68

Hey there @gpambrozio, mind taking a look at this pull request as it has been labeled with an integration (ft63x6) you are listed as a code owner for? Thanks! Hey there @kroimon, mind taking a look at this pull request as it has been labeled with an integration (tt21100) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

probot-esphome[bot] avatar Feb 21 '24 01:02 probot-esphome[bot]

@clydebarrow Could you please approve this PR, thanks.

nielsnl68 avatar Feb 21 '24 01:02 nielsnl68

Hey there @gpambrozio, mind taking a look at this pull request as it has been labeled with an integration (ft63x6) you are listed as a code owner for? Thanks! Hey there @kroimon, mind taking a look at this pull request as it has been labeled with an integration (tt21100) you are listed as a code owner for? Thanks! (message by CodeOwnersMention)

probot-esphome[bot] avatar Feb 21 '24 03:02 probot-esphome[bot]