Skip to content

How to handle __FlashStringHelper #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wmarkow opened this issue Apr 5, 2021 · 5 comments · Fixed by #11
Closed

How to handle __FlashStringHelper #5

wmarkow opened this issue Apr 5, 2021 · 5 comments · Fixed by #11

Comments

@wmarkow
Copy link
Collaborator

wmarkow commented Apr 5, 2021

@2bndy5, I just copy here your sentence from your wmarkow#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
Copy link
Collaborator Author

wmarkow commented Apr 5, 2021

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
Copy link
Member

2bndy5 commented Apr 5, 2021

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
Copy link
Collaborator Author

wmarkow commented Apr 5, 2021

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
Copy link
Member

2bndy5 commented Apr 5, 2021

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 wmarkow changed the title Ho to handle __FlashStringHelper How to handle __FlashStringHelper Apr 5, 2021
@wmarkow
Copy link
Collaborator Author

wmarkow commented Apr 14, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants