Skip to content
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

Add method Print.printf(). Fixed & tested. #592

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

u48
Copy link

@u48 u48 commented Jan 25, 2019

Added the printf() method to the Print class.

With this implementation, Print.print() works with any devices - Uart, TFT, SDCard and others.

The previous version has not been tested. I'm sorry.
This worked on other my platforms, but a bug was discovered on the Arduino STM32.
Fixed.

u48 added 4 commits January 9, 2019 11:07
The printf() method always uses self instance of the class Print, without additional implementation of the output to UARTs, TFTs, and etc.
enable printf by default
Fix bug with use fopencookie() for implemet Print.printf().
I checked it, and Print.printf()  now works.
cleanup source
@RickKimball
Copy link
Contributor

Is there any way to add an additional format specifier for binary? I'd love to be able to use:

Serial.printf("bits=0b%08b\n", 0x33);

@justinschoeman
Copy link
Contributor

Not tested, but this should work if you are not using nano.specs:
https://www.gnu.org/software/libc/manual/html_node/Customizing-Printf.html

@stevstrong
Copy link
Collaborator

@RickKimball
Copy link
Contributor

Thanks @justinschoeman! We learn something new everyday.

@rogerclarkmelbourne
Copy link
Owner

rogerclarkmelbourne commented Jan 28, 2019

Why does one of the the change to print.cpp include this block of comment ?

`/*
int Print::printf (__const char *__restrict __format, ...)
//Bug detected
{
//failed: fopencookie( 0, "w+", { NULL, WR_fn, NULL, NULL })
FILE *__restrict __stream;
//failed: fopencookie( 0, "rw+", { NULL, WR_fn, NULL, NULL })
int ret_status = 0;
//

//worked: fopencookie( 0, "rw+", { RD_fn, WR_fn, NULL, NULL })

//worked: fopencookie( 0, "rw+", { NULL, WR_fn, NULL, NULL })
va_list args;
//
va_start(args,__format);
//Resume: To fix - you should always specify the cookies_read function, zero is not valid.
ret_status = vfprintf(__stream, __format, args);
*/`

As far as I can tell this code references the function

fopencookie

which does not exist

@u48
Copy link
Author

u48 commented Jan 28, 2019

Is there any way to add an additional format specifier for binary? I'd love to be able to use:

Serial.printf("bits=0b%08b\n", 0x33);

As implemented in LIBC. :)
Need to check this.

@u48
Copy link
Author

u48 commented Jan 28, 2019

As far as I can tell this code references the function
fopencookie
which does not exist

Hmm..
This is a LIBS extension defined in the header file studio.h. And provided by the compiler you are using.

This is supported in the native Linux gcc, Raspberry, gcc for Arduino DUE and ESP8286/32.
But why is it disabled in the mingw.
What compiler You use?

@u48
Copy link
Author

u48 commented Jan 28, 2019

Why does one of the the change to print.cpp include this block of comment ?
It's just to avoid questions why need an empty cookie_read_helper () function.
And there was no attempt to remove it.
Read and delete it.

@rogerclarkmelbourne
Copy link
Owner

I didn't test whether it compiled yet.
I just wasnt aware that fopencookie was a function in the standard c libraries.

I'll locally merge in a separate branch and see whether I get any errors when I compile.

@rogerclarkmelbourne
Copy link
Owner

OK.
It looks like it fopencookie can be linked in.

But more testing would be needed before I can merge

add #define _GNU_SOURCE 
So more correct for use fopencookie()
@u48
Copy link
Author

u48 commented Jan 29, 2019

fopencookie () is an extension of LIBC.

The correct use of this extension is as follows:
   #define _GNU_SOURCE
   #include <stdio.h>

And not as it is now written in the source, only
#include <stdio.h>

It is possible that such a trifle is a source of problems.

@rogerclarkmelbourne
Copy link
Owner

This PR does not seem to work for me.

Please post a small example sketch which works
Also let me know if you have USB serial connected and what upload method you are using (as this affects the config of the USB Serial and Serial devices e.g. "Serial" can be either USB Serial or UART Serial depending on upload method

@u48
Copy link
Author

u48 commented Feb 25, 2019

I suspect that this is not compiled correctly or works.
I take 1-4 days for tests.

I will do a few test sketches, and describe their behavior on various new platforms assembled from scratch.
For experiments there is all the necessary and unnecessary hardware

@rogerclarkmelbourne
Copy link
Owner

OK.

Thanks

BTW.
@RickKimball

Did this PR work for you ?

//failed: fopencookie( 0, "rw+", { NULL, WR_fn, NULL, NULL })
//
//worked: fopencookie( 0, "rw+", { RD_fn, WR_fn, NULL, NULL })
//worked: fopencookie( 0, "rw+", { NULL, WR_fn, NULL, NULL })
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Line 229 (worked) seems to be identical to line 226, which failed.
Btw, do we really need this comment block? I would remove it.
The solution matters, which is implemented below.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So basically this PR doesn't work unless some lines are uncommitted ???

Copy link
Author

@u48 u48 Feb 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Block comments should be removed. :)

The GLIBC documentation says that when fopencookie() is called, pointers to BOTH the write() and read() functions must be passed.

That is, if reading is not required, a valid pointer to a dummy function should be passed, and not NULL. Otherwise it does not work.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand the reason, I just asked to remove unneeded comment block (which includes contradictory information).

delete contradictory information
@victorpv
Copy link
Contributor

victorpv commented Jan 2, 2020

Steve looks like the comments were cleaned up on February 28th, after your last comment.

@fpistm
Copy link
Contributor

fpistm commented Jan 2, 2020

@stevstrong
Copy link
Collaborator

stevstrong commented Nov 3, 2023

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 this pull request may close these issues.

7 participants