Probe offset wizard for color ui #19742
Conversation
|
CI is broken until #19723 get merged. |
rhapsodyv
force-pushed
the
fix-probe_offset_wizard_menu-for-color-ui
branch
from
99fe6b8 to
7d1e66a
Compare
| MENU_ITEM_ADDON_END(); | ||
| MENU_ITEM_ADDON_END(); | ||
| #else | ||
| SUBMENU_P(tmp, []{ _goto_manual_move_z(float(SHORT_MANUAL_Z_MOVE)); }); |
There was a problem hiding this comment.
You cannot pass a string in SRAM to this macro. It requires a PROGMEM string. This hack will not work on platforms where string storage is specially handled.
There was a problem hiding this comment.
I followed the same pattern of other places, like Marlin/src/lcd/menu/menu_motion.cpp, line 186. The SUBMENU_P will only run in hardware that support TFT_COLOR_UI.
There was a problem hiding this comment.
The issue is that we cannot have "free" calls to lcd_put_u8str, as it is specific for DOGM, and the menu system is used by color ui too.
There was a problem hiding this comment.
This usage of SUBMENU_P is not supported and it is not clear that this is a special exception for certain platforms. If this is going to be a common thing, there ought to be a SUBMENU_* that specifically takes an SRAM string. It can simply be an alias to the PROGMEM version on platforms that have no special PROGMEM accessors.
|
It appears from this that Color UI is not supporting menu add-ons generally. Menu item add-ons are needed to display data in ways that are not supported by the general menu item methods. If add-ons should be supplanted for Color UI (or other types of displays) it would be better to generalize the solution at the class level rather than add special cases for some types of displays in the generalized menu system. If this menu item represents something that should be more generally available, we could also add a new type of menu item to the system. |
Added to the list to make menu add-ons a class, that can be implemented by the ColorUI, as other menus. |
Description
Fix probe offset wizard menu for color ui
Benefits
Fix #19738
Related Issues
#19738