Hi all!
I use an Arduino Mega to measure, control and log dissolved oxygen values in fish tanks. To measure dissolved oxygen, I use two FireStingO2 optical oxygen sensors that are connected to the Arduino via Serial.
I'm a biology student and this is my first big project that involves coding and microprocessors, so my learning curve was very steep and the sketch looks - even to me - very convoluted and probably, there's a lot of clutter in it.
The system has now been operating for over 8 weeks continuously and more or less successfully. However, there are several issues with this system that I would like to ask anybody interested to help me with.
Several sections of the code are hardcoded to fit my application (measurement with two sensors, using all 4 channels each), because I desperately had to get it to work. If anybody would like to give it a look and comment on how to make the code more adaptive to e.g. a different number of measurement channels , I'd be very grateful. Also, I simply copied the functions that use serial communication to switch between Serial1 and Serial2 on the Arduino Mega. Is there a better way to do this?
The program is designed to run 24h non-stop for 8 weeks. And it does work for the better part of the time but every now and then, there is a communication error between the Arduino and the sensor and I can't wrap my head around why it happens. The basic principle is simple:
Arduino sends measurement command "SEQ 1\r"
Sensor echoes measurement command if received correctly - so far it has always worked
I included a communication check using strncmp() to compare outgoing and incoming data. And every now and then - at completely random timepoints - the sensor gets stuck echoeing the measurement command "SEQ 1" when it should report the values "REA 1 3 4 95682"
I tried everything I can think of to resolve this issue: clearing the char arrays for incoming serial data, sending only carriage returns "\r" to elicit an error message from the sensor and rebooting the Arduino using
void(* resetFunc) (void) = 0;
So far, only the reboot has solved the problem. And this is how I have been running this system for the last 8 weeks. However, I worry that these frequent reboots lead to problems (e.g. memory fragmentation) and quite frankly, I would really like to know what's going on here.
For anybody who's interested, I attached the arduino sketch, a schematic drawing of the whole system and a puTTY logfile in which the errors occurred.
I'm sure you can strip down that 741 kB log file to a few lines: some good, some bad, showing what's going on. This is WAY too long to dig through.
Your sketch indeed contains lots of repetition, like the logging of a date can be placed in a function. Makes code readable and more maintainable.
Make your code more adaptable by using more arrays, and setting a variable at the tp of your code defining how many sensors you have, e.g.:
const byte NSENSORS = 4;
Also this construction is interesting:
temp[0] = relayArray[0][i]; // store higer value + relayPin in temp array
temp[1] = relayArray[1][i];
temp[2] = relayArray[2][i];
temp[3] = relayArray[3][i];
relayArray[0][i] = relayArray[0][j]; // move lower value to position i
relayArray[1][i] = relayArray[1][j];
relayArray[2][i] = relayArray[2][j];
relayArray[3][i] = relayArray[3][j];
relayArray[0][j] = temp[0]; // insert higher value at position j
relayArray[1][j] = temp[1]; // now the values at i and j have switched places in the relayArray
relayArray[2][j] = temp[2];
relayArray[3][j] = temp[3];
This, with the variable set above, can be shortened to:
for (uint8_t k = 0; k < NSENSORS; k++) {
temp[k] = relayArray[k][i]; // store higer value + relayPin in temp array
relayArray[k][i] = relayArray[k][j]; // move lower value to position i
relayArray[k][j] = temp[k]; // insert higher value at position j, now the values at i and j have switched places in the relayArray
}
wvmarle, thank you for your suggestions!!
You're completely right, I exchanged the logfile with an abbreviated version that shows a relevant time window: first two successful measurement cycles and then the mismatch between echo and sent string that repeats until the arduino restarts and then another successful measurement cycle.
Also good advice with the additional loop. To sort values in an array was a mind-bender for me, so I had to do it step by step to not get confused and this part of the code was still a relict of that learning process. I changed the passage as you suggested.
For sorting elements in an array, look at the qsort() function.
Too much work to fully analyse that sketch. Bring it down to basic communication with one sensor: just send the command, read the raw data, and print that out. That's the way to test whether you have your communication correct. Then when you reliably receive the response, thing about parsing the data.
One thing I notice is the while (Serial.available() > 0) part (together with tests of two other bools). You appear to expect to see the full reply in the Serial buffer. That's usually not the case; when reading Serial input I normally don't expect to see more than one character, after all at a typical 9,600 bps it takes over 1 ms for a character to arrive. Usually it's just checking for a character in the Serial buffer, read that character, put it in my buffer, and continue doing other things until the end marker is received.
I basically copied this piece of code from Robin2's tutorial on serial communication (link) - as far as I understand, it does always only read one character, stores it in the receivedChar[] array and then moves to the next character and stores it in the adjacent position in the receivedChar[] array until the endmarker is received.
while (Serial1.available() > 0) { // only read serial data if the buffer was emptied before and it's new data
delay(5);
rc = Serial1.read();
if (rc != endMarker) {
receivedChars[ndx] = rc; // store the latest character in character array
ndx++;
if (ndx >= numChars) { // make sure that array size is not exceeded
ndx = numChars - 1;
}
}
else {
receivedChars[ndx] = '\0'; // terminate the string if the end marker is received
ndx = 0;
comCheck = false; // the incoming string has not yet been checked with the sent string
}
}
I took your advice and am currently testing different communication sketches with the sensor, it seems that the delay() between sending a command and reading the response is crucial. This is probably due to the response time of the sensor. It also seems that the serial buffer is cleared with any new incoming data, is that correct?
I found the following:
Send measurement command to sensor
(Sensor echoes measurement command)
Send readout command to sensor
(Sensor echoes readout command and value)
Receive serial data >> Only the most recent sensor Echo (readout command) is in the serial buffer
StefanM:
I took your advice and am currently testing different communication sketches with the sensor, it seems that the delay() between sending a command and reading the response is crucial. This is probably due to the response time of the sensor. It also seems that the serial buffer is cleared with any new incoming data, is that correct?
It makes sense that the sensor needs some time to respond, but the delay should not be necessary with proper coding. Also if the sensor for whatever reason is a little slower it will still cause problems.
The serial buffer gets emptied as you read it.
I'd rather try something like this for reading the response:
void loop() {
if (Serial.available()) {
Serial.println((uint8_t)Serial.read());
}
}
What this does: every time a character comes in, it's read and printed out as value - if you expect characters, it'd be the ASCII code of those characters. In that case you could also try:
void loop() {
if (Serial.available()) {
Serial.print((char)Serial.read());
}
}
Then using millis() based timing you can send your read command every 5 second or so:
void loop() {
static uint32_t lastReadCommand;
if (millis() - lastReadCommand > 5000) {
Serial.print("<read command>"); // replace this with the actual command for this sensor.
}
if (Serial.available()) { // Print the response as it comes in.
Serial.print((char)Serial.read());
}
}
Instead of (or in conjunction with) the printing of the characters you can of course also place them in a buffer, and when the last character is arrived (the end character) you call a function to parse the data: then you have a complete message.
Keep track of how many characters you received to prevent buffer overflows.
Check for start characters (if there is one) and if you receive this just ignore whatever you received and start buffering again.
wvmarle, thank you for the input!
I've been busy with experiments the last week but I'll try and see if it helps to switch from while(Serial1.available() > 0) to if(Serial1.available()).
The sensor replies with a mixed string of characters and numbers (ASCII coded), the length varies with the measured value and there's no start marker, i.e. if it measures 99.999% air saturation, it sends "REA 1 3 4 99999" (length = 17) and at 100.999% it sends"REA 1 3 4 100999" (length = 18).
However, none of these should lead to buffer overflow if read out.
If it measures >100% humidity you have to get really worried about the reliability of the thing.
The REA is your start marker.
It may send a \n, \r or \0 as end marker, you don't see that if you just print the values as char because they're unprintable. If you want to see the ASCII values of the received characters, including unprintable ones, change the print to:
