Nick, where is the unfixed memory leak bug?
Check out this thread:
There is a suggested fix, and I also suggest a way of doing it without fiddling with the library.
Short answer: the bug is in free.
The minimal change would be to change:
char* getResponse()
{
...
char response[50];
_response.toCharArray(response, 50);
return response;
}
to:
char response[50];
char* getResponse()
{
...
_response.toCharArray(response, 50);
return response;
}
But then what's the point of returning a pointer to a global variable?
Also I am still concerned about all those String objects.
while (client.connected() && client.available())
{
processIncomingByte(client.read());
}
If the client is still connected, but there is a delay in the server sending the code, this loop will end before all the data has been sent. That may not be what you want to do.
Wow! Lots of great info here! Thank you all! I'm going to try and work backword through it... Paul, if I'm understanding you correctly, hanging my hat on client.available() isn't ideal. Does something like (c=client.read()) give a boolean true or false? Then I could use something like if (client.connected() { while (client.read())... so I could stay at this loop until the entire message is received?
Nick, Skyjumper,
I'm understanding defining the response char array globally is the better way to go. I can do that. When I was researching memory leaks, it seemed to be the concensus that defining your variables within a function was a good thing because when that function lost focus, the memory would be returned to the system. But maybe that could still lead to fragmentation? Regardless, I'll define it globally. Also, the end result (what gets sent through the socket back to the dll) has to be a char array. The only reason I used all those string variables was so I could concatenate the whole thing together. If there is a way to build the response without strings, I'm all for it. I'm just not sure how. There will be some fixed variable names but the values could be integers or floats. I know this is no excuse but I'm only in my sixth week of C++. Prior to owning my first Arduino, I'd never used it. That goes for C# as well.
Again, thank you all for the most excellent feedback. It's very much appreciated!
don
Does something like (c=client.read()) give a boolean true or false?
How could it? It needs to return the character read.
You need to test client.connected() and client.available() independently. There may be times when there is a client connected but there is no data to read. There will be times when there is no client connected.
The client disconnects when the server is done sending data to it, usually. So, the server data has all been read only when client.connected(0 returns false.
Then I could use something like if (client.connected() { while (client.read())... so I could stay at this loop until the entire message is received?
Why do you think you need to do this? Use a flag to determine if you have read all the client data, and don't try to use the client data until you have set that flag to true. Set it back to false after using the data.
donjuevo:
When I was researching memory leaks, it seemed to be the concensus that defining your variables within a function was a good thing because when that function lost focus, the memory would be returned to the system.
Yes, but you can't then return a pointer to the memory that has been returned to the system. Do you see why?
Nick,
When you put it that way... I'm an idiot. Pointers are a brand new concept to me and I feel like I'm only just now beginning to understand them. I thank you for that.
I've made some revisions but still hitting walls. The important bits as they stand now...
char response[50] = "";
int LV = 0;
int RV = 255;
float PS = 0;
void getResponse()
{
// Gather sensor data here
PS = 13.25;
char buff[16];
String _PS = dtostrf(PS, 5, 8, buff);
Serial.print("\r\n");
Serial.print(_PS);
sprintf(response, "|LV:%d|RV:%d|PS:%s", LV, RV, _PS);
Serial.print ("\r\n The Response from getResponse function: ");
Serial.write(response);
Serial.print("\r\n");
}
This compiles but on execution never displays the message "The Response from getResponse function..." in the serial monitor which makes me think its getting stuck on the sprintf line. If I write that line like this: response = sprintf("|LV:%d|RV:%d|PS:%s", LV, RV, _PS);
the compile error is Invalid conversion from 'int' to 'const char*'. In looking at the Arduino language reference, it seems to me that this should compile but again, a lot of these C++ variable nuances are still lost on me. Nick had pointed out that my earlier use of a lot of strings to concatenate a response together was unwise. Sprintf seemed a better, cleaner way.
My apologies for being the squeaky wheel.
don
Get rid of the String _PS. The result of the conversion is being put in buff. Use that in your call to sprintf.
I made that change and still getting the same compile error.
sprintf(response, "|LV:%d|RV:%d|PS:%s", LV, RV, buff);
Can you post all the code please?
Sure thing Nick...
#include <SPI.h>
#include <Ethernet.h>
byte mac[] = { 0x90, 0xA2, 0xDA, 0x0D, 0x07, 0xBC };
IPAddress ip(192,168,3, 111);
EthernetServer server(12821);
EthernetClient client;
typedef enum { NONE, _LX, _LY, _RX, _RY, _LT, _RT, _AB, _BB, _XB, _YB, _LS, _RS, _ST, _BK, _DU, _DD, _DL, _DR, _LJ, _RJ, _BG } states;
states state = NONE;
unsigned int currentValue;
int _currentValue;
char response[50] = "";
int iterations = 0;
// GamePad State Variables
int LX = 0; int LY = 0;
int RX = 0; int RY = 0;
int LT = 0; int RT = 0;
bool AB = false; bool BB = false; bool XB = false; bool YB = false;
bool RS = false; bool LS = false;
bool DU = false; bool DD = false; bool DL = false; bool DR = false;
bool ST = false; bool BK = false;
bool LJ = false; bool RJ = false;
bool BG = false;
// Response Variables
int LV = 0;
int RV = 0;
float PS = 0;
void setup()
{
Serial.begin (115200);
state = NONE;
Ethernet.begin(mac, ip);
server.begin();
Serial.println("Debugging...");
}
void applyVariables()
{
LV = LT;
RV = RT;
}
void getResponse()
{
// Gather sensor data here
PS = 13.25;
char _PS[16];
dtostrf(PS, 5, 8, _PS);
Serial.print("\r\n");
Serial.print(_PS);
response = sprintf("|LV:%d|RV:%d|PS:%s", LV, RV, _PS);
Serial.print ("\r\n The Response from getResponse function: ");
Serial.write(response);
Serial.print("\r\n");
//return response[n];
}
void handlePreviousState()
{
switch (state)
{
case _LX:
_currentValue = currentValue - 255;
LX = _currentValue;
break;
case _LY:
_currentValue = currentValue - 255;
LY = _currentValue;
break;
case _RX:
_currentValue = currentValue - 255;
RX = _currentValue;
break;
case _RY:
_currentValue = currentValue - 255;
RY = _currentValue;
break;
case _LT:
LT = currentValue;
break;
case _RT:
RT = currentValue;
break;
case _AB:
AB = (currentValue == 1) ? true : false;
break;
case _BB:
BB = (currentValue == 1) ? true : false;
break;
case _XB:
XB = (currentValue == 1) ? true : false;
break;
case _YB:
YB = (currentValue == 1) ? true : false;
break;
case _LS:
LS = (currentValue == 1) ? true : false;
break;
case _RS:
RS = (currentValue == 1) ? true : false;
break;
case _ST:
ST = (currentValue == 1) ? true : false;
break;
case _BK:
BK = (currentValue == 1) ? true : false;
break;
case _DU:
DU = (currentValue == 1) ? true : false;
break;
case _DD:
DD = (currentValue == 1) ? true : false;
break;
case _DL:
DL = (currentValue == 1) ? true : false;
break;
case _DR:
DR = (currentValue == 1) ? true : false;
break;
case _LJ:
LJ = (currentValue == 1) ? true : false;
break;
case _RJ:
RJ = (currentValue == 1) ? true : false;
break;
case _BG:
BG = (currentValue == 1) ? true : false;
break;
}
//Serial.write(state);
//Serial.print(": ");
Serial.print(currentValue);
Serial.print(":");
currentValue = 0;
}
void processIncomingByte (const byte c)
{
//Serial.write(c);
if (isdigit (c))
{
currentValue *= 10;
currentValue += c - '0';
}
else
{
handlePreviousState();
switch (c)
{
case 'A':
state = _LX;
break;
case 'B':
state = _LY;
break;
case 'C':
state = _RX;
break;
case 'D':
state = _RY;
break;
case 'E':
state = _LT;
break;
case 'F':
state = _RT;
break;
case 'G':
state = _AB;
break;
case 'H':
state = _BB;
break;
case 'I':
state = _XB;
break;
case 'J':
state = _YB;
break;
case 'K':
state = _LS;
break;
case 'L':
state = _RS;
break;
case 'M':
state = _ST;
break;
case 'N':
state = _BK;
break;
case 'O':
state = _DU;
break;
case 'P':
state = _DD;
break;
case 'Q':
state = _DL;
break;
case 'R':
state = _DR;
break;
case 'S':
state = _LJ;
break;
case 'T':
state = _RJ;
break;
case 'U':
state = _BG;
break;
}
}
}
void loop ()
{
client = server.available();
if (client)
{
Serial.println("Client Connected");
while (client.connected() && client.available())
{
processIncomingByte(client.read());
}
applyVariables();
getResponse();
Serial.print ("\r\n The response from Loop(): ");
Serial.write(response);
Serial.print("\r\n");
client.write(response);
client.flush();
client.stop();
iterations ++;
Serial.print("\r\n");
Serial.print (iterations);
Serial.print (" Client Disconnected");
}
}
Thank you again for taking the time.
don
response = sprintf("|LV:%d|RV:%d|PS:%s", LV, RV, _PS);
You need to look up how sprintf works. The first argument is "where the string goes". The second argument is the formatting information. The third argument on is the data to be formatted.
You forgot the format string for the argument:
http://www.cplusplus.com/reference/clibrary/cstdio/sprintf/
OOPS, Nick already got there...
Fellas, my fault. Sprintf exists in PHP and I use it all the time. The way I wrote it is how it's handled in PHP. Sorry. It did compile and early tests are looking good. I appreciate all the help. I really do. I think I'm finally ready to start doing some work in the real world with speed controllers, relays, a soldering iron, some duct tape and a tube sock. Sorry, had to toss in a MacGuyver reference.
Thank you all again!
don
I suppose I could say PHP is evil, but I don't really believe that. It allows developers to be evil 
Quite a bit of PHP is taken directly from the standard C library, so you'll find that many of the functions have the same arguments. But they changed some of them, to make things "easier." If you come to PHP from a C background, it can drive you a bit nuts. You had the opposite experience.
Im not sure how much of the errors you saw. I use Arduino IDE 1.0. So when I pasted your sketch in and compiled it, the compiler gave me the errors in red. It emitted several lines of useful information, which included the line number and the fact that the error referred to the second argument. Look for that stuff, and then the first thing to do is to check the way the function is supposed to be called. Very often, its a matter of parameters being reversed, forgotten, whatever.
But let the compiler help you. Sometimes the errors messages are cryptic, but they always tell you right where the complaint comes from.
Sometimes the errors messages are cryptic, but they always tell you right where the complaint comes from.
At least they point to the last place the compiler thinks that maybe it understood what was happening.