New Assisted Tramming (and Wizard) + accompanying changes and fixes

[lukasradek]

Summary

Adds completely reworked Tramming Wizard and unifies it with G35 tramming. Tramming wizard supports from 3 to 9 (used to be 6) points, which is handy for large beds with 3x3 screw grids. Has more practical and intuitive menu system, supports milimeters and "screw turn" units.

Menu (loop) patch to handle STATIC_ITEM in other places then just "one on top as a menu header".

Adds screen that can show custom long (multiline with autowrapping) message.

Language changes to reflect all mentioned above. Some small fixes.

Description

Full description on per file basis of changes and reasoning behind them.

Marlin/src/lcd/menu/menu_tramming.cpp This is the Tramming Wizard (TW) implementation. Tramming wizard was a great idea but it was underbaked a little bit. It was functional but far from comfortably usable. New tramming wizard workflow and possibilities:

1. User opens TW and is prompted to select point to measure the reference Z value or select Z=0 for absolute measurement. The value is remembered, not the point! That is important and different from previous version. BEFORE: When you adjusted the whole bed and decided to remeasure the reference point, your reference height would automatically change (without notifying user), which would throw off all measurements you made on already adjusted screws and the reference point would still show zero. If you wanted to change reference point, you had to close and open TW again and loose all measurements. NOW: The reference height never changes unless user intentionally chooses to. Even if user measures the point he used for reference again, the reference height stays the same, therefore the reading will show how the height at that point changed. At anytime user is able to change the reference height just by selecting a point from which the height should be taken (or Z=0). No measurements are lost, they are just updated to reflect new reference height.
2. After the reference height is selected. Measuring menu is presented to the user. There is a list of all tramming point menu items. BEFORE: User had to first select a point, which would take him to point submenu. There he would have the option to measure a value or go back. NOW: User is able to measure each point just by clicking the corresponding menu item. Measured value is displayed directly in that menu item's title, aligned to the right. (That is why this PR needs that PR to be accepted). There is no need to go into submenus for measurement.
3. In the Measuring menu there are further options. (None of which were in the previous version). User can Measure All points at once (the printer does it sequentially of course 😉) User can Toggle between units - Milimeters / Screw turns (if TRAMMING_SCREW_THREAD was defined in config). User can Toggle menu mode - Measuring / Point detail mode User can Invoke Reference height measurement again
4. Point detail mode I have already described Measuring mode above. That is where you measure point by clicking menu items. When user selects Point detail mode, all tramming point menu items change from ACTION_ITEM to SUBMENU_ITEM. Now clicking on the item takes user into submenu for the specific tramming point. In tramming point submenu (point detail) user is presented with height difference in milimeters and screw turns. There is an option to measure and set this point's height as reference height.
5. Logic related change: I have switched the way milimeter difference is presented. Now it shows height differences in a more logical (cartesian) way. If point shows +1mm, it means it is 1mm above reference and vice versa. (It used to be the other way around).

Marlin\src\gcode\bedlevel\G35.cpp This is the implementation for G35 GCODE tramming. I have made some changes to improve the reported information. I have fixed a mistake... it used to output "Turn clockwise by 1 turn and 30 minutes". 1 full turn is 360 degrees, 1 degree is 60 minutes. Now the same instruction would output "... 1 turn and 180 degrees".

Marlin\Configuration_adv.h

1. Remove REPORT_TRAMMING_MM. Milimeters are universal and do not require any additional information (as opposed to screw turns, which require correct thread info). My tramming wizard and G35 show milimeters by default and screw turns only if user explicitly defines screw thread.
2. I have made it more clear what CW and CCW means. Yes... most of us know that (C)CW means (Counter)ClockWise, but what does it mean in the bed tramming context? Depending on the bed screw mechanism, tightening the screw can sometimes lower the bed, other time raise it. So now user is clearly instructed that CW/CCW is the direction of bed lowering. No more ambiguity.

Marlin\src\feature\tramming.h Marlin\src\feature\tramming.cpp Marlin\src\core\macros.h Those were modified to increase the maximum number of points from 6 to 9. The idea was mine, but the Macro Magician who made it look nice and concise was @thinkyhead . Thanks.

Marlin\src\lcd\language\language_xx.h Added some strings to show in the wizard and removed some from the old wizard.

Marlin\src\lcd\marlinui.h Marlin\src\lcd\menu\menu.cpp

1. There was a bug in MarlinUI::synchronize(...). Line 238: static FSTR_P sync_message = fmsg ?: GET_TEXT_F(MSG_MOVING); Since this was a static initialization, it was evaluated only the first time this function ran. Therefore any subsequent call showed the same message as the first one (or garbage). Noone probably run into this issue since it is not that frequent or important. But now it is fixed and I tied it nicely into my other UI function (which is new).
2. MarlinUI::goto_message_screen(str1, str2) I needed / wanted a way to inform user by a full screen message (which also conveniently prevents further input). I also wanted some flexibility, so this function can accept up to 2 string pointers, join them seemlessly and print them while wrapping the text to the screen. User (of this function) can also define linebreaks by inserting \n in the string. It has some limitations - prints from LCD_HEIGHT / 2 - 1 up to the end of the screen and does not give a damn about words (when wrapping), but well... it works for a reasonable input. While it would be a nice to have for my Tramming Wizard, it is not neccessary if it is deemed not worthy. It can be easily dropped from wizard's code.

Marlin\src\lcd\menu\menu_item.h Crucial bug fix: Original code considered STATIC_ITEMs to be used in menu screens only as a single line headline (at the very top). That raised some problems when I wanted to use STATIC_ITEM as a separator in menu and as a double line headline. The problem was that STATIC_ITEMS were able to jump your encoderLine ("selection cursor") underneath them (_skipStatic). That is so that when you open menu with STATIC_ITEM at the top, your encoderLine is on the first clickable item and not the STATIC_ITEM. That is fine... but STATIC_ITEM does not care if you are coming from above them or from the bottom.. they always skip you down. Meaning that if you found yourself under STATIC_ITEM it would not let you scroll above it even though it wasn't the top item in the menu. Very... selfish. Line351 && encoderTopLine == 0 ... this condition prevents skipping unless you are scrolled all the way up in the menu.

Less crutial (requires quite special circumstances), but still a bug fix: There was also a bug, when multiple STATIC_ITEMs in a row (or column to be exact) skipStaticed you to a line that was behind LCD_HEIGHT. In such case, the UI slept a bit on scrolling and it caused some problems with printing characters (it would print a few lines over themselves). Line352 && encoderLine < encoderTopLine+LCD_HEIGHT-1) ... this prevents skipping if it would skip you behind LCD_HEIGHT.

Benefits

1. Much more user friendly and capable Tramming Wizard and a bit more user friendly G35 (it wasn't that bad before and I just matched it's logic to TW).
2. Clearer Assisted Tramming setup in Configuration_adv.h
3. Assisted Tramming is now useful for 3x3 screw grid beds.
4. Ability to display custom long message which blocks user input.
5. Fixed stuck synchronization message.
6. Static items can now be used in interactive menus at any position.
7. Me being happy about my first Marlin contribution 😉.

Configurations

Assisted tramming and Wizard turned on.

Related Issues

[FR] More optimized ASSISTED_TRAMMING #24448

You can read all of the above or just look at pictures

(Sorry, I forgot to switch my -V flag off 😁)

Measure Mode vs Point Detail Mode & Milimeters vs Screw Turns

Point detail menu & Multiline message screen

[sbasmanov]

Fantastic! I'll test it later today when get home. Thank You so much.

[lukasradek]

@sbasmanov Crossing fingers you printer doesn't blow up 😁. Btw what screen do you have? There is a need for testing TFTGLCD and E3V2.

[sbasmanov]

@sbasmanov Crossing fingers you printer doesn't blow up 😁. Btw what screen do you have? There is a need for testing TFTGLCD and E3V2.

Everything works perfect! Leveling never been that easy :) I have Ender 3 Pro with stock (CR10) display. Couple notes:

1. When 'measure all' or moving between points, there is extra small lift of head. After measuring with bltouch it usually return to original height (probably it is somewhere in config, don't remember). So, probe goes down, up, then little more up, then to next point. Not sure if this 'extra up' can be configured somewhere.
2. Not sure if this is intended, but when cursor reach menu item with dashes (at bottom), cursor disappears at all. But UI thinks this is menu item and don't skip it. So, extra wheel click needed to see cursor again. Probably not an issue on touch screens :)
3. May be make config option to hide initial 'select reference point' menu? Something like DEFAULT_REFERENCE_POINT and if defined, don't show selection when wizard started (it's still possible to change ref point in menu later).

So, now I can say I'm happy :) Thank You very much. Hope this PR will be accepted soon and become part of marlin.

Oh, btw, saw one warning: Marlin/src/lcd/menu/menu_tramming.cpp:69:14: warning: 'float _mock_probe_at_point(xy_pos_t)' defined but not used [-Wunused-function] 69 | static float _mock_probe_at_point(const xy_pos_t pos) {

[lukasradek]

So nice to see it working somewhere else in the world 🙂.

1. I think that "extra up" is configurable. I think it is to protect the BLtouch. I haven't touched the motion code at all, so it is the same as it was before.
2. Selection disappears on the dashed line to indicate, that it is not an action item. The same goes for the top items in point detail menu. I am 50:50 on whether it should be indicated as selected or not.
3. I think there is no reason to hide "select reference point" menu. You have to make the first measurement anyway, so the menu instructs you to do it so that there is no confusion about what is the reference.
4. The warning is a non issue.

[sbasmanov]

I think it is to protect the BLtouch. I haven't touched the motion code at all, so it is the same as it was before.

Hmmm. I'll check configuration. On 2.1.2 release with old wizard it was moving straight to next point without this hop. May be something changed after release - I applied diff to config files with my changes, so may be missed some defaults.

I am 50:50 on whether it should be indicated as selected or not.

I think that non-action items should be skipped and cursor moved to next available action item for better user experience.

You have to make the first measurement anyway, so the menu instructs you to do it so that there is no confusion about what is the reference.

Agree. Except situation when Z=0 :) If it will be configurable (#define in config) it could do automatic measurement of pre-configured point on wizard start. May be someone always using same corner for leveling.

Anyway, this is not an issue, because wizard itself works very well.

[sbasmanov]

I think that "extra up" is configurable. I think it is to protect the BLtouch.

Ok, I see what's going on. Before, I had to exit menu, select point, enter menu, then measure. This is why I didn't notice that extra small hop between points. Now when it does 'measure all' it is noticeable. Especially because Z speed is different when doing this extra hop. So, what I found so far. After homing, it sets Z=10 + Zoffset. After probe_at_point it sets Z=5+Zoffset+measured height. It seems this behavior is controlled by 4 params:

#define Z_CLEARANCE_DEPLOY_PROBE   10 // Z Clearance for Deploy/Stow
#define Z_CLEARANCE_BETWEEN_PROBES  5 // Z Clearance between probe points
#define Z_CLEARANCE_MULTI_PROBE     5 // Z Clearance between multiple probes
//#define Z_AFTER_PROBING           5 // Z position after probing is done

Not sure if it's possible to return head to original Z, so all moves between points will be smooth. Also, may be I have different speeds for travel move and for probe move, so this is why I see that extra hop.

[thinkyhead]

The last few months have been focused on Input Shaping, Fixed-Time Motion, fixing a Linear Advance issue that appeared in 2.1.2, cleaning up probe behavior, and doing a major refactor of LCD pins … so I'm only now getting a chance to review feature additions and improvements like this one.

So, this PR also needs a git merge bugfix-2.1.x and reconciliation with the new SS_FULL style for aligning Label / Value items. As with #25619, I recommend reapplying this feature on top of the latest code rather than attempting the git merge directly. Then you can force-push the final result to this PR's branch to update the PR.

As there is some overlap with #25619, you might want to just combine those two PRs into a single PR.

[lukasradek]

Great Scott!! What a commit hurricane 😀. Thanks for looking into this @thinkyhead, you focus on technical features is well understood 🙂. Right now I am quite busy with other stuff, so it might take me a while to go throught this - I assume a few weeks.

[sbasmanov]

@lukasradek, is there any chance You will have time in near future to take a look at this? I'm using Your patch since day one, and I can say with 100% confidence, that this wizard is best leveling tool I ever tried. There are some new features and bug fixes in Marlin since April, but I won't move until Your patch will be available :)

[lukasradek]

Hello @sbasmanov , that is very nice to hear!

Unfortunately when I had time, Marlin devs were busy with other features and now when they responded, I am busy with my non-Marlin life 🙂. I definitely want to finish this though and I believe I will have more time to do that during July.

[sbasmanov]

@lukasradek , that's normal :) All of us have our own life and it is not always aligns with our hobby :) I just wanted to make sure You didn't forget about such great feature and some day this PR will be accepted. We're not in hurry, so July sounds great. Thank You.

[lukasradek]

@sbasmanov I think it might be 🙂. Especially since you praise it so much 😁. Btw... a little sneak peak of a future version.

[lukasradek]

I have separated (naming-wise) method that draws messaage on screen MarlinUI::draw_message_on_screen from method that draws select prompt. Now developer can use that method to just draw a message screen (without select prompt) and select prompts now use that method to draw its message without code duplication. And I have implemented that method for TFT_COLOR_UI as well since it previously used completely different logic to draw select screens.

My MarlinUI::goto_message_screen now uses that unified method ui.draw_message_on_screen and has the option to synchronize itself with planner. Meaning it should be possible to remove the separate MarlinUI::sychronize method which could handle only one string that would be force printed on one line regardless of the fact if it fits.

I have also removed some code duplication because different TFT resolutions had the same implementation of draw_select_screen in each resolution file.

It still need some refactor and cleanup.

[vlsi]

Here's an alternative implementation that uses a probe+buzzer so the user can have audio and visual feedback when tuning screws: https://github.com/MarlinFirmware/Marlin/blob/d56136f06cca075e801aabcff76bd207d4da349f/Marlin/src/lcd/menu/menu_bed_tramming.cpp#L270-L277

[thinkyhead]

The lcd_put_u8str methods were a bit more standardized recently so they behave more consistently across different display types. I'm sure I didn't merge everything quite perfectly with these string methods. Please fix up these methods starting from the state of bugfix-2.1.x and bringing in your changes in the proper manner to fit the current code.

[sbasmanov]

@vlsi

Second, I wonder if you considered an alternative option for adjusting the screws. What if the probe moves over the screw at exactly the height the probe should trigger. Then user could adjust the screw until the probe triggers. Unfortunately, BLTouch would need to re-deploy after triggering, so it would probably require pressing "hover probe again" so user could tune the screw again.

This is how another leveling wizard (don't remember its name) works. Biggest problem here with BLtouch is that it is always moving bed up. BLtouch can't trigger, when bed goes down. This ends up completely loosing screws. So, that's why I opened FR, which was perfectly implemented by Lukas. And after 9 months of use, I could say that this is definitely the best leveling wizard for BLTouch and beds with screws like Ender. Now I can see all 4 corners on screen and even adjust without re-probing every corner. This process takes less than minute and hundred times easier than it was before.

[lukasradek]

@vlsi As @sbasmanov mentioned, leveling by adjustment according to probe height is either PROBE_MANUALLY or AUTO_BED_LEVELING_MANUAL (or similar) in config, but that is leveling... not tramming. Even though in this case it sort of blends together.

[vlsi]

BLtouch can't trigger, when bed goes down

That is indeed unfortunate, however, the wizard could at least assist when the bed should tune up. In other words, it could say something like "move the bed up until BLTouch triggers" in case the bed is too low.

The wizard could measure four corners and then walk them in such an order so the bed is only moved up.

An inductive sensor can trigger both ways.

[lukasradek]

An inductive sensor can trigger both ways.

This is not entirely true. Inductive sensor can change state going both ways, but the "let go" distance is greater than the trigger distance. So the only viable way of doing this is to only instruct to adjust upwards, which can be reliably measured. Furthermore there is no way you can adjust downwards if you use NOZZLE_AS_PROBE 😄.

Anyway, I am willing to implement something like this but first I would like to see this merged in this "basic" form. I have put quite a lot of work into this already.

[vlsi]

I would like to see this merged in this "basic" form

Do you think you could fix utf-8 issues first? In the current form, the PR is completely unworkable in Russian locale: it causes "blank screens" (fixed in https://github.com/vlsi/reborn2-marlin/commit/7cc9fea2bd49d117e24f90096bb349fde0238a46 ) and it triggers board resets caused by utf-8 runaway.

[lukasradek]

Sure, I can. But I don't have time for this right now unfortunately. Hopefully December will do it.

[sbasmanov]

That is indeed unfortunate, however, the wizard could at least assist when the bed should tune up. In other words, it could say something like "move the bed up until BLTouch triggers" in case the bed is too low.

The wizard could measure four corners and then walk them in such an order so the bed is only moved up.

This is exactly how #define LEVEL_BED_CORNERS works when #define LEVEL_CORNERS_USE_PROBE is set. It measures all corners, selects highest corner as reference and then asks to move up all other corners until probe triggers. When screws become too loose, I have to tight all screws to some level and start again. Another problem here - springs have different tension when screw is too loose and it is very easy to break bed level if You touch it by accident. So, it's better to keep all springs at similar level (tension), somewhere in middle, so they work properly.