-
-
Notifications
You must be signed in to change notification settings - Fork 19.3k
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
Fix status_printf() jumbled script #26949
Fix status_printf() jumbled script #26949
Conversation
I have not reviewed this because I honestly do not know when each of these is appropriate: Feel free to educate me and convince me that what you have done is correct. |
It's been a longtime convention for classes to have a method that takes a straight Anyway, I believe we should retain the " |
15483bb
to
1f87179
Compare
A quick build test shows that the build size with MarlinUI on AVR isn't significantly impacted by the use of the inline passthrough for FSTR => PSTR. Specifically, removing it only reduces the build size by 6 bytes. Following this PR's suggestion that we need to improve clarity, I've kept the parameter renaming and also applied some more of that to the string classes. We have to keep using PROGMEM strings for the benefit of AVR, so hopefully we can continue to improve the clarity of our usage conventions, and, where we can, we will continue to transition to To clarify, we use In most places we've made sure to distinguish functions that expect PROGMEM strings (which differ on AVR) from functions that expect C-strings (which can only be SRAM strings on AVR) and we've transitioned code that took P-strings into code taking F-strings, retaining the P-string versions for utility. And I started the process of moving to String Stuff
In some instances we've ignored code that neglects the existence of PROGMEM strings, since the affected code was only designed to run on ARM. However, there is no reason to limit code to ARM (especially LCD code) if AVR boards could otherwise run it. So it's a good idea to convert code to use F-strings wherever we possibly can. This will also help to unify the language translations across all displays. |
@thinkyhead I notice something which is odd, sometimes using basically I'm not quite sure the approach to take. stick with |
Co-authored-by: Scott Lahteine <[email protected]>
Description
status_printf_P
=>status_printf()
- marlinui.hfmt
topfmt
as it is statedva_start(args, FTOP(pfmt));
va_start(args, pfmt);
works/compiles normally.so Question: was having
FTOP
there meant for a reason? because it seems to be fine without it.I was able to make it show, short clip of what is happening for context:
Marlin-status_printf.mp4
At some random times I've noticed this strange phenomena that would happen. The status display would show different things, and back tracing to what it could be I narrowed it down to lcd/marlinui.h
when hovering over this function I get this:
and so I think I solved the issue by simply removing the
template
associated with it. and just simplify the function to one, like it was in v.2.1.1.1 or earlierRequirements
Noticed with ProUI and JyersUI, not sure about any other.
Benefits
hopefully stops random status messages from appearing.
Configurations
Related Issues
could be an underlying issue that stays hidden until random events.