Skip to content

Added support for printing uint64_t numbers #1686

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

Closed
wants to merge 0 commits into from

Conversation

mykeul
Copy link

@mykeul mykeul commented Feb 26, 2016

No description provided.

@igrr
Copy link
Member

igrr commented Feb 26, 2016

The issue with this implementation of printing 64-bit numbers is that every other number (8, 16, 32-bit) will be converted to 64 bit before printing. This is a pretty big overhead, especially since most users will not be actually printing 64 bit numbers.
It has been suggested on Arduino developers mailing list [1] that the right way to implement this feature is to write separate routines for 64-bit case, so that smaller sized integers still go through the 32-bit printNumber function. Besides, we are much less constrained on code size than AVR based Arduinos, so having an extra function for the 64-bit case if it is actually used is not an issue.

[1] https://groups.google.com/a/arduino.cc/d/topic/developers/Royw6V4ictU/discussion

@mykeul
Copy link
Author

mykeul commented Mar 7, 2016

Hi Ivan,

I updated my branch. printNumber() is now a template and produces two versions of the function : printNumber() and printNumber<uint64_t>(). Shorter (16 or 8 bits) versions may be used too, but I didn't moved that far as it may use more flash on some projects.

MyKeul

@ghost
Copy link

ghost commented Apr 11, 2016

Can one of the admins verify this patch?

@igrr
Copy link
Member

igrr commented Apr 13, 2016

Waiting for https://travis-ci.org/esp8266/Arduino/builds/122758554 to check code size difference.

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.

2 participants