Retrieve const strings from AVR program memory correctly

[antonrotar]

The constant strings in Unity are stored in AVR program memory, which is a great way to reduce RAM consumption.
However retrieving them back to RAM for usage requires special operations, as described here:
https://www.nongnu.org/avr-libc/user-manual/pgmspace.html

This commit fixes that.
Tested on a atmega328p.

[mvandervoord]

Does strcpy_P handle the situation where a string is NOT in program memory also, or will it only work for those in program memory?

[antonrotar]

I fear no: https://www.nongnu.org/avr-libc/user-manual/group__avr__pgmspace.html#ga5749897c91c479d02054fc02128de482
strcpy_P will read the memory from the given address in program space, where something different is stored.
This is what actually currently happens on an AVR without that change: the string address is pointing to program space but is read with strcpy from data space. The outcome is garbage unfortunately, so the test output is not really readable.

[neomanic]

We got bitten by this yesterday, trying to run unit-tests on a physical AVR. As it stands, #431 broke this completely, as has been noted by this pull request, but this does not solve the problem either, since it assumes then that all strings are PROGMEM.

Arduino solves this issue by using class typing and function overloading (explanation here) but that is obviously a C++ solution that is not available in C.

@mvandervoord, I have a proposed solution, but it requires some significant changes, so I wanted to run them past you before doing the work. These would be:

• add a UnityPrintFlash(const char* string) (or maybe better UnityPrintConst or suggestions welcomed...)
• which on all non-AVR architectures is an alias for UnityPrint(...)
• on AVR, will load from program memory into a scratch char, then call UnityPrintChar(&scratch)
• UnityPrint calls in unity.c that refer to these program memory strings would be changed to be UnityPrint[Flash/Const] instead.
• AVR user then has a choice of using the regular UnityPrint() for dynamic/SRAM strings, or the new function for program memory-located strings
• (separately), to save extra RAM Move the TEST_ASSERT constant strings from unity.h to unity.c, specify as PROGMEM and specify them extern

So there would be no change to a typical users use of Unity, only the slight chance of confusion around the new function dedicated to constant strings from it's usage in unity.c. For on-device AVR users, they would need to be told to use the right function according to whether their strings are RAM or flash.

If this is too substantial a change and muddies the clarity of the library too much just for this niche use case, then I suggest instead:

• reverting pull request optimisation(AVR): Store static string in AVR EEPROM #431 completely, and
• leaving a comment on that request as to the correct solution for on-device AVR users, in case someone runs into this requirement in the future.

I'll note that we've worked around the issue by just commenting out the #ifdef AVR block, so all strings just go to RAM. This isn't an issue for us currently as we have enough to spare. But if we start running low, I will do the implementation as described here and leave it available as a branch on a fork. If we don't end up running low, then I won't bother unless it'll be accepted, hence why I'm asking in advance.

Thanks.

[antonrotar]

Hi @neomanic, thanks for picking this up. Your suggestion with the UnityPrintConst is actually quite similar to what I proposed here. For me it works - I run tests on a physical AVR and use the outputs for verification. I admit however that having two different print functions is not an elegant solution.
For me it would also be fine to revert #431.

@mvandervoord please consider that in the current state it is not possible to run tests on physical targets.