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.

Sign up for GitHub

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

Jump to bottom

samples: drivers: added a simple touch controller sample #77227

Merged
merged 3 commits into from
Oct 9, 2024
Merged

samples: drivers: added a simple touch controller sample #77227

merged 3 commits into from
Oct 9, 2024

Conversation

fkokosinski
Copy link
Member

This PR adds a sample for the touch controller. The sample is supposed to help examine issues when working with touchscreens. It draws a plus in the last touched position.

Can be tested natively with:

west build -p -b native_posix/native/64 samples/drivers/touch -t run

@fkokosinski fkokosinski force-pushed the touch-sample branch 3 times, most recently from 46b12b8 to b933835 Compare August 19, 2024 11:30
@fkokosinski fkokosinski marked this pull request as ready for review August 19, 2024 11:49
@zephyrbot zephyrbot added the area: Samples Samples label Aug 19, 2024
@fkokosinski fkokosinski force-pushed the touch-sample branch 2 times, most recently from 5657949 to 15d0c29 Compare August 19, 2024 12:41
@kartben
Copy link
Collaborator

kartben commented Aug 19, 2024 •
edited
Loading

Thanks for the PR @fkokosinski!
FYI we already have a similar pending PR #67508 - while LVGL is slightly more heavyweight, it probably makes sense to converge to having only one sample?
cc @faxe1008 @MrMarteng

@faxe1008
Copy link
Collaborator

@kartben I would go for the LVGL one and try to debulk it some more to make it easier for lower end targets.

@fkokosinski
Copy link
Member Author

Hey @kartben @faxe1008, thanks for your comments.

Ideally, we'd like this demo to work on smaller targets as well, where e.g. there's not enough space for LVGL. Plus, we think that having a kind of "minimal" demo with no additional library overhead can be beneficial when working on a new input driver, or debugging an old one. So we'd opt for keeping this demo separate from the one you've linked.

Let me know what you guys think. Thanks

@fkokosinski
Copy link
Member Author

Hey @kartben, let me know if there's something blocking us from moving forward with this PR. Thanks!

gmarull
gmarull previously requested changes Sep 9, 2024
View reviewed changes
samples/drivers/touch/src/main.c Outdated Show resolved Hide resolved
samples/drivers/touch/src/main.c Outdated Show resolved Hide resolved
samples/drivers/touch/src/main.c Outdated Show resolved Hide resolved
faxe1008
faxe1008 reviewed Sep 9, 2024
View reviewed changes
samples/drivers/touch/src/main.c Outdated Show resolved Hide resolved
samples/drivers/touch/src/main.c Outdated Show resolved Hide resolved
samples/drivers/touch/src/main.c Outdated Show resolved Hide resolved
samples/drivers/touch/src/main.c Outdated Show resolved Hide resolved
samples/drivers/touch/src/main.c Outdated Show resolved Hide resolved
samples/drivers/touch/src/main.c Outdated Show resolved Hide resolved
fabiobaltieri
fabiobaltieri requested changes Sep 9, 2024
View reviewed changes
samples/drivers/touch/boards/native_posix_64.overlay Outdated
Comment on lines 3 to 4
zephyr,display = &sdl_dc;
zephyr,touch = &input_sdl_touch;
Copy link
Member

Choose a reason for hiding this comment

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

the first is already in boards/native/native_sim/native_sim.dts, it should not be needed here, the second should probably go in the board file, @kartben what do you think?

samples/drivers/touch/boards/stm32f429i_disc1.overlay Outdated Show resolved Hide resolved
samples/drivers/touch/src/main.c Outdated Show resolved Hide resolved
samples/drivers/touch/src/main.c Outdated Show resolved Hide resolved
samples/drivers/touch/src/main.c Outdated Show resolved Hide resolved
@zephyrbot zephyrbot added area: native port Host native arch port (native_sim) platform: STM32 ST Micro STM32 labels Sep 10, 2024
@zephyrbot zephyrbot added platform: NXP NXP platform: Ezurio area: Shields Shields (add-on boards) platform: Renesas SmartBond Renesas Electronics Corporation, SmartBond area: Input Input Subsystem and Drivers labels Sep 27, 2024
faxe1008
faxe1008 previously approved these changes Sep 29, 2024
View reviewed changes
Copy link
Collaborator

@faxe1008 faxe1008 left a comment

Choose a reason for hiding this comment

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

Looks very good. Only thing from my side: iirc there is a doc file which explains the different chosen nodes used in zephyr upstream. An entry there would be also good.

@fkokosinski fkokosinski force-pushed the touch-sample branch 2 times, most recently from 6cd4b55 to 2c62c0e Compare September 30, 2024 10:09
faxe1008
faxe1008 previously approved these changes Sep 30, 2024
View reviewed changes
kartben
kartben approved these changes Sep 30, 2024
View reviewed changes
Copy link
Collaborator

@kartben kartben left a comment

Choose a reason for hiding this comment

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

Great work, thanks!
@fkokosinski in the future please refrain from marking comments as resolved yourself - it's rather inconvenient to have to go through all closed conversations to check and, more importantly, it's up to the original reviewer to confirm their recommendations have been properly addressed. https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#workflow-suggestions-that-help-reviewers
Thanks!

@fkokosinski
Copy link
Member Author

Great work, thanks! @fkokosinski in the future please refrain from marking comments as resolved yourself - it's rather inconvenient to have to go through all closed conversations to check and, more importantly, it's up to the original reviewer to confirm their recommendations have been properly addressed. https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#workflow-suggestions-that-help-reviewers Thanks!

Right, sorry about that. I'll keep this in mind in the future :)

@fkokosinski
Copy link
Member Author

The CI failure seems to be unrelated. On 8fda052 (main) I seem to be getting the exact same error when running west build -p -b esp32s3_touch_lcd_1_28/esp32s3/appcpu tests/lib/c_lib/common -T libraries.libc.common.

@fkokosinski fkokosinski force-pushed the touch-sample branch 2 times, most recently from de4ced4 to 25158c3 Compare October 7, 2024 08:59
@laudominik @fkokosinski
samples: drivers: added a simple touch controller sample
3440128 
The sample is supposed to help examine the issues with touchscreen.
It draws a plus in last touched position.

Signed-off-by: Dominik Lau <dlau@internships.antmicro.com>
Signed-off-by: Filip Kokosinski <fkokosinski@antmicro.com>
@laudominik @fkokosinski
boards: added touch controller to the /chosen node
01c7d6f 
This adds zephyr,touch property to boards with touch controllers, analogous
to `zephyr,display`.

Signed-off-by: Dominik Lau <dlau@internships.antmicro.com>
Signed-off-by: Filip Kokosinski <fkokosinski@antmicro.com>
@laudominik @fkokosinski
boards: stm32f429i_disc1: inverted touch controller axes
bb062b0 
This inverts x and y axes as reported by stmpe811 driver for stm32f429i.

Signed-off-by: Dominik Lau <dlau@internships.antmicro.com>
Signed-off-by: Filip Kokosinski <fkokosinski@antmicro.com>
@fkokosinski
Copy link
Member Author

Rebased due to failing CI.

@fabiobaltieri fabiobaltieri merged commit c344cca into zephyrproject-rtos:main Oct 9, 2024
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aescolar aescolar aescolar left review comments

@gmarull gmarull gmarull left review comments

@faxe1008 faxe1008 faxe1008 approved these changes

@fabiobaltieri fabiobaltieri fabiobaltieri approved these changes

@kartben kartben kartben approved these changes

@nashif nashif Awaiting requested review from nashif

@erwango erwango Awaiting requested review from erwango

@FRASTM FRASTM Awaiting requested review from FRASTM

@gautierg-st gautierg-st Awaiting requested review from gautierg-st

@GeorgeCGV GeorgeCGV Awaiting requested review from GeorgeCGV

@marwaiehm-st marwaiehm-st Awaiting requested review from marwaiehm-st

@mathieuchopstm mathieuchopstm Awaiting requested review from mathieuchopstm

@avisconti avisconti Awaiting requested review from avisconti

@jfischer-no jfischer-no Awaiting requested review from jfischer-no

Assignees

@kartben kartben

Labels
area: Input Input Subsystem and Drivers area: native port Host native arch port (native_sim) area: Samples Samples area: Shields Shields (add-on boards) platform: Ezurio platform: NXP NXP platform: Renesas SmartBond Renesas Electronics Corporation, SmartBond platform: STM32 ST Micro STM32
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@fkokosinski @kartben @faxe1008 @fabiobaltieri @gmarull @aescolar @zephyrbot @laudominik