Skip to content

Conversation

ChocoChipset
Copy link

Pull request to solve issue #1962.

Floating point numbers with a trailing . character would be rendered as integers.
For example: .032 => will become 32.

The new function peekNextDigitOrPoint takes into account this and is used in the function parseToFloat.

Let me know if any re-factor or modification is required.

@matthijskooijman
Copy link
Collaborator

It looks like a correct fix for the issue, thanks. I do have a few remarks:

  • The commit message is a bit short. Preferably a commit message has short first line the summarizes what changed (e.g., "Allow a leading decimal separator in Stream::parseFloat"), then an empty line and then more lines with detailed explanations. A reference to the github issue definately belongs in the commit message, but not in the first line. Also, of you write "Fixes Serial.parseFloat doesn't parse leading decimal points #1962", github can automatically close the issue for us.
  • You're now duplicating a bunch of code. It might be better to instead add a bool allowPoint argument to peekNextDigit.
  • While you're here, it might be good to rename peekNexDigit to something like skipUntilDigit, since the current name really doesn't describe what the function does. This should preferably happen in a separate commit, probably before the main fix.

@ChocoChipset
Copy link
Author

Thanks for all the pointers!

I agree on all remarks and will be resubmitting the pull request in brief.

Fixes arduino#1962 :
Floating point numbers with a trailing '.' character would be rendered as integers.
For example: .032 => will become 32.
@ChocoChipset
Copy link
Author

Ready now, @matthijskooijman! ;]

@matthijskooijman
Copy link
Collaborator

Looks good to me!

Now, it's up to the dev team to merge.

@ChocoChipset
Copy link
Author

Purrfect!

@Chris--A
Copy link
Contributor

This PR is incomplete.

The changes require Stream.h to be updated also. I have a new PR which fixes this: #3337

@facchinm
Copy link
Member

Superseded by #3337

@facchinm facchinm closed this Jul 13, 2015
@ffissore ffissore modified the milestone: Release 1.6.6 Aug 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Related to the code for the standard Arduino API Type: Bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants