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

Global printf implemented as a define #20

Closed Chris--A closed 8 years ago

Chris--A commented 8 years ago

Update:

This is a proposal to add a replacement for the global printf function.

See: https://github.com/Chris--A/PrintEx/pull/20#issuecomment-235508973

The rest of this PR text is outdated, as new possibilities have become available


Original PR:

It stores a reference to a Print object, which is typically tied to serial. The functions are:

However this is done through a define, which translates the call to a PrintEx::printf usage.

#define xprintf(str, ...) PrintEx(*PrintEx::_stdout).printf(str, __VA_ARGS__);

Is related to conversation in issue #15

bperrybap commented 8 years ago

Was there an issue with having a function named printf() that overrides the stdio printf() function? I'm assuming there was some kind of issue.

It would be nice to use the name printf() vs a another function name as well as ensure that printf() and sprintf() would be using the same formatter code.

It sure would be nice to have a printf() function built into PrintEx that could use any Print class object once configured, as it would allow all platforms/cores to work the same.

Just define some global wrap object or do a function call to set up the stdout Print class object, then printf() "just works".

It might be a day or so before I can really look at it.

Chris--A commented 8 years ago

Yeah, overloading the stdio.h printf seems pretty much impossible to do with functions because it is a variable argument function.

I have a method working now, it is a bit of a hack though.

This code now works:

#include <PrintEx.h>

void setup() {
  Serial.begin(9600);
  printf("Test 1 %p\n\n", F("A PGM String"));
  printf(F("Test 2 %p\n\n"), F("A PGM String"));
}

void loop() {}

I'll have to check to make sure the #define doesn't cause problems.

Chris--A commented 8 years ago

@bperrybap

This PR essentially now adds two new global features making the collection this:

I'll have to come up with some logic to handle every board as it sets the default 'stdoutobject to Serial. Do you know of boards that do not containSerial`? Although the Arduino bootloaders use it so I'd imagine they all have it.

I'll add in some method to change the internal pointer for 'stdout'.

bperrybap commented 8 years ago

So you figured out how to override the stdio printf()? If so, that is awesome. I'll have to do some testing.

So far all the boards I've worked with have had the Serial object. (chipkit, Teensy (all), uno, leonardo, mega)

The actual class of the object can vary.

I think in some cases like on Teensy, there may be a mode where there isn't a serial port. I can't remember. It may be that the USB virtual port just moves over to the hardwareserial when USB is used for something else. I'll have to go back and check.

Although, it might be an issue on the ATTINY chips. I have some but haven't used Arduino on them. But I'm not sure if that comes with a default serial port object since it is so pin limited. However, those tend be so code limited that most couldn't use the library.

Does printf() really need to have a default output class? (The AVR libC printf() requires some support code initialization to set it up) There are many ways to set up the global that are pretty minimally invasive to the code. Either a global definition of some sort, or a function call. And the user will usually have to initialize any output class object anyway - like with Serial. So there isn't a way to create a solution that work "out of the box" without the user having to do some sort of initialization on the output class.

Chris--A commented 8 years ago

Does printf() really need to have a default output class?

I can add a function to make changing the pointer easier. Unless someone wants to set their own Print object to use, shouldn't a default be provided to make it usable out of the box?

I'll look at adding code to only give a default to boards that are known to have a Serial class.

Chris--A commented 8 years ago

PrintEx now has a static function which will allow changing the stdout pointer.

PrintEx::set_stdout( Serial );
bperrybap commented 8 years ago

I like the ability to set the stdout object, but I'm still a bit hesitant about having a default output class.

While there can be a default stdout object such as Serial, printf() can't really can't be used directly out of the box since most objects that use the Print class need some sort of initialization. Even Serial requires calling begin() to set the baud rate and on devices that use USB for their Serial device (like Leonardo, Teensy, etc..) you have spin waiting on a boolean to wait for the virtual connection to the host be active. So so there is already some required initialization code associate with the stdout object.

Another reason for my hesitation about using a default object is also that whatever the default is, will likely be linked in do to the object reference. So if a sketch is not using Serial, it will still have all the Serial code, which includes loss of ram for the buffers.

I'm thinking of the case, where it is being used on something like a lcd or graphic lcd where Serial not be used at all by the sketch.

Would it be possible to also add this set_stdout() function to the wrap class? i.e. so a user could call foo.set_stdout() with no argument to set the stdout object to foo so that all future printf() calls would be directed to foo? It might be nice as then a user could create a wrapped object, then do all the needed initialization, and call set_foo.stdout() to set printf() to use it. for uno it would be:

Serial.begin(9600);
Serial.set_stdout();

Or what about a definition in addition to a global function?

perhaps something like:

PrintEx::stdout = foo;

So the user could simply insert this at the top of his code and define the printf() output class.

Chris--A commented 8 years ago

While there can be a default stdout object such as Serial, printf() can't really can't be used directly out of the box since most objects that use the Print class need some sort of initialization. Even Serial requires calling begin() to set the baud rate and on devices that use USB for their Serial device (like Leonardo, Teensy, etc..) you have spin waiting on a boolean to wait for the virtual connection to the host be active. So so there is already some required initialization code associate with the stdout object.

I guess I was expecting people to do this.

Another reason for my hesitation about using a default object is also that whatever the default is, will likely be linked in do to the object reference. So if a sketch is not using Serial, it will still have all the Serial code, which includes loss of ram for the buffers.

You are right, I have done a little testing and it appears to be around 800 flash bytes, and 170 ram bytes larger. I'll change the code to have a null and the printf implementation will check for a null before continuing.

Would it be possible to also add this set_stdout() function to the wrap class?

This is a good idea, I'll add it to all the interfaces.

Or what about a definition in addition to a global function? perhaps something like: PrintEx::stdout = foo;

There is a global definition. PrintEx::_stdout. I cannot use stdout as it is also a define in stdio.h :disappointed:

bperrybap commented 8 years ago

Was wanting to have an example in hd44780 that uses printf() but the code for this does not seem to be on the master branch. I tried using the dev_merge_test branch which seems to have the code, but still no joy.

I'm not sure of the syntax. I tried this: PrintfSupport::_stdout = &Serial;

But got this error:

PrintEx:35: error: '_stdout' in 'struct PrintEx' does not name a type
 PrintEx::_stdout = &Serial;

What is the correct syntax to do this (without using the set_stdout() call)? And any estimates as to when this will be available on the master branch and available through the Arduino library manager?

Chris--A commented 8 years ago

I was just testing the merges from all my working branches in 'dev_merge_test'. The actual _stdout is global now. I'll have to push an update to remove the PrintfSupport version (not used).

Assignments can't be done at global scope. If you want to define it you need to provide the full declaration: Print *_stdout = &Serial;

However, it can be assigned in a function:

void setup() {
  _stdout = &Serial;

  printf( "test%20r", '!' );
}

I'll have to update the repo though for the first version to make the original definition weak, otherwise it will give you a multiple definition error.

Chris--A commented 8 years ago

I'm also going to merge the printf_specifiers & global_printf branches into develop, I'll see if I can get that done now.

The develop branch is the main working branch. I'll try and get the next update on master available in the next few days.

EDIT: all changes are now in 'develop'.