Chris--A / PrintEx

An extension to the Arduino Print library, and much, much more...
GNU General Public License v3.0
61 stars 16 forks source link

Format string cannot be a flash string using F() #15

Open bperrybap opened 8 years ago

bperrybap commented 8 years ago

It would be nice if the format strings could be flash strings which are declared using F(). Using format strings in flash can be critical the AVR to keep const strings out of SRAM.

It is fairly easy to add, it requires creating an overload function where the const format string is declared something like:

pft printf(const __FlashStringHelper *format, ...);

And then the appropriate functions to fetch/process the string from flash as well as appropriate guards for AVR code as it only applies to the AVR processor.

Chris--A commented 8 years ago

This is something I have already implemented in test code, just never got around to adding it. Currently only the additional parameters can be flash strings. I'll finish my other changes and paste this in.

What I was originally hoping was to modify the printf code to also use something similar to my PGMWrap library so it doesn't need a buffer to store the flash string while processing. However as a quick fix I can add in a method with a buffer for now.

In the future I can look at modifying the background processing and changes shouldn't affect the users.

A bit off topic: As for PROGMEM on AVR's, it has come to my attention that @igrr or others at the ESP8266 repo have implemented flash handling using the PROGMEM keyword, and have re purposed the pgm_read_xx() functions also (See: https://github.com/Chris--A/PGMWrap/issues/1#issuecomment-233501010).

Here is the ESP source: pgmspace.h, pgmspace.cpp

Chris--A commented 8 years ago

I have added a patch in the 'printf_update' branch. It is just a quick fix. I'm still looking at moving a few things around to make printf a bit more efficient.

bperrybap commented 8 years ago

so do you actually have a printf() function? I didn't see it but that would be really nice as people have been asking for that for many years. It must have some sort of global declaration/definition where the application defines the default Print class device.

Chris--A commented 8 years ago

The code test worked:

#include <PrintEx.h>

void setup() {
  auto &serial = PrintEx::wrap(Serial);

  serial.begin(9600);
  serial.printf( F("A test of printf using PROGMEM strings!\nProgmem data:\t%p"), F("A PROGMEM String") );
}

void loop() {}

https://github.com/Chris--A/PrintEx/blob/printf_update/src/lib/PrintExtension.h#L68

Chris--A commented 8 years ago

It also affects sprintf

#include <PrintEx.h>

void setup() {
  char buff[100];

  Serial.begin(9600);
  sprintf( buff, F("A test of printf using PROGMEM strings!\nProgmem data:\t%p"), F("A PROGMEM String") );
  Serial.print( buff );
}

void loop() {}
bperrybap commented 8 years ago

Neither of those is a printf() function. The first example is a wrap that adds a printf() method to the new object. I'm talking about a stand alone printf() function just like the standalone sprintf() function. I'm assuming that a standalone printf() does not yet exist. Seems like everything needed to do it is pretty there. It just needs a global to define the default Print class to use for the output.

Chris--A commented 8 years ago

Ah, I see what you mean, I'll add that as well.

bperrybap commented 8 years ago

oooh. Nice! a real printf() is an Arduino feature that MANY people have wanted for years. (It probably deserves its own feature request issue) I think PrintEx library use is about to really take off. I'm loving it.

Chris--A commented 8 years ago

Just trying a few different things. I can't use a define like sprintf as printf can be standalone or part of an object.

Chris--A commented 8 years ago

Unfortunately I cannot override the stdio version due to the variable argument function. The flash string version works because the first parameter are different.

If it can have a different name, this would be simple, like Printf(...), _printf(...), PrintEx(...), format(...).

bperrybap commented 8 years ago

I'm not understand the issue you are facing. Is there some sort of name collision with a function in stdio.h ?

A printf() function is basically the same as a sprintf() function other than the first argument is different (removed).

What are you needing? function or macro? I've used both vararg functions as well as variadic macros to offer printf() like functions in openGLCD.

macro when on AVR platform:

#define SerialPrintf(fmt, ...) _SerialPrintf(PSTR(fmt), ##__VA_ARGS__)
void
_SerialPrintf(const char *fmt, ...)
{
FILE stdiostr;
va_list ap;

  fdev_setup_stream(&stdiostr, serialputc, NULL, _FDEV_SETUP_WRITE);

  va_start(ap, fmt);
  vfprintf_P(&stdiostr, fmt, ap);
  va_end(ap);
}

In a function the arg list is different to support both "format string" and F("format string")

void gText::Printf(const char *format, ...)

void gText::Printf(const __FlashStringHelper *format, ...)

Chris--A commented 8 years ago

There is a global printf in stdio.h, yeah there is a name collision. I haven't found a method of overloading the original with a new version. It simply selects the original.

The flash version is working as the format parameter is a different type and unique. I've tried using a namespace however this is not ideal either:

This does work:

#include <PrintEx.h>

void setup() {
  Serial.begin(9600);

  std::printf("A test 1 %p\n\n", F("A PGM String"));
  std::printf(F("A test 2 %p\n\n"), F("A PGM String"));
}

void loop() {}

This will not work (only flash version), and imports both printf sources increasing flash usage:

#include <PrintEx.h>

void setup() {
  Serial.begin(9600);

  using namespace std;
  printf("A test 1 %p\n\n", F("A PGM String"));
  printf(F("A test 2 %p\n\n"), F("A PGM String"));
}

void loop() {}
Chris--A commented 8 years ago

The code I was using for the test above is in the global_printf branch.

bperrybap commented 8 years ago

I ran into printf() issues in my library. I never bothered to dig deep enough to solve it. That is why you see above that the method was name Printf() vs printf() I couldn't even use the name printf() as a method in the class as it caused compilation errors. Seems like there were even macro issues relating to this name as well - I can't remember that far back (2009 time frame) I guess its time to dive deep and finally resolve how to work around this. I'll go off and do some checking & experimentation as well.

Chris--A commented 8 years ago

I have done an update the the global_printf branch. It now contains a #define for xprintf.

It is quite reasonable I think.

EDIT: See PR #20