2

I have an Arduino Nano, and I have some issues with it. Basically it needs to control the brightness of a lamp via PWM (plus some circuitry of course). It's hooked up via serial to a little x86 embedded board which runs Linux, and that board is telling the Arduino the desired brightness level (0-255) every 5 seconds over serial. So the only thing the Arduino has to do is retrieve numbers from the other device over serial, and set the PWM output to those values. Here's the code that runs on the Arduino (I inserted some comments so it's easier to read):

Edit: I removed the original code to clean things up as it's not relevant anymore, but you can still view it here.

There's nothing fancy about the code really. It checks for incoming data about 10 times a second, and if there's any, it converts it into an int and sets the PWM output to that value. I know, that part where I manually extract the number characters probably could be replaced by Arduino library calls for cleaner code, but whatever. It works just fine.

The incoming data as I mentioned is the textual representation of an integer ranging from 0-255 followed by a new line character. This data is sent to the Arduino once every 5 seconds.

So whenever I turn this on, it works just fine for a day or so. But this is supposed to be a system that's constantly on 24/7. If I leave it running for a day or so, the output will start acting weird. I already checked, and the values that the other device sends are perfectly normal. It's the Arduino that messes up something. The brightness is NOT what it needs to be. The Arduino is outputting wrong PWM values. If I simply reset the Arduino (without restarting the other device), everything goes back to normal, and it works again for a day, so the problem is definitely in the Arduino somewhere. But I don't think it's in my code. This piece of code looks super simple and innocent, and I bet it is. I have no idea what causes this.

Could this be a hardware problem? I do have another Arduino, so I could try replacing it with that one. I haven't tried that yet since I already put all components in a case, fixed everything in place with screws, and soldered wires directly to the Arduino, so I would rather not replace it if it's not necessary.

So, any ideas on what could cause this?


Update:

I updated the programs on both devices to use a more error-prone protocol for communicating (as suggested here), which is kind of similar to the NMEA 0183 protocol (basically a simplified version of that). The sender sends the data in the following format now:

  • A single $ character to indicate the beginning of the data
  • The data itself (which is the textual representation of a single integer)
  • A single * character to indicate the end of the data
  • An exactly two digit long hexadecimal checksum value. This is the bitwise XOR of the ASCII codes of all the characters between $ and * (not including those). Letters (if any) are always uppercase.
  • The packet is finished up with either \n or \r\n

An example would be $178*3E\n, where \n of course means a newline character.

The code running on the Arduino now is the following:

#include <Time.h>

#define _analogWrite(a, b) analogWrite(a, 255-(b))
#define clamp(X, minVal, maxVal) min(max(X, minVal), maxVal)

const PROGMEM int pwmPin = 3;
time_t lastUpdate = 0;
String incoming = "";
int pwmValue = 0;

inline void flushSerialBuffer() {
    while (Serial.available()) Serial.read();
}

void setup() {
    Serial.begin(9600);
    Serial.setTimeout(500);
    pinMode(pwmPin, OUTPUT);
    _analogWrite(pwmPin, 0);
}

void loop() {
    time_t currentTime = now();

    if (Serial.available()) {
        delay(250); // Just to make sure that all data has arrived in the buffer
        incoming = "";
        incoming = Serial.readStringUntil('\n');
        flushSerialBuffer();
        int b = incoming.indexOf('$') + 1;
        if (b == 0) return; // -1 was returned
        int e = incoming.indexOf('*', b);
        if (e == -1) return;
        int checksum = 0x0;
        for (int i=b; i<e; i++) checksum ^= incoming[i];
        String hexString = String(checksum, HEX);
        hexString.toUpperCase();
        if (hexString.length() < 2) hexString = String("0") + hexString;
        if (!hexString.equals(incoming.substring(e+1, e+3))) return;

        pwmValue = incoming.substring(b, e).toInt();

        if (pwmValue < 0 || pwmValue > 255) pwmValue = 0;
        lastUpdate = currentTime;
    }

    if (currentTime - lastUpdate > 30) pwmValue = 0;

    _analogWrite(pwmPin, pwmValue);

    delay(250);
}

It seems like it's working so far, although I'm still testing if this is reliable over a longer period of time. I will edit the question once again to let you know.


Update 2:

Still no success. After a while it just gets stuck on one PWM value and keeps outputting that, regardless of what it receives over serial. It's kind of like it ignores the incoming data after a day or so. I can verify that the other device still sends data correctly, since if I only restart the Arduino, everything goes back to normal.

  • The Arduino is outputting wrong PWM values; could you be more specific? Does "resetting" the program that is sending the serial data also fix the problem? Just something that I noticed, is that you don't actually check if a newline has been sent. If your loop reads the serial buffer while the x86 is still sending the data, your program could read e.g. 25, while the x86 is trying to send 250\n. Serial.readStringUntil('\n') could help in that case. – Gerben Feb 20 '16 at 19:51
  • @Gerben Please read my comment here – adam10603 Feb 20 '16 at 20:02
  • How often are you sending values to the arduino? – Gerben Feb 20 '16 at 20:05
  • It's in the question. Once every 5 minutes. Also, I should say that using Serial.readStringUntil('\n') creates potential bugs. Imagine if the other device sends faulty data, or maybe noise gets into the system, so there would be junk characters after the end line character. That command would only read up until and including that character, but no further. Anything after it would be left in the buffer and picked up by the next iteration of the loop, and that junk data could potentially be converted into an integer causing wrong output. – adam10603 Feb 20 '16 at 20:09
  • Edit: once every 5 seconds. Sorry. – adam10603 Feb 20 '16 at 23:11
1

You have a race condition in your code: if you try to read a message while it's being sent, you may catch only a partial message, and end up reading the wrong numbers. My bet is that this is exactly what is biting you. Once this happens once, it is bound to happen again for some time, since 5 seconds is a multiple of your 100 ms delay.

The right thing to do would be to wait until you get the new-line character (then you know you have a complete message) before trying to parse the message. A quick and dirty hack is to just delay(10) after Serial.available() but before Serial.readString().

  • Serial.readString() does have a timeout associated with it. In my case it's 250 milliseconds. Which means the following: Serial.available() returns true, indicating that there's incoming data. Then Serial.readString() reads all data that it can read in the next 250 milliseconds which is plenty enough time for the transmission to finish, and the entire data to be in the buffer. This is what guarantees that the entire message is read correctly. So I'm constantly reading the input for 250 milliseconds. – adam10603 Feb 20 '16 at 19:50
  • If Serial.readString() did not have a timeout, but instead just read the buffer exactly once, and returned, you would be right. But instead it waits a certain amount of time before returning, waiting for potential incoming data. Since I don't think that transferring a couple bytes at 9600 baud takes anywhere near 250ms, this should be fine. – adam10603 Feb 20 '16 at 19:56
  • I checked the source code of Stream::readString(). You are right, it waits for every byte. I thought it would only wait for the first, the documentation is not clear about that. Then, I cannot see the problem with your code. I can only suggest you echo back what you read, with some extra delimiters to show what you interpreted as message boundaries. This could hint you about what is going on. – Edgar Bonet Feb 21 '16 at 7:26
  • Yeah maybe. The problem is that it's very hard if not impossible to reproduce the problem on demand. I'll try though. – adam10603 Feb 21 '16 at 13:40
  • @adam10603: If you send the messages at a higher frequency, the problem is likely to show up sooner. – Edgar Bonet Feb 21 '16 at 14:51
1

After trying the original code using Arduino IDE 1.6.7, I do not see a problem with using up memory.

Depending on the nature of the errors you are seeing, another possible explanation is that you are getting spurious data between real events. If electrical noise or whatever produces one or more characters on the serial line in the long interval between real data points, the current code will treat it as a new data point. If the bogus characters are numeric, they will be sent to the pwm. In the more likely case of non-numeric data, this code converts them to a value of 0 and sends that to the port. This situation is likely if the Arduino is more than a few feet from the PC. A slew rate limited differential driver and receiver like the MAX488 should greatly reduce the chance of this happening. Also make sure there is a good ground between the ends.

You could also reduce the chance of undetected data errors by delimiting your data with something like a '$' to start, then the data, then '*' then a checksum then a newline. The checksum could be simply the sum of all the characters up to the *, truncated to 8 bits and sent as 2 hex characters. This is what the NMEA protocol used by GPS receivers does. At the receiving end you only accept data packets with the proper format and checksum.

  • The two devices are literally right next to each other, the cable that connects them is about 10 inches long. I don't think it's interference. Although I will implement characters that mark the beginning and end of the data plus a checksum value, that's always a good idea. – adam10603 Feb 26 '16 at 9:34
  • I did some modifications to the system based on your suggestions. See the updated part in the question. I'm still testing if it works properly now. – adam10603 Feb 26 '16 at 17:12
0

Your problem may be related to the dynamic memory allocation of your String objects. Each time the Arduino receives data, you are creating two new String objects. Perhaps the memory is not being released properly, causing eventual exhaustion of your RAM.

You could make your Strings global and empty them out each time, but a far better solution would be to replace all your string manipulation with a call to Serial.ParseInt() so your big if block becomes

if (Serial.available()) { pwmValue = Serial.ParseInt(); while(Serial.available() > 0 { Serial.read(); // flush anything after the integer // Save the current time lastUpdate = currentTime; }

Serial.ParseInt also makes use of the timeout so you don't have to worry about partial values being read.

  • This is exactly what I did :) See my comment here. I'm still testing it – adam10603 Feb 21 '16 at 18:50
  • In the posted code, no String survives the return from loop(). – Edgar Bonet Feb 21 '16 at 20:07
  • The Strings don't survive but apparently the heap can still get fragmented and eventually run out of space. I have not verified that this is what is causing the problem here, but I think the code I suggested above is more robust. – Jim Harman Feb 21 '16 at 20:39
  • I tried the original code and it does not appear to have a memory problem. See 2nd answer below. – Jim Harman Feb 22 '16 at 16:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.