Color UI remaining time

[mjbogusz]

Description

Add the possibility of displaying remaining print time with TFT_COLOR_UI.

The time remaining logic was copied from DOGM code, as it seemed the most complete - either taking time from M73 or extrapolating time elapsed by print progress.

Additionally the display items (in particular the feed/flow rate) are slightly rearranged to make room for elapsed/remaining time icons.

Requirements

Requires an LCD display.

Benefits

Fixes #19780, improves the display usability/readability.

Configurations

I'm testing this with MKS Robin Nano 1.2 and its 3.5" 480x320 LCD.

The important bits are:
Configuration.h:

// Line 3125
#define TFT_COLOR_UI

Configuration_adv.h:

// Line 1442
#define SHOW_REMAINING_TIME

Related Issues

Fixes #19780.

TODO before exiting WIP status:

• Implement for 320x240, 1024x600 displays
• Add icons for time elapsed/remaining
• Add unit tests
• Rebase on top of ✨♻️ Big progress display revision #24767 (once it's merged)
• Fix bugs/errors

[mjbogusz]

Current status: very much a draft/WIP.
The TODO list to be completed before becoming not-WIP:

• Implement for 320x240, 1024x600 displays
• Add icons for time elapsed/remaining
• Add unit tests
• Fix bugs/errors, obviously

[EvilGremlin]

See #24767 - it's going to be merged soon, so better base your code on that.

[mjbogusz]

See #24767 - it's going to be merged soon, so better base your code on that.

I've seen it and I'll gladly rebase once it gets merged.
Luckily it doesn't modify the files I'm changing here so I hope it'll be mostly just switching some conditionals around.

---

Right now this PR results in this layout (once with an estimated time remaining available, one without):

[mjbogusz]

I've addressed the comment on the commit - if possible, I'd like to ask to comment on code within the PR's context and not the commit's, because as it is now the comment (and response) is not visible in the discussion here.

Tests/builds are broken - as for everyone on bugfix-2.1.x - due to the example configs being already updated/merged for #24767 but not the code.
I must say I have at best mixed feelings about such a workflow around breaking changes, but that's a topic for another time and place.

[mjbogusz]

I've added icons for time elapsed and remaining. For now only the 32px bitmaps and classes were added - is there a place where I could upload the entire GIMP project files (128x128, with layers, paths and all that stuff) without littering the repo?

The UI with the icons looks like this:

[EvilGremlin]

Yeah, it's very annoying when you have to rename things in conf... i vote for ditching example confs dependency and putting everything in tests.
Constants - it's not really a rule, more of a general convenience to keep all compile-time stuff in preprocessor, for ease of maintenance etc. xy being exception in color_ui because there's just so much of it.
BTW - here's script for 4bit images https://github.com/EvilGremlin/Marlin/blob/newcolor_ui/Marlin/src/lcd/tft_newcolor/convert.py
pic source - put in same place they are now, but better redo them in Inkscape

[mjbogusz]

Yeah, it's very annoying when you have to rename things in conf... i vote for ditching example confs dependency and putting everything in tests.

Agreed - tests belong with the code and if the example configs are tests, then they belong here too.
Not only should it be easier for config key changes but it would also drop the implicit dependency on internet connection for running tests - I was quite surprised when I've stumbled upon it in the use_example_config script.

Constants - it's not really a rule, more of a general convenience to keep all compile-time stuff in preprocessor, for ease of maintenance etc. xy being exception in color_ui because there's just so much of it.

I see the point being to have it solved by the compiler, though I'd assume a single-define-single-use variable should be optimized away anyway. x/y variables are not and cannot be constant, at least in most of the code I've touched (e.g. y specifically is incremented).

BTW - here's script for 4bit images https://github.com/EvilGremlin/Marlin/blob/newcolor_ui/Marlin/src/lcd/tft_newcolor/convert.py pic source - put in same place they are now, but better redo them in Inkscape

Now this is a script I've been missing! The previous version was a result of a crude modification of the bmp2hex script, but this will save me some time. I think it'd be great for future contributors if it was available in buildroot/bin with at least a short description/mention in the contribution guide.

I'll redo the pics in Inkscape and correct the "shadow" while at that as it was slightly inconsistent with the existing ones.
Are there by any chance sources of the existing icons available anywhere?

[mjbogusz]

I've added new icons redone in Inkscape, including the svg project files.

The conversion script was... a bit rough on the edges - I've modified it quite a bit: https://github.com/mjbogusz/Marlin/blob/bitmap2cpp/buildroot/bin/bitmap2cpp

[mjbogusz]

I've rebased onto the updated bugfix branch (after #24767 merge) and updated the code to utilize the new flags.

[EvilGremlin]

👍
i feel bitmap2cpp is way too generic name for what it is

[mjbogusz]

320x240 version added, however I have no such display to test it.

[EvilGremlin]

just use simulator...

[mjbogusz]

Oooh that'll help for sure, thanks.

Wish it was described in the development docs - I'm definietely gonna put a PR for the documentation after finishing this PR with this and my other findings.
The simulator in particular is effectively undiscoverable - after reading your message I duckduckwent for "marlin firmware simulator" and it turned up nothing and google only found a single Twitter thread.

Anyway, back to working on the 1024x600 version.

[EvilGremlin]

native.ini have almost all info you need

[mjbogusz]

Rebased onto updated bugfix-2.1.x again.

Quick notes on setting up the simulator on WSL before I forget:

• Setup VcXsrv, add required things into your bashrc/zshrc
• source ~/.platformio/penv/bin/activate
• buildroot/bin/use_example_configs "Simulator"
• sudo apt install libglm-dev libsdl2-dev libsdl2-net-dev
• export LIBGL_ALWAYS_INDIRECT=0
• open .pio/libdeps/simulator_linux_debug/MarlinSimUI/src/MarlinSimulator/window.cpp, comment out lines 39-40 (setting multisampling attributes); if the file doesn't exist try building the binary once to let PIO download the deps
• platformio run --environment simulator_linux_debug
  • or, if building via VSCode:
    • open the PlatformIO environment selector and select simulator_linux_debug or simulator_linux_release (NOT linux_native!)
    • build
• .pio/build/simulator_linux_debug/MarlinSimulator should launch the simulator!

Now, for simulating/testing the ColorUI:

• enable TFT_COLOR_UI, disable REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER in Configuration.h (lines 45/48)
• [optionally] adjust the resolution in lines 3144-3147 of Configuration.h
• apply some random patches:
  • define TOUCH_MISO_PIN, TOUCH_MOSI_PIN, TOUCH_SCK_PIN in Marlin/src/HAL/NATIVE_SIM/spi_pins.h
  • remove/comment out void MarlinUI::rotate_progress() in Marlin/src/lcd/marlinui.cpp (lines 1731-1745)
• For touch screen buttons:
  • enable TOUCH_SCREEN (Configuration.h, line 3194)
  • disable TOUCH_SCREEN_CALIBRATION (Configuration.h, line 3202)

I'll try to open PRs with the fixes/adjustments mentioned above, but first I want to finish this one - it proved to be much bigger than I thought it'll be ;)

[mjbogusz]

320x240 layout with everything enabled:

[thinkyhead]

Hmm I thought perhaps some of these changes were a little strange. We'll need to revert some of them to deal with layout breakage. Rather than revert all changes I'm sure we can just cherry-pick to fix these.

CC: @mjbogusz / @uncellon

[uncellon]

• missed settings and SD card button for touch screen
• the second image shows that new label doesn't fits on the screen with SHOW_REMAINING_TIME flag

Tested on TS35 with a resolution of 320x480.

upd.: attached config

upd 2.: attached correct config

Configuration.zip

[uncellon]

I checked the code briefly, the settings and the SD card buttons are simply cut out in the ui_480_320.cpp file.

[ellensp]

@uncellon provided configs seem to be firmware stock ramps configs, please try again

[uncellon]

@ellensp, sorry, I didn't notice that I attached the wrong archive. I updated the post and attached the correct archive

[thinkyhead]

