arduino / ArduinoCore-megaavr

Arduino Core for the ATMEGA4809 CPU
103 stars 62 forks source link

No need for the F() macro on the the megaavr architecture #53

Closed MCUdude closed 4 years ago

MCUdude commented 4 years ago

I'll leave this here: https://github.com/SpenceKonde/megaTinyCore/issues/81

In short terms, it has no RAM usage benefits, but it wastes flash memory. Should probably be defined as #define F(string_literal) (string_literal) instead.

facchinm commented 4 years ago

Totally agree on this; would you like to open a PR so I can give the right attribution? Thanks

MCUdude commented 4 years ago

Sure, but that PR has to go into the Arduino API repo (String.h IIRC). I thought this repo should be kept architecture-neutral? Because this is very much an architecture relate thing.

facchinm commented 4 years ago

Correct; probably the right approach is to #undef F macro after including ArduinoAPI.h and redefine it as no-op

habazut commented 3 years ago

Did this https://github.com/arduino/ArduinoCore-megaavr/commit/a0f6beb55e6c60fdd7baffb306d99c0844377f43 (which reverted the fix) break this again or do you have another solution for the F() Flash Macro which is present in many codes? Regards, Harald.

facchinm commented 3 years ago

Hi @habazut , in fact a0f6beb "broke" this again. We couldn't come up with a clever solution that fits all the situations. Anyway, in the process we added a bunch of F() relates examples to the CI, just to make sure we are not breaking anyone's sketch again, so if you have any proposal just file a PR and it will be checked against all corner cases.