Skip to content

Due SAM Core -Remove compiler Warning and improve readability plus simple documentation fix #4447

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 3 commits into from

Conversation

techpaul
Copy link
Contributor

Discovered a core library warning of signed/unsigned comparison, which at first wa confusing due to font as one variable being 'l' lower case L, actually looked like a number one ('1'). Discovered this temporary variable was actually unsigned being compared to unsigned buffer variables.

This can be tested and found compiling using ALL warnings on Arduino 1.6.7 and Due Board Manager V1.6.6,. Noting that repository has more up to date Stream.cpp to avoid another compiler error that can be seen. Simple empty sketch (empty setup and loop) will show this

This is simple fix in UARTClass.cpp to

1/ Correct temporary variable to signed to match other variable and other similar comparisons in module.

2/ Change variable name to nextWrite for better readability

Other change is to remove questionable wording comment for RingBuffer.h, that implies no one knows what the definition is for.

@sandeepmistry
Copy link
Contributor

@techpaul can you please remove the hardware/arduino/sam/cores/arduino/UARTClass.cpp~ file. It looks like a temporary file from your editor.

@sandeepmistry sandeepmistry added feature request A request to make an enhancement (not a bug fix) Architecture: SAM Applies only to the SAM microcontrollers (Due) labels Jan 18, 2016
@sandeepmistry
Copy link
Contributor

Also, same comment for hardware/arduino/sam/cores/arduino/RingBuffer.h~

@techpaul
Copy link
Contributor Author

Will do thought it was on exclude

@techpaul
Copy link
Contributor Author

Should be visible now

@sandeepmistry
Copy link
Contributor

@techpaul thanks!

I've cherry picked fa0d580.

@cmaglie cmaglie added this to the Release 1.6.8 milestone Jan 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Architecture: SAM Applies only to the SAM microcontrollers (Due) feature request A request to make an enhancement (not a bug fix)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants