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

Two bug fixes: TCPsend argument type, and missing end-of-buffer check #86

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

Conversation

cornwarecjp
Copy link

Changed argument type of TCPsend from char * to uint8_t *. Without this change, compilation failed on type incompatibility in the arduino.cc IDE version 2:1.0.5+dfsg2-4 (as distributed with Debian Jessie), when compiling for Arduino UNO. I guess the incompatibility is because of the signed/unsigned difference.

Moved the end-of-buffer check from the outer loop to the inner loop in readline(): this is necessary, because the inner loop is capable of traversing through the buffer. Also, use sizeof() to determine the end of buffer, like in readRaw().

Tested superficially by compiling and running this together with my own sketch code. I verified that this compiles without warnings, and I can communicate with my FONA (fona.begin() and fona.unlockSIM() are successful).

Without this change, compilation failed on type incompatibility in the arduino.cc IDE version 2:1.0.5+dfsg2-4 (as distributed with Debian Jessie), when compiling for Arduino UNO. I guess the incompatibility is because of the signed/unsigned difference.
…n readline(): this is necessary, because the inner loop is capable of traversing through the buffer. Also, use sizeof() to determine the end of buffer, like in readRaw().
@cornwarecjp
Copy link
Author

Travis CI's build failure seems completely unrelated to my changes: it only reports this error on some builds:

fatal error: SoftwareSerial.h: No such file or directory

and SoftwareSerial.h is apparently included from the example sketches. I'd expect that Travis CI will also fail to build the current contents of the master branch.

Copy link

@simonmags simonmags left a comment

Choose a reason for hiding this comment

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

Changes Look Good, recommend this be made part of the main stream release.

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