PROGMEM methods for Print.cpp #1094
Conversation
| // (returns maxLen instead of required size) | ||
| while (1) { | ||
| char buffer[size + 1]; | ||
| auto sz = m_vsnprintf(buffer, sizeof(buffer), fmt, va); |
There was a problem hiding this comment.
I guess this PR should wait for #1077. Then you shuld have directly the correct size needed +1 ending char. So I would not use while(1) and wait for the PR
| { | ||
| va_list va; | ||
| va_start(va, fmt); | ||
| auto cnt = vprintf(fmt, va); |
There was a problem hiding this comment.
Please only use auto when hiding an ugly/templetized type specifier. In this case it is size_t. IMHO it's faster to read and debug if you know the types.
| @@ -26,12 +26,14 @@ | |||
|
|
|||
| #define INITIAL_PRINTF_BUFFSIZE 128 | |||
|
|
|||
| #undef printf_P | |||
There was a problem hiding this comment.
You should remove this here. If you do not want name collision you have 2 options: namespaces or explicit calling e.g. this.printf_P
There was a problem hiding this comment.
You are right, this is ugly and will cause problems. Therefore I would suggest the following procedure:
- wait for PR fix m_vsnprintf return value #1077 and m_printf: PROGMEM support added #1096
- optional: implement printf_P() etc. as functions instead a macros in FakePgmSpace (can be changed to an inline wrapper for printf() if m_printf: PROGMEM support added #1096 is accepted)
- remove printf_P() from this PR as it is no longer needed
- add printf_P() and println_P() instead
What do you think?
There was a problem hiding this comment.
Yes, I guess if printf / m_printf will be progmem safe, then no need for _P variant. For printf I do not care about speed so much, but sprintf I would keep it fast
However, I have no idea how to fix the conflict with the new macro printf_P in FakePgmSpace.h :-(
(see #1067)