Here is a snippet of code that I have been running on Core for months now. The identical program compiles and flashes on Core and on Photon, but the results are different, as described below:
Code snippet:
if(bufferReadout != "") // skip empty log entries
{
int index;
// parse the comma delimited string into its substrings
parser(bufferReadout);
// format the sequence number and place into bufferReadout
bufferReadout = "(S:";
bufferReadout += dest[1];
bufferReadout += ")";
// Determine message type
if(dest[0] == "S") // sensor type message
{
// format the sensor Name from the index
index = dest[2].toInt();
bufferReadout += sensorName[index];
bufferReadout += " tripped at ";
}
else // advisory type message
{
bufferReadout += dest[2];
bufferReadout += " detected at ";
}
// add in the timestamp
index = dest[3].toInt();
bufferReadout += Time.timeStr(index);
bufferReadout += " Z (epoch:";
bufferReadout += dest[3];
bufferReadout += "Z)";
}
bufferReadout.toCharArray(stringPtr, bufferReadout.length() + 1 );
stringPtr[bufferReadout.length() + 2] = '\0';
On a core, it produces the following result (as intended):
(S:13)No one is home detected at Sun May 31 03:55:14 2015 Z (epoch:1433044514Z)
This same exact code, on the Photon, gives the following result:
(S:13)No one is home detected at Sun May 31 03:55:07 2015
Note that the successive building of the string seems to stop before the " Z (epoch: …" is added. This is NOT a character or buffer size limitation, as the string on photon always terminates without the Z (epoch:…" even when the strings are of different lengths.
Yes, the code is probably ugly but it should work and does work on a Core but terminates prematurely on a Photon. Any ideas? Is this my bug or a compiler issue?
@BDUB: I ran your test and got surprising results:
Core:
(S:0)Garage Door tripped at Z (epoch:1433216115Z)Tue Jun 2 03:35:15 2015
(S:1)PIR D tripped at Z (epoch:1433216121Z)Tue Jun 2 03:35:21 2015
Photon:
(S:0)Garage Door tripped at Z (epoch:1433216109Z)Tue Jun 2 03:35:09 2015
(S:1)PIR D tripped at Z (epoch:1433216114Z)Tue Jun 2 03:35:14 2015
With this simple switch, Core and Photon give the same results. I went back that retested the original code:
Core:
(S:0)Garage Door tripped at Tue Jun 2 03:40:29 2015 Z (epoch:1433216429Z)
(S:1)PIR D tripped at Tue Jun 2 03:40:34 2015 Z (epoch:1433216434Z)
Photon:
(S:0)Garage Door tripped at Tue Jun 2 03:40:21 2015
(S:1)PIR D tripped at Tue Jun 2 03:40:27 2015
Yes - it is verified that with the original code the Photon truncates the string but with the string building reversed, Core and Photon both produce the correct results.
Any ideas? Is there some problem with String concatenation on new (Photon) libraries where if the added string starts with a blank (space) it isn’t added anymore? These results are significant but what do they mean?
I was thinking that either the Time.timeStr() function is behaving differently on Core vs Photon, with something going on with the null termination, or maybe concatenating onto a null-terminated string behaves differently?
which does work and still is concatenating onto a null-terminated string (but not a String to a string/String).
But since timeStr() returns a String instance, trying bufferReadout += Time.timeStr(index).c_str(); instead might reveal some issue with the String class - either the += operator overload or the (const char*) cast operator.
Yeah, it’s been too long since I dealt with the details of C. I thought that regular strings were null-terminated, but I couldn’t remember for sure. So yeah, likely a bug in how += is handled for the String class.
I’ve been using loosely-typed languages as a web developer so many years, I have to consult references when I’m trying to resolve variable casting and such in C/C++.
I tried the change suggested by BDUB. No joy - it is the same. I tried deleting the space before the Z (i.e. "Z (ep… vs " Z (ep… No joy - it is the same. I tired commenting out the string building lines right after bufferReadout += “Z (epoch:”; No joy - same result. I wanted to try some Serial.println() debugging but I found that my computer (Win 7) doesn’t have a device driver for the Photon and Windows can’t find one. Win device manager reports this as Other devices Photon with WiFi. I tried updating the driver both with a local search and an Internet search but cannot get a serial device driver into my computer - so No Joy here too. Finally, I tried ScruffR’s suggestion and guess what - IT WORKS!
ScruffR: what does this do and why do you think that it corrected the problem?
BDub: I’l love to call you. How do I get in touch with you directly?
All: Photon driver for Windows?
@BobG, I guess you tried the suggestion to use c_str().
This function returns a const char* string instead of a String object.
While the += operator of the String class taking a const char* is proven to work as expected, the += operator taking a &String or &&String is not.
It’s a while since I checked the String implementation, but last time I looked the assignment and concat operators taking String where still marked “experimental”.
This is one of the reasons why I chose to stay away from String wherever possible (another is possible heap fragmentation ;-))
Thanks to everyone for their investigative inputs! We clearly have a regression from 0.3.4 to 0.4.0 - I’ll look into this today so we can roll out a fix for 0.4.1 if needed.
There are two bugs hiding in String right now that I have made Matt @mdma aware of. Here is some work-around code for you test–I am assuming dest[] is declared as int:
if(bufferReadout != "") // skip empty log entries
{
int index;
// parse the comma delimited string into its substrings
parser(bufferReadout);
// format the sequence number and place into bufferReadout
bufferReadout = "(S:";
bufferReadout += (long)dest[1];
bufferReadout += ")";
// Determine message type
if(dest[0] == "S") // sensor type message
{
// format the sensor Name from the index
index = dest[2].toInt();
bufferReadout += sensorName[index];
bufferReadout += " tripped at ";
}
else // advisory type message
{
bufferReadout += (long)dest[2];
bufferReadout += " detected at ";
}
// add in the timestamp
index = dest[3].toInt();
//work-around!
String tmp = Time.timeStr(index);
bufferReadout += tmp;
bufferReadout += " Z (epoch:";
bufferReadout += (long)dest[3];
bufferReadout += "Z)";
}
bufferReadout.toCharArray(stringPtr, bufferReadout.length() + 1 );
stringPtr[bufferReadout.length() + 2] = '\0';
Thanks all for the help. I am happy to hear that mdma will look into the regression issue, as the original code works fine for Core but not for Photon. I understand what ScruffR is saying; however, the String class concatenation function is supposed to work (it is documented as such) and thus it is best if it is treated as a bug and fixed (long term, I will use the “.c_str()” fix that ScruffR first suggested and works well for now).
C++ does not seem to have a Stringbuffer class, like e.g. Java, so I agree that this method of string building most likely causes heap fragmentation. Since C strings are mutable, I had once tried to initialize the String object with the longest possible string that my application would generate and then tried to overwrite this data with the real data. That didn’t work out, so I have accepted the potential for heap fragmentation for now. My current work is researching some application algorithms, so I am less concerned about neat and clean code at the moment, so long as it runs reliably (which it has been on Core). After this investigative stage is complete, I might take a pass back through the code and use low level C string functions as opposed to the String class to make this cleaner. But that will depend upon the ultimate outcome of the research.
Thanks again for all of your help and thanks, mdma, for treating this as a Particle bug.
@bko: thanks for making us aware of these bugs in the String class. However, this particular bug seems to be in the Time class (Time.timeStr() method, at least). ScruffR found this and his fix works, albeit it is a bug for the Particle folks to investigate since the original code works with on a Core.
When the integer epoch time is larger than 7 decimal digits, Photon will hard fault (SOS and one red flash). Why the Core does not do this is a mystery since the bug has been there for a while.
The anonymous String object concatenation is newly working after some compiler issues were worked out, but I think there is still a bug there.
@bko: strange that you encounter this. Here is a recent result from my Photon using the code discussed above:
(S:12)No movement detected at Wed Jun 3 02:51:09 2015 Z (epoch:1433299869Z)
The integer epoch time is 10 decimal digits long. I did not cast this into a long, although a long and an int are the same on a Core/Photon. But clearly there are some issues with the Time class that crept in when Core was ported to Photon.
What I did (effectively) is:
String timeStamp = "";
timeStamp += Time.now();
...... later on .....
int index;
index = timeStamp.toInt();
String message;
message = "";
message += Time.timeStr(index).c_str(); // ".c_str()" is bug fix per ScruffR
message += ........... // etc.
