Strings finally in Flash!

[crankyoldgit]

Finally convince the compiler to store the text strings into flash.

Saves approx 2k of Global ram, for a trade-off of between ~0-0.5k of extra flash space used.

e.g. (updated)

• IRMQTTServer example code:

  • Before:
    RAM: [===== ] 54.1% (used 44344 bytes from 81920 bytes)
    Flash: [===== ] 54.2% (used 566209 bytes from 1044464 bytes)
    Bin file size = 570368
  • After:
    RAM: [===== ] 51.3% (used 41992 bytes from 81920 bytes)
    Flash: [===== ] 54.2% (used 566201 bytes from 1044464 bytes)
    Bin file size = 570352

• IRrecvDumpV2 example code:

  • Before:
    RAM: [==== ] 37.9% (used 31044 bytes from 81920 bytes)
    Flash: [==== ] 35.6% (used 372025 bytes from 1044464 bytes)
    Bin file size = 376176
  • After:
    RAM: [==== ] 35.5% (used 29072 bytes from 81920 bytes)
    Flash: [==== ] 35.7% (used 372525 bytes from 1044464 bytes)
    Bin file size = 376672

Fixes #1614
Fixes #1493

Co-authored with @mcspr

[crankyoldgit]

FYI @Pecacheu & @mcspr

[NiKiZe]

This is almost a "replace" of PROGMEM char* x with PROGMEM char x[]
and then same type change of the extern declarations.

[crankyoldgit]

This is almost a "replace" of PROGMEM char* x with PROGMEM char x[] and then same type change of the extern declarations.

That is pretty much correct. Outside of that, it adds #include "IRtext.h" to IRtext.cpp and some notes about why it is the way it is, for future generations of frustrated Arduino ESP8266 programmers. Le Sigh!

[crankyoldgit]

I mean seriously ... char *array and char array[] are equivilent AFAIK, but clearly not in Arduino-land.

[mcspr]

at least one thing will break here - bulk of IRtext strings is used in the IRac::strTo* with strcasecmp, which will crash. will try to find the other calls with '<string>' funcs...

specific to IRac, these should be strcasecmp_P instead and the test framework updated with something like these:
https://github.com/esp8266/Arduino/blob/9d024d17fd18dffa569fa646b5326b5e8f58b0d5/tests/host/sys/pgmspace.h#L57

[crankyoldgit]

at least one thing will break here - bulk of IRtext strings is used in the IRac::strTo* with strcasecmp, which will crash. will try to find the other calls with '' funcs...

specific to IRac, these should be strcasecmp_P instead and the test framework updated with something like these: https://github.com/esp8266/Arduino/blob/9d024d17fd18dffa569fa646b5326b5e8f58b0d5/tests/host/sys/pgmspace.h#L57

Latest commit to the branch should address that now. Let me know if it doesn't.

I couldn't find any other likely occurrences anywhere else in the code base, so fingers crossed, I think I may have them all.

[crankyoldgit]

FYI. This utterly breaks IRrecvDumpV2 etc. So don't use this. Depending on the fix, I may go with plan B

[mcspr]

FYI. This utterly breaks IRrecvDumpV2 etc. So don't use this. Depending on the fix, I may go with plan B

Ok... This is embarrassing, because only part of the WString deals flashmem & ram string handling and it will fail with operator+= / concat. The first crash is at here:

0x40203c9b: String& String::operator+=<__FlashStringHelper const*>(__FlashStringHelper const* const&) at C:\Users\max\.platformio\packages\framework-arduinoespressif8266\cores\esp8266/WString.h:134
  \-> inlined by: resultToHumanReadableBasic(decode_results const*) at C:\Users\max\Documents\PlatformIO\Projects\IRremoteESP8266\src/IRutils.cpp:384

b/c of the strlen call trying to access flash address (see String::concat), but I was looking at the different operator and it got lost in the linked issue 🤦

FPSTR(kProtocolStr) will do reinterpret_cast<const __FlashStringHelper*>(kProtocolStr) and things will work though. But, since this is another new code, let me think on alternative a bit more.
(and maybe patch the esp8266 Core as well)

[mcspr]

Just as a quick example
https://github.com/mcspr/IRremoteESP8266/tree/strings_in_flash_flashstringtype
(however... strange that looks :)

But it is slightly larger, will try actually looking at the resulting binary tomorrow

(penv) ~\D\P\P\I\e\IRrecvDumpV2> pio run -e nodemcuv2 -t checkprogsize
...
RAM:   [====      ]  35.5% (used 29068 bytes from 81920 bytes)
Flash: [====      ]  35.7% (used 372553 bytes from 1044464 bytes)

(...probably b/c of some implicit string conversions...)

[mcspr]

(...probably b/c of some implicit string conversions...)

And one more thing

diff --git a/src/IRutils.cpp b/src/IRutils.cpp
index 1d8e4b5..0c18209 100644
--- a/src/IRutils.cpp
+++ b/src/IRutils.cpp
@@ -535,6 +535,11 @@ namespace irutils {
     return addLabeledString((value ? kOnStr : kOffStr), label, precomma);
   }

+  String addBoolToString(const bool value, const __FlashStringHelper* label,
+                         const bool precomma) {
+    return addBoolToString(value, String(label), precomma);
+  }
+
   /// Create a String with a colon separated labeled Integer suitable for
   /// Humans.
   /// e.g. "Foo: 23"
diff --git a/src/IRutils.h b/src/IRutils.h
index fdda6d7..795e25b 100644
--- a/src/IRutils.h
+++ b/src/IRutils.h
@@ -48,6 +48,8 @@ float fahrenheitToCelsius(const float deg);
 namespace irutils {
   String addBoolToString(const bool value, const String label,
                          const bool precomma = true);
+  String addBoolToString(const bool value, const __FlashStringHelper* label,
+                         const bool precomma = true);
   String addIntToString(const uint16_t value, const String label,
                         const bool precomma = true);
   String addSignedIntToString(const int16_t value, const String label,

Size does go down to Flash: [==== ] 35.3% (used 368389 bytes from 1044464 bytes), but it feels like there's a bug somewhere in the String itself

[crankyoldgit]

b/c of the strlen call trying to access flash address (see String::concat), but I was looking at the different operator and it got lost in the linked issue 🤦

FPSTR(kProtocolStr) will do reinterpret_cast<const __FlashStringHelper*>(kProtocolStr) and things will work though. But, since this is another new code, let me think on alternative a bit more. (and maybe patch the esp8266 Core as well)

Yeah, I found that out already. Where I use strlen(well, strlen_P) via a pointer to an arbitary spot in a PSTR, it crashes the ESP8266, thus I had to replace/implement it via using the pgm_blah functions to get one byte at a time.
I've gotten it to progress a lot further by wrapping most kBlahStrs with FPSTR(kBlahStr)s or PSTR(kBlahStr) but this is getting ugly fast.
i.e. I'm happy to do it inside the library, but if some existing ESP8266 user has relied on accessing kBlahStr it's going to cause them no end of grief working out why.

If I am going to cause that much pain, I might as well hide the strings behind an access function so we are sure we are getting a correct/safe type to a user.

/me shakes fist at the ESP8266 architecture!

[crankyoldgit]

Just as a quick example
https://github.com/mcspr/IRremoteESP8266/tree/strings_in_flash_flashstringtype
(however... strange that looks :)

Okay. You've convinced me to stick with/go with that approach for now. That's less messy than my experimental code was turning into.

[crankyoldgit]

Okay. I've tested IRrecvDumpV2 & IRMQTTServer on a Nodemcu & a d1_mini. All seem to work.

[crankyoldgit]

FYI, this now saves ~2k of global ram. We lost a bit on the flash space, but I think I have some ideas on how to claw that back.

Updated figures:

• IRMQTTServer example code:

  • Before:
    RAM: [===== ] 54.1% (used 44344 bytes from 81920 bytes)
    Flash: [===== ] 54.2% (used 566209 bytes from 1044464 bytes)
    Bin file size = 570368
  • After:
    RAM: [===== ] 51.3% (used 41992 bytes from 81920 bytes)
    Flash: [===== ] 54.2% (used 566201 bytes from 1044464 bytes)
    Bin file size = 570352

• IRrecvDumpV2 example code:

  • Before:
    RAM: [==== ] 37.9% (used 31044 bytes from 81920 bytes)
    Flash: [==== ] 35.6% (used 372025 bytes from 1044464 bytes)
    Bin file size = 376176
  • After:
    RAM: [==== ] 35.5% (used 29072 bytes from 81920 bytes)
    Flash: [==== ] 35.7% (used 372525 bytes from 1044464 bytes)
    Bin file size = 376672

@NiKiZe Time for another re-review of the code I'm afraid. It's changed a lot.
FWIW, I'm fairly happy with it, but I'm not going to admit it's a) easy to understand & b) that I understand it fully either.

@mcspr Endless thanks mate! I'm not sure I'd have gotten this worked out in any reasonable timeframe without your help.

[NiKiZe]

I can not find anything against this, but not tested in any way.

is there anything that we need to update in .py grabber scripts or similar?

[crankyoldgit]

I can not find anything against this, but not tested in any way.

is there anything that we need to update in .py grabber scripts or similar?

I updated the IRtext.h builder script. I can't immediately think of any of the python scripts that need adjusting.

Possibly the keywords.txt generator (which probably needs a complete overhaul or removal anyway)

[mcspr]

btw, also re.

/me shakes fist at the ESP8266 architecture!

another issue with not using ..._P aligned-access helpers is the the alternative is pretty slow - we could setup a hardware software trap for the exception, and redirect the read to the correct value. but, it does not know the thing / pattern to access the thing we want, so the result is hit and miss most of the time.
esp8266 Core has un-documented handler for 'generic' flash & iram access, since Core 3.0.0+ (esp8266/Arduino#7060)
(something like this is happening with esp32 builds, but b/c of a slightly different memory composition placing strings into data rom vs. the instruction one (ref. 1.1). and sw variant will possibly will be enabled with esp8266 rtos ports)

edit: typo, clarify esp32 difference

[crankyoldgit]

FYI, the changes mentioned above have now been included in the new v2.8.0 release of the library.