Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Added support for HEX leading zero in print #6750

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

Open
Tijgerd wants to merge 2 commits into arduino:master
base: master
Choose a base branch
Loading
from Tijgerd:master

Conversation

Copy link

@Tijgerd Tijgerd commented Sep 21, 2017
edited
Loading

for issue #1097

Added the leading zero for the print funcionality

if the argument PRINT_LEADINGZERO is added after the base definition, a leading zero is added to the hex

if PRINT_LEADINGZERO is in the arg, add a leading zero to a HEX if the value is smaler than 0x10
@facchinm facchinm added the Print and Stream class The Arduino core library's Print and Stream classes label Sep 22, 2017
Copy link
Author

Tijgerd commented Sep 26, 2017
edited
Loading

@clivepengelly, @fake-name, @technikadonis, @cmaglie is this a solution for the problem?
Anyone willing to test this?

Copy link

fake-name commented Sep 26, 2017
edited
Loading

I still stand that the current implementation is broken, and the correct option is to have print HEX always emit a leading zero, but that's probably not a winning opinion at this point.

Really, this seems overcomplicated. Why not just have an alternative to the HEX (maybe ACTUAL_HEX? or the less-accurate HEX_LEADINGZERO suggested in the bug report) parameter, rather then redefining a bunch of methods and changing their signatures?

mikaelpatel, Ivan-Perez, and Duckle29 reacted with thumbs up emoji

Copy link
Author

Tijgerd commented Sep 26, 2017

@fake-name ,
I do support your opinion, I'm using HEX because I want it to be with leading zero..

BUT

HEX, DEC, etc. are a bunch of predefined values indicating the base of the value (e.g. HEX = 16)
This base is used as a divider for the value, so for each "letter" printed, the value is divided by the base.

adding a strange value for the base, like ACTUAL_HEX, will be a bit odd because it will be something like: 42 which is not a base value.

I added a OPTIONAL input value which defaults to PRINT_NOARG when not assigned.
if it IS assigned it checks if the given HEX value is smaller than 16, if so, it will add a leading zero.

With this solution, old scripts can still be used :)
I think that's the whole point.. they want it to be backward compatible.

Copy link

fake-name commented Sep 26, 2017
edited
Loading

HEX, DEC, etc. are a bunch of predefined values indicating the base of the value (e.g. HEX = 16)
This base is used as a divider for the value, so for each "letter" printed, the value is divided by the base.

So change that to an enum?

Also, gah, what terrible API design.

Copy link
Author

Tijgerd commented Sep 26, 2017

That's what my initial plan was BUT there may be some users that did actual write the base number and so.. It won't be backward compatible.

I know, it doesnt feel right for the HEX not printing the leading zero but this was once implemented and changing it would make a small group of arduino users unhappy ;)

Copy link

fake-name commented Sep 26, 2017
edited
Loading

That's what my initial plan was BUT there may be some users that did actual write the base number and so.. It won't be backward compatible.

That's not documented, and the documentation makes no statements about what HEX, DEC, etc... actually contain, so they're relying on undefined behaviour now. Their code is already broken, it just coincidentally works.

In fact the documentation explicitly states:

An optional second parameter specifies the base (format) to use; permitted values are BIN (binary, or base 2), OCT (octal, or base 8), DEC (decimal, or base 10), HEX (hexadecimal, or base 16).

So passing anything but BIN, OCT, DEC or HEX is incorrect.

Copy link
Author

Tijgerd commented Sep 26, 2017

Copy link

fake-name commented Sep 26, 2017
edited
Loading

Ah, it's kind of crappily documented on https://www.arduino.cc/en/Serial/Println, the contents of which actually contradict https://www.arduino.cc/en/Serial/Print (which I'm quoting literally above).
Well, except that println() then says "has the same interface as print(). so WTF?

Siiiiiiiigh, Arduino code in a nutshell.

saniyo reacted with thumbs up emoji

Copy link
Author

Tijgerd commented Sep 26, 2017

True 👍

Copy link

fake-name commented Sep 26, 2017
edited
Loading

Anyways, print()'s documentation explicitly enumerates the valid values for format when val is integral, so I'm still sticking with just change the function of format. If you really want to keep existing behaviour, just place the new enum values in the top bits of format. If anyone is printing in > base 64, they're sufficiently nuts that I think we can ignore them.

It's horrible, yeah, but not any more so then the implementation it's papering over top of.

Copy link
Author

Tijgerd commented Dec 22, 2017

@facchinm any news on this?

Copy link
Contributor

cousteaulecommandant commented Dec 25, 2017
edited
Loading

Some ideas/opinions/thoughts:

  1. Arduino's print() is meant as a simple printing function rather than a fully featured formatting one, so I'm not sure adding more arguments is a good idea.
  2. For more advanced formatting specifications, I think using printf() is a better idea. See Add function "printf" to AVR core class "print" #5938 which adds support for Serial.printf().
  3. Back in the day I thought of including more fields in the format specifier, like
    #define WIDTH(x) ((x)<<8) // up to 127
    #define SIGN (1<<7) // include +/-
    #define ZERO_PAD (1<<6) // pad with zeros instead of spaces
    #define BASE(x) (x) // up to 36
    Serial.print(x, WIDTH(2) | ZERO_PAD | BASE(16));
    
    which would allow cramming multiple format specifiers on a single argument, thus being compatible with the proposal in Variadic print #5829 .
  4. Maybe it is a good idea to just print UNSIGNED integers (byte, word, unsigned long) as fixed width (zero-padded), and SIGNED integers (signed char, int, long) as regular numbers with a sign (e.g. Serial.print(-15,HEX) should print -F, not FFFFFFF1; see Print negative numbers as negative numbers regardless of base #4535 ). This way, a simple conversion would be enough in most cases.


do {
//the argument is only valid for HEX values smaler than 0x10
if(base != 8 || n>=16) arg = 0;
Copy link

Choose a reason for hiding this comment

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

if(base != HEX || n>=16) arg = 0;

Copy link
Author

Tijgerd commented May 2, 2018

@retfie , if I remember correctly, HEX is defines as 16 (By default in the original arduino code)

Copy link

CLAassistant commented Apr 9, 2021
edited
Loading

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
1 more reviewer

@retfie retfie retfie left review comments

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
Print and Stream class The Arduino core library's Print and Stream classes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /