How to handle __FlashStringHelper

[wmarkow]

@2bndy5, I just copy here your sentence from your https://github.com/wmarkow/RF24Log/pull/1

Turns out that the __FlashStringHelper isn't compatible with all Arduino boards (probably another pgmspace.h problem). @wmarkow Please read this addressed issue. It talks about the differences in flash memory manipulation/storage on various architectures, and how it relates to the F() macro (and consequently the __FlashStringHelper wrapped datatype).

__FlashStringHelper* in the class definitions that end up breaking on MBED, SAMD, and megaAVR (which doesn't include ATmega328) architectures. As mentioned in the issue I linked, the __FlashStringHelper is defined in a way so that non-AVR architectures would never use the libraries' functions that overload char* with __FlashStringHelper.

Wouldn't it be better anyway to have the user pass const char* instead of explicitly (const __FlashStringHelper*) vendorID that way the compiler can properly optimize accordingly (depending on the architecture) when we delaget it to flash using F("str") in the function definitions?

The purpose of this issue is how to deal with the __FlashStringHelper which helps on Arduino boards to save some RAM where the strings are stored in the FLASH memory.

Also need to reconsider how much using of __FlashStringHelper can help to save some RAM. Maybe it will not help to much?

[wmarkow]

Wouldn't it be better anyway to have the user pass const char instead of explicitly (const __FlashStringHelper) vendorID that way the compiler can properly optimize accordingly (depending on the architecture) when we delaget it to flash using F("str") in the function definitions?

That will not work on AVR archtiecture (or will have a not desired effect). If you pass a const char* (even if you cast the result of F(str) macro) then compiler will treat it as a const char* and finally in the log handler will call Print::print(const char str[]) instead of Print::println(const __FlashStringHelper *ifsh) and the string will be read from RAM memory instead of FLASH. Of course the code will compile but some garbage may be printed int the output.

__FlashStringHelper is some kind of marker datatype; all low level Arduino compatible methods reads from FLASH when thay deal with __FlashStringHelper.

More info also here and here.

Maybe we should remove __FlashStringHelper completely and replace it with const char*. Then it will work on all architectures. We can see how it behaves and in the meanwhile think of introducing __FlashStringHelper somehow again so it will work for avr devices?

[2bndy5]

Since it's only helpful to AVR arch, we could wrap the functions that take__FlasStringHelper with #ifdef ARDUINO_ARCH_AVR, but that would also mean removing explicit calls from the examples.

[wmarkow]

That makes sense, but then we need to forget about those templates methods, right? Lets stick to the example of RF24Logger::log() method.

In the basic all-arch-version we would have: void log(uint8_t logLevel, const char *vendorId, const char* message, ...)

and additionally we could introduce later something for avr (wrapped around in#ifdef ARDUINO_ARCH_AVR): void log(uint8_t logLevel, const __FlashStringHelper *vendorId, const __FlashStringHelper *message, ...) or just void log(uint8_t logLevel, const __FlashStringHelper *vendorId, const char *message, ...)

Actually for logging purposes void log(uint8_t logLevel, etc...) is good enough and we could replace the other error(), warn(), info(), debug() with macros (I already did some preliminary tests) but this is a topic for a different issue.

[2bndy5]

Adding some vital arch info that was left out from the referenced PR:

EDIT: Note WString.h includes avr/pgmspace.h, and WString.h (like pgmspace.h) isn't available to all Arduino cores. This is what String.h does to compensate.

[wmarkow]

In #6 we did some investigation about __FlashStringHelper. It looks like it helps very much in avr architecture to save RAM.

In this comment I propose a "new API methods" in RF24Logger. Let me check how it can looks like, then I will go back with some results.