Fixes for FTDI Eve Touch UI

[marciot]

Other FTDI Eve Touch UI Fixes

Use correct leveling menu for CocoaPress UI

Partial reversion of e1907a9 to fix function overloading in PROGMEM strings in FTDI Eve Touch UI

Clean up the use of strings in FTDI Eve code to allow function overloading to work properly. This partially reverts a commit which replaced F() with PSTR() and cleans up other places in the FTDI Eve code where the things were confusing or inconsistent.

In the FTDI Eve Touch UI code, F() must be used instead of PSTR() because the FTDI Eve Code does not use "_P" suffixed routines, instead, it uses function overloading and template programming to select proper types. For this to work properly, the following conventions are used in the FTDI Eve code:

1. Use F() for PROGMEM strings any time when calling an FTDI Eve routine (these lack the _P suffixes)
2. Use PSTR() for PROGMEM strings any time when calling a non-FTDI Eve routine with the "_P" suffix
3. Functions which take PROGMEM strings have a function signature of "progmem_str"
4. Functions which take RAM strings have a function signature of "const char *"
5. Functions which take either PROGMEM string or RAM strings are overloaded and expect the above conventions to be followed to allow the compiler to pick the proper type

The reason PSTR() cannot be used in FTDI Eve routines because it has a signature of "const char *" which is the same signature as "const char *" used for RAM strings; hence, the compiler will pick the incorrect RAM based routine in such cases.

The choice of using F() in the FTDI Eve code is a complex one and cannot be easily avoided or reversed at this time. This PR simply restores the conventions that have been verified to work consistently.

[thinkyhead]

I have a vague recollection that we had chosen PSTR() over F() throughout Marlin because there was some amount of extra overhead for F(), but I cannot find that discussion.

So what is the deal with F() anyway? Is it merely a type-unique wrapper for a PROGMEM string, essentially equivalent to PSTR() which requires the receiver of its pointer to use PROGMEM accessors, or does the _FlashStringHelper include code to do automatic copying of the PROGMEM string into an SRAM buffer before passing the string's pointer?

If a function takes a char * and expects a normal static string and you pass an F() string, which is a PROGMEM string, then I would expect the function to break on AVR because it is not using proper PROGMEM accessors for the string it receives, unless that string were copied into a normally-accessible buffer before being sent.

The important things here are to ensure that:

• Static strings are stored as PROGMEM strings on AVR platforms;
• Correct accessors are used for F() and PSTR() strings on all platforms;
• Wrappers should not add extra overhead for storage.

[thinkyhead]

In various parts of Marlin we've been using PGM_P and PGM_P const as the arguments for PROGMEM strings. Would it be more type-safe and allow the compiler to better catch bad parameters if we used progmem_str throughout the codebase?

[marciot]

I have a vague recollection that we had chosen PSTR() over F() throughout Marlin because there was some amount of extra overhead for F(), but I cannot find that discussion.

So what is the deal with F() anyway? Is it merely a type-unique wrapper for a PROGMEM string, essentially equivalent to PSTR() which requires the receiver of its pointer to use PROGMEM accessors, or does the _FlashStringHelper include code to do automatic copying of the PROGMEM string into an SRAM buffer before passing the string's pointer?

I'll be honest with you, I do not understand the exact semantics of "progmem_str", AFAIK, it is part of the Arduino library and it is meant to solve a specific problem, but its poorly documented and I don't even know what it was originally for. But I will tell you why I ended up using it.

When I initially developed the FTDI Eve Touch UI code, I needed to be able to have overloaded functions that could properly deal with either RAM or PROGMEM strings. Using "progmem_str" gave me the behavior I needed, so I stuck with them. I believe they work because they are a wrapper class, which makes them distinct from "char *"

I believe you are correct in that they have some degree of overhead, I remember reading about this too. So I would not recommend changing PSTR() to F() all over Marlin. However, within the FTDI Eve code, it certainly is necessary right now for AVR compatibility.

[marciot]

In various parts of Marlin we've been using PGM_P and PGM_P const as the arguments for PROGMEM strings. Would it be more type-safe and allow the compiler to better catch bad parameters if we used progmem_str throughout the codebase?

I think it depends on what PGM_P is. If it simply is a typedef to "char *", then it would provide no type safety and it would be up to the programmer to use the correct function (either with or without the _P prefix). I think in order to make it type safe, it would need to be a class -- I use "progmem_str", but like you said, it has some overhead which are unrelated to what I am using it for.

[marciot]

Oh, it's starting to come back to me now. I think the Arduino library defines "progmem_str" for use with "println" and "print". In Arduino land, you can either use Serial.print("Some RAM string") or Serial.print(F("Some PROGMEM string")). I think I did some digging in the source and found out that they were able to overload based on "progmem_str", so I started to use that convention in my code too, specifically because I needed the overloading ability.

[marciot]

Okay, so it looks like this is the definition for F():

https://github.com/arduino/ArduinoCore-avr/blob/24e6edd475c287cdafee0a4db2eb98927ce3cf58/cores/arduino/WString.h#L37

And this is the definition for progmem_str in Marlin:

Marlin/Marlin/src/core/types.h

Line 30 in cce585f

typedef const __FlashStringHelper *progmem_str;

So, at first glance, I'm not sure why it would have an overhead, although the comment does mention the following:

// When compiling programs with this class, the following gcc parameters
// dramatically increase performance and memory (RAM) efficiency, typically
// with little or no increase in code size.
//     -felide-constructors
//     -std=c++0x

So, possibly that is a clue as to why F() is less efficient than PSTR()

-- Marcio

[thinkyhead]

I'll get more intimate with progmem_str and F() soon, to see where it helps and where it falls short. If needed it should be possible to create a new lightweight wrapper class or typedef for PROGMEM strings just to distinguish function arguments, but if the __FlashStringHelper can be used in an overhead-free manner then we'll just go for that. I think that the current string wrappers are meant to be C-friendly, so that may be part of what limits them. Anyway, since this ultimately only affects AVR I won't sweat it too much.