FPSTR() incorrectly defined in pgmspace.h

[bxparks]

Hardware:

Board: ESP32 Dev Module
Core Installation/update date: 2018-05-01
IDE name: Arduino IDE
Flash Frequency: 80Mhz
Upload Speed: 921600

Description:

The FPSTR() macro in ./cores/esp32/pgmspace.h is incorrectly defined. It is currently

#define PSTR(s)       (s)

But that macro should do a cast to a (const __FlashStringHelper*) instead.
See https://github.com/esp8266/Arduino/blob/master/cores/esp8266/WString.h#L38

The sketch below works on ESP8266 but fails on ESP32.

The solution is to remove FPSTR() from pgmspace.h and add the following to WString.h:

#define FPSTR(pstr_pointer) (reinterpret_cast<const __FlashStringHelper *>(pstr_pointer))
#define F(string_literal) (FPSTR(PSTR(string_literal)))

(where I have redefined F() in terms of FPSTR() as done in the ESP8266 version of this file).

I can send you a PR if you agree that this is the correct solution.

Sketch:

#include <Arduino.h>

#if defined(ESP32) || defined(ESP8266)
  #include <pgmspace.h>
#else
  #error Unsupported platform
#endif

const char flashString[] PROGMEM = "abcedf";
const __FlashStringHelper* const flashHelperString = FPSTR(flashString);

void setup() {
}

void loop() {
}

Debug Messages:

Build options changed, rebuilding all
In file included from /home/brian/Downloads/Arduino/arduino-1.8.5/hardware/espressif/esp32/cores/esp32/WString.h:29:0,
                 from /home/brian/Downloads/Arduino/arduino-1.8.5/hardware/espressif/esp32/cores/esp32/Arduino.h:150,
                 from /home/brian/dev/arduino/esp32/fpstr_demo/fpstr_demo.ino:1:
/home/brian/Downloads/Arduino/arduino-1.8.5/hardware/espressif/esp32/cores/esp32/pgmspace.h:35:41: error: cannot convert 'const char*' to 'const __FlashStringHelper* const' in initialization
 #define FPSTR(p)      ((const char *)(p))
                                         ^
/home/brian/dev/arduino/esp32/fpstr_demo/fpstr_demo.ino:10:54: note: in expansion of macro 'FPSTR'
 const __FlashStringHelper* const flashHelperString = FPSTR(flashString);
                                                      ^
exit status 1
Error compiling for board ESP32 Dev Module.

[bxparks]

Looks like this bug was introduced in #6.

[arjanmels]

Any update on this? (Of course it is easy to work around, but still breaks compatibility.)

[eabase]

@bxparks
You say that FPSTR macro is wrongly defined, yet you show the definition of PSTR!?
So what is it?

[bxparks]

That was a typo, I meant that type that the following in cores/esp32/pgmspace.h is incorrect:

#define FPSTR(p)      ((const char *)(p))

and should be the following instead:

#define FPSTR(pstr_pointer) (reinterpret_cast<const __FlashStringHelper *>(pstr_pointer))

The purpose of FPSTR() is to coerce the string into a __FlashStringHelper* so that the compiler binds to the correct function/method which accepts a PROGMEM string, instead of a normal string. Of course, on the ESP32, it does not matter. But if we want source-level compatibility with ESP8266, then we should fix this.

I will send a PR shortly.

[eabase]

I think the key words here are:
Of course, on the ESP32, it does not matter.. 👍

[bxparks]

What's the harm in fixing this? It makes code that was created on a ESP8266 work on the ESP32, without having to sprinkle convoluted conditionals like this in various places:

#if defined(ESP8266)
  #include <pgmspace.h>
#elif defined(ESP32)
  #include <pgmspace.h>
  // Clobber the incorrect definition of FPSTR
  #undef FPSTR
  #define FPSTR(pstr_pointer) \
      (reinterpret_cast<const __FlashStringHelper *>(pstr_pointer))
#endif

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

[bxparks]

Verified fixed in 1.0.3-rc2 (and rc3 for completeness). Closing.

[bxparks]

Version 1.0.3 now has this.

Quick note that I discovered myself: If you have some old code that was targeted to both ESP8266 and ESP32, and tried to workaround the FPSTR() bug on the ESP32 by doing something like this:

#if defined(ESP32) && defined(FPSTR)
  #undef FPSTR
  #define FPSTR(p) (reinterpret_cast<const __FlashStringHelper *>(p))
#endif 

You may now get a compiler warning about FPSTR() being redefined in <WString.h>. The warning happens if the above workaround code is seen by the compiler before <WString.h> is included.

As far as I can tell, there is no macro that contains the version number of the ESP32 core (e.g. something that resembles #define ESP32_CORE_VERSION 10003), so we can't conditionally guard the workaround code using a version number.

The simplest solution that I've found to prevent the compiler warning is to explicitly add a

#include <WString.h>
{place workaround code above here}

just before the workaround code.