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

Full Touch support for default Marlin Menus - Touch to Select! #18239

Merged

Conversation

rhapsodyv
Copy link
Sponsor Member

@rhapsodyv rhapsodyv commented Jun 9, 2020

Description

Hi,

Using a touch screen with default Marlin menu is a bit annoying because we need to use the buttons bellow the screen.
But it changes now :-)

I'm enabling it with TOUCH_BUTTONS. I can emulate a click too, but seems touch to select is better.

The change could be done just in the xpt2046.cpp file, but I think the "screen_click" function must be part of MarlinUI... Tried to keep the thinks in the right place.

Benefits

It's more easier and user friendly to touch directly where you want, in a TFT display that uses the default marlin menu.

Some users have touch displays that don't have any other UI. Link TronXY users using Marlin. So it helps this users to have a better UI experience.

This videos explains better :-)

ezgif-4-208ce3332154
ezgif-4-b166bfb54b8c

I think I must explain my code decisions in the PR:

I could do it a lot better, with more classes and stuff, supporting double-touch, touch on icons, edit screen, and so on.

But, my main goal here was:

keep changes at minimum, dont break any compatibility, yet supporting the most used touch actions for the users, to get this PR accepted as soon as possible without any risk to Marlin

So, with only 3 files and a few lines changed, we get the most action covered:

  • touch to select menus
  • touch to select confirm action
  • touch to go to next/prev pages

Why do I want to keep things so minimum now? Because the TFT users that don't have smart screens need it, as the case of tronxy x5sa users - we had working on this about a week or so.

I already working in a 2.0 version of this full touch, that each screen could define its touchable actions - icons, menus, edit, support double touch, etc. The code is better, but the change is a lot bigger. If I did the big version first, the tronxy x5sa users (and a lot of others users) will need to wait a long time to use it, because a more complex and big PR isnt so easy to merge without break anything - it needs more review, more tests, and so on...

Well, I think I could explain my design/code decisions to help it get accepted as soon as possible :-)
I have a few users testing it, and they are enjoying a lot!

Thanks!

…the user can touch DIRECTLY on the menu he wants to select!!

a) Touch to select menus.
b) In confirmation screen, lelt screen select left btn, right screen select right btn
@thinkyhead
Copy link
Member

Nice, this is a very welcome feature. I'm also working on making the native Marlin menu system work on DWIN screens with a standard DWIN_SET, so it will be great to have touch support for that from the start.

@rhapsodyv
Copy link
Sponsor Member Author

rhapsodyv commented Jun 9, 2020

Nice, this is a very welcome feature. I'm also working on making the native Marlin menu system work on DWIN screens with a standard DWIN_SET, so it will be great to have touch support for that from the start.

We also can make the status screen act if it has buttons. For example: touch on the cooler icon goes to fan speed control screen; touch on nozzle temperature goes to nozzle temp config screen, and so on..
I din't it right now, because we are focusing on Tronxy hardware first. But it will be nice :-)

@thinkyhead
Copy link
Member

Is this touch button device only able to report touch events and not un-touch events? If the device could tell us that the touch is being sustained, we could use that data to continually jog the edited value, with acceleration, until it is released.

@tpruvot
Copy link
Contributor

tpruvot commented Jun 9, 2020

there is a few small issues, but the idea is good : https://drive.google.com/file/d/1oIJs7qwAxI1L76X1XXeGwSEdX2_y98QZ/view?usp=sharing

(gdrive could take a few minutes to process the upload of the vid)

  • on status screen, edit the feedrate... maybe should avoid by default...
  • menu y scroll behavior seems reversed

@rhapsodyv
Copy link
Sponsor Member Author

there is a few small issues, but the idea is good : https://drive.google.com/file/d/1oIJs7qwAxI1L76X1XXeGwSEdX2_y98QZ/view?usp=sharing

(gdrive could take a few minutes to process the upload of the vid)

  • on status screen, edit the feedrate... maybe should avoid by default...
  • menu y scroll behavior seems reversed

Seems the video isnt public.

What display are you using?

@tpruvot
Copy link
Contributor

tpruvot commented Jun 9, 2020

ILI9341 (U20 config in marlin samples) video is public but in process, take some time on google drive

@thinkyhead
Copy link
Member

public but in process

The page asks visitors to provide their email address for an access link.

@rhapsodyv rhapsodyv closed this Jun 9, 2020
@rhapsodyv
Copy link
Sponsor Member Author

clicked Hahahaha wrong button!! "close comment"!!! dammit

@rhapsodyv rhapsodyv reopened this Jun 9, 2020
@tpruvot
Copy link
Contributor

tpruvot commented Jun 9, 2020

yep, its only 25MB but discord only allow 8... cant share it there rmm.. let me see where i can upload...

anyway, Y is reversed in menus, first item select the bottom line... and last one the first...

@rhapsodyv
Copy link
Sponsor Member Author

yep, its only 25MB but discord only allow 8... cant share it there rmm.. let me see where i can upload...

The bottom buttons are working correctly, is just the menu touch that is wrong? it's inverted?

@tpruvot
Copy link
Contributor

tpruvot commented Jun 9, 2020

yes... y seems correct else buttons should not work...

Marlin/src/lcd/ultralcd.cpp Outdated Show resolved Hide resolved
…he current touch assumes the screen is 320x240, not the LCD_FULL*, that can be upscaled.
@rhapsodyv
Copy link
Sponsor Member Author

@rhapsodyv, you might be interested in new implementation of TFT and touch screen support.
It uses full screen, has better fonts and graphics and touch interaction with displayed elements.
Support TFT screens with SPI and FSMC interfaces on STM32F1 and STM32F4 MCUs.
Pictures - #18129
Code - #18130

I have saw. Impressive work. I made it PR trying to change as little as possible, to keep compatibility and get soon to the official branch, because owners of TronxXY will need it together with others PR I have made.

@rhapsodyv
Copy link
Sponsor Member Author

yea good idea, and i was thinking... if Y < 32 scroll up (up button), if Y > last row... scroll down (down button)

I have done: row > LCD_HEIGHT / 2 => UP, row < LCD_HEIGHT / 2 => DOWN... I think its easier, because we dont have a UP/DOWN button draw on screen...

BUT, we could try put a discreet ^ (up) and / (down) arrows in the corners. But it will change more things.

@rhapsodyv
Copy link
Sponsor Member Author

nice @jmz52 but... may be useful to have both, until its fully functional...

Something to note related to 480x320 new displays... The ratio is not exactly the same as 320x240...
its 3:2 (1.5) vs 4:3 (1.33) so the buttons and dogm Y position need to be ajusted by different offsets to be more accurate. Here is what is needed with the 32px top offset on 320x240

-         row = y * (LCD_HEIGHT) / (BUTTON_AREA_TOP);
+         row = (y - LCD_PIXEL_OFFSET_Y) * (LCD_HEIGHT) / (BUTTON_AREA_TOP - LCD_PIXEL_OFFSET_Y);

maybe the horizontal separator could be skipped for the x3 upscale, which should ajust the buttons position

I will test and fix offset Y issues, thanks.

@jmz52
Copy link
Contributor

jmz52 commented Jun 10, 2020

@tpruvot 320x240 UI is functional, except for advanced pause screen (feature is not available on STM32) and UBL screen (lack of knowledge).
I've ordered two 480x320 screens but it may take a week or two until they are delivered.
SDIO can be used in slow but quite stable 1-bit mode @ 2.4 MHz clock.
This module still need some love, but I had no SDIO read errors with my test "prints". (dry runs as I am stuck with no access to my printer)

The most tricky part it to configure build environment for new board to build Marlin with HAL_STM32, but this is something I would be happy to help.
If you are willing to try new UI, please send me your configuration files, so I can configure a build environment for you.

@tpruvot
Copy link
Contributor

tpruvot commented Jun 10, 2020

my config is alfawise U20 sample in marlin, STM32F103VE_longer env.

seen good deals for the mks robin nano + tft 3.5 (20€ without drivers) on aliexpress a 103VET6 too

@nicedevil007
Copy link

nicedevil007 commented Jun 10, 2020

Can't compile it :(

Marlin\src\feature\touch\xpt2046.cpp:44:26: error: 'CS_PIN' was not declared in this scope; did you mean 'SS_PIN'?

BTT SKR 1.4 Turbo with BTT 3.5TFT V3.0

@jmz52
Copy link
Contributor

jmz52 commented Jun 10, 2020

@nicedevil007 this code is for STM32 boards with TFT screen connected via FSMC.
It is not designed for MKS or BTT Smart Screens.

@nicedevil007
Copy link

@nicedevil007 this code is for STM32 boards with TFT screen connected via FSMC.
It is not designed for MKS or BTT Smart Screens.

nooooooooooooooooooooooooooooo :D

@rhapsodyv
Copy link
Sponsor Member Author

@nicedevil007 this code is for STM32 boards with TFT screen connected via FSMC.
It is not designed for MKS or BTT Smart Screens.

This smart screens doest work with default marlin menus? I know sapphire pro tft might work. Maybe just define the TOUCH_BUTTONS pins

@jmz52
Copy link
Contributor

jmz52 commented Jun 10, 2020

Sapphire pro is based on MKS Robin Nano with means STM32 and dumb TFT screen on FSMC interface.
Smart TFTs are connected to mainboard via UART and are totally different devices with their own MCU and firmware.

@rhapsodyv
Copy link
Sponsor Member Author

@tpruvot take a look in the last commit. I have fixed the Y offset calc. I have tested with a big and small LCD_PIXEL_OFFSET_Y.
Can you test in your screen?

@le3tspeak
Copy link
Contributor

Is it possible to extend this by the following that if you want to change values eg the temperature of the nozzle when I tap the screen on the right for plus and left for minus.

And there is also an error at the moment where you can scroll in a menu, for example when changing the Nozzle temperature menu point it goes to the MAX value when I move my finger up and when I go down it is 0

@rhapsodyv
Copy link
Sponsor Member Author

Is it possible to extend this by the following that if you want to change values eg the temperature of the nozzle when I tap the screen on the right for plus and left for minus.

And there is also an error at the moment where you can scroll in a menu, for example when changing the Nozzle temperature menu point it goes to the MAX value when I move my finger up and when I go down it is 0

Yes, sure. Right now I didnt any handle for the edit screens. I need a way to identify it, but I dint want to make a lot of changes in the code. By now, just works:

  • menu select
  • confirm select
  • next/prev page in menus

I will try to find a way to identify edit screens without changing a lot of places. In my local repo, I even did the the confirmation screen buttons touchable.
But I think it will be for a new PR.

@thinkyhead what do you think?

@rhapsodyv
Copy link
Sponsor Member Author

I think I must explain my code decisions in the PR:

I could do it a lot better, with more classes and stuff, supporting double-touch, touch on icons, edit screen, and so on.

But, my main goal here was:

keep changes at minimum, dont break any compatibility, yet supporting the most used touch actions for the users, to get this PR accepted as soon as possible without any risk to Marlin

So, with only 3 files and a few lines changed, we get the most action covered:

  • touch to select menus
  • touch to select confirm action
  • touch to go to next/prev pages

Why do I want to keep things so minimum now? Because the TFT users that don't have smart screens need it, as the case of tronxy x5sa users - we had working on this about a week or so.

I already working in a 2.0 version of this full touch, that each screen could define its touchable actions - icons, menus, edit, support double touch, etc. The code is better, but the change is a lot bigger. If I did the big version first, the tronxy x5sa users (and a lot of others users) will need to wait a long time to use it, because a more complex and big PR isnt so easy to merge without break anything - it needs more review, more tests, and so on...

Well, I think I could explain my design/code decisions to help it get accepted as soon as possible :-)
I have a few users testing it, and they are enjoying a lot!

Thanks!

@thinkyhead
Copy link
Member

I will try to find a way to identify edit screens without changing a lot of places.

Add a bool flag called on_edit_screen to MarlinUI. Set this flag to true in MenuEditItemBase::goto_edit_screen and set it to false in MarlinUI::goto_previous_screen.

@thinkyhead thinkyhead merged commit c6f3511 into MarlinFirmware:bugfix-2.0.x Jun 11, 2020
@tpruvot
Copy link
Contributor

tpruvot commented Jun 12, 2020

changes are good, just a visual/design issue to fix with #18273

edit value touch is not yet really "usable" they tend to only do min/max values, but well, can help too

@dandantsui
Copy link
Contributor

Just tested this with my MKS_Robin with Robin_TFT28 and it is working well! The scrolling on the right side is very ingenious! Maybe it could have an icon or scrollbar of some sort?

@rhapsodyv
Copy link
Sponsor Member Author

Just tested this with my MKS_Robin with Robin_TFT28 and it is working well! The scrolling on the right side is very ingenious! Maybe it could have an icon or scrollbar of some sort?

Good to know! I'm working on better scroll and "touch space" for smaller screens (to fit the fingers!!).

But I just will send a PR when the smaller screen I ordered arrive. Currently I just have one 3.5" 480x320.

Some preview
Before:
image0

After:
image0-2

@dandantsui
Copy link
Contributor

Oops! I meant to say TFT32! Just an upscaled version of TFT28. Not sure why my stepper motors aren’t moving with the bugfix branch. Will have to resolve that before I can continue.

@dandantsui
Copy link
Contributor

Hmm steppers are working now. But have a glitchy TMC connection (Some axis saying no connection or all low, randomly). I think something was changed in bugfix that is causing intermittent single wire communication on TMC2209. Switching back to 2.0.5.3 with the same settings shows no problems. I will occasionally flash back to bugfix to test things out if you’d like!

@thinkyhead
Copy link
Member

glitchy TMC connection

If a bug report is filed with sufficient information then someone will look into it.

@dandantsui
Copy link
Contributor

@thinkyhead I’m really not sure what’s going on. The connection errors are very sporadic and randomly jump between the drivers. There is no set driver that reports connection error but rather random amount return that problem.

Is there any way I can provide more info besides the sporadic M122 response? I am more than happy to help diagnose this out.

jmp0x0000 pushed a commit to jmp0x0000/Marlin that referenced this pull request Aug 7, 2020
@rhapsodyv rhapsodyv deleted the fullscreen-touch-support branch October 25, 2020 00:33
HairingX pushed a commit to HairingX/Marlin that referenced this pull request Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants