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

Probe offset wizard for color ui #19742

Merged
merged 1 commit into from
Oct 15, 2020
Merged

Probe offset wizard for color ui #19742

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions Marlin/src/lcd/menu/menu_probe_offset.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,20 @@ void probe_offset_wizard_menu() {
SUBMENU(MSG_MOVE_01MM, []{ _goto_manual_move_z( 0.1f); });

if ((SHORT_MANUAL_Z_MOVE) > 0.0f && (SHORT_MANUAL_Z_MOVE) < 0.1f) {
extern const char NUL_STR[];
SUBMENU_P(NUL_STR, []{ _goto_manual_move_z(float(SHORT_MANUAL_Z_MOVE)); });
MENU_ITEM_ADDON_START(0 + ENABLED(HAS_MARLINUI_HD44780));
char tmp[20], numstr[10];
// Determine digits needed right of decimal
const uint8_t digs = !UNEAR_ZERO((SHORT_MANUAL_Z_MOVE) * 1000 - int((SHORT_MANUAL_Z_MOVE) * 1000)) ? 4 :
!UNEAR_ZERO((SHORT_MANUAL_Z_MOVE) * 100 - int((SHORT_MANUAL_Z_MOVE) * 100)) ? 3 : 2;
sprintf_P(tmp, GET_TEXT(MSG_MOVE_Z_DIST), dtostrf(SHORT_MANUAL_Z_MOVE, 1, digs, numstr));
char tmp[20], numstr[10];
// Determine digits needed right of decimal
const uint8_t digs = !UNEAR_ZERO((SHORT_MANUAL_Z_MOVE) * 1000 - int((SHORT_MANUAL_Z_MOVE) * 1000)) ? 4 :
!UNEAR_ZERO((SHORT_MANUAL_Z_MOVE) * 100 - int((SHORT_MANUAL_Z_MOVE) * 100)) ? 3 : 2;
sprintf_P(tmp, GET_TEXT(MSG_MOVE_Z_DIST), dtostrf(SHORT_MANUAL_Z_MOVE, 1, digs, numstr));
#if DISABLED(HAS_GRAPHICAL_TFT)
extern const char NUL_STR[];
SUBMENU_P(NUL_STR, []{ _goto_manual_move_z(float(SHORT_MANUAL_Z_MOVE)); });
MENU_ITEM_ADDON_START(0 + ENABLED(HAS_MARLINUI_HD44780));
lcd_put_u8str(tmp);
MENU_ITEM_ADDON_END();
MENU_ITEM_ADDON_END();
#else
SUBMENU_P(tmp, []{ _goto_manual_move_z(float(SHORT_MANUAL_Z_MOVE)); });
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

@thinkyhead thinkyhead Oct 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

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.

#endif
}

ACTION_ITEM(MSG_BUTTON_DONE, []{
Expand Down