Thanks for having a look at the layout issues and working towards a patch. I've been all over the place lately with issues of many different flavors. I was going to check the state of the simulator and see how well it handles these different sizes of TFT UI since that would make debugging and fixing these kinds of things go a lot quicker.

[lukasradek]

Same here... I am using landscape 320x240 with the current bugfix and the SD card and Settings buttons are missing. That includes the touch control areas.

[ellensp]

add_control(SETTINGS_X, y, menu_main, imgSettings); is missing form 480x320 so no setting icon

as a quick hack, this will bring it back

diff --git a/Marlin/src/lcd/tft/ui_480x320.cpp b/Marlin/src/lcd/tft/ui_480x320.cpp
index 8e554cf413..2e41683c85 100644
--- a/Marlin/src/lcd/tft/ui_480x320.cpp
+++ b/Marlin/src/lcd/tft/ui_480x320.cpp
@@ -340,6 +340,10 @@ void MarlinUI::draw_status_screen() {
     TERN_(TOUCH_SCREEN, touch.add_control(FLOWRATE, x, y, component_width, 32, active_extruder));
   #endif
 
+  #if ENABLED(TOUCH_SCREEN)
+    add_control(SETTINGS_X, y, menu_main, imgSettings);
+  #endif
+
   y += TERN(HAS_UI_480x272, 36, 44);
 
   const progress_t progress = TERN(HAS_PRINT_PROGRESS_PERMYRIAD, get_progress_permyriad, get_progress_percent)();

Issue: #26064

[vovodroid]

Hi, do we really need to show seconds? They just take screen space and switching between one and two digits jerking the whole time string.

[mjbogusz]

Good point.
I think seconds are only relevant in the last stages of a print, so it would make sense to skip them if there's more than an hour left (or elapsed). Similarly, to avoid switching number of digits/characters displayed perhaps the minutes number should be displayed all the time, padded with zeros.

How do you feel about displaying 01h23m if the time is more than an hour and 00m59s when it's less?

[lukasradek]

Seconds are irrelevant for 3D printing regardless of the time left. Since there is so much preprocess / postprocess to be done and prints usually take hours rather than units of minutes, it doesn't really matter if you arrive +- a minute at your printer. 😁 Furthermore the estimate usually is not that precise.

[ellensp]

I disagree, some of us are impatient and want to see the print finish and want to see the seconds count down.

I like the idea of switching scale

[vovodroid]

some of us are impatient and want to see the print finish and want to see the seconds count down.

Ok, we can show seconds only for last minute. But I don't see too much value for seconds.

[vovodroid]

How do you feel about displaying 01h23m if the time is more than an hour and 00m59s when it's less?

I feel very good)))

[lukasradek]

@ellensp Well, that is just an OCD 😁. If you want to see the print finished, what difference does it make if you arrive a minute sooner? If you really want to see it finish, you should do that anyway since the estimate is not seconds-precise.

[mjbogusz]

I mostly agree; however for example I just love watching the finishing touches on the last layer or two ;) speedprinting people would probably prefer the seconds too - however I must acknowledge that both of these are minor edge cases.

Displaying 01m58s instead of 00h01m is just, in my opinion, a bit more elegant.
Also, even if the slicer didn't insert M73, the estimate should be fairly precise towards the end of a print.

[vovodroid]

Is https://github.com/MarlinFirmware/Marlin/blob/bugfix-2.1.x/Marlin/src/libs/duration_t.h#L130 the only function involved in remaining time? If it also affects elapsed time, I don't think it would be a problem.

And I don't see any usage of toCompactString

[mjbogusz]

This PR used toCompactString() for the remaining/elapsed time if both were enabled - it was added to avoid breaking other places where toString() was used. This has changed later when the separate per resolution files were merged and cleaned up in #26077.

The implementation of toCompactString() could be replaced with the "two most significant units" as described above and the method used for time elapsed (https://github.com/MarlinFirmware/Marlin/blob/bugfix-2.1.x/Marlin/src/lcd/tft/ui_color_ui.cpp#L339) and remaining (https://github.com/MarlinFirmware/Marlin/blob/bugfix-2.1.x/Marlin/src/lcd/tft/ui_color_ui.cpp#L365)