next topic »

[on:]

I've got a sketch that seems to stop running after a long period of time. It doesn't reset itself, it just becomes unresponsive. Typically, it will run for about 2 weeks, then fail. In some cases it fails in just a few hours. At any rate, clicking the reset button or cycling the power will get it working again. If someone has seen something like this before and can offer advice, that would be great. Also, are there any do's and dont's for sketches that need to run for months at a time?

I will post the code here in case anyone wants to look at it. The code has three functions:
1) Read RFID card serial numbers from a Wiegand card reader (on interrupt pins) and send them out over serial
2) Detect button presses (on a digital input pin) and send a message out over serial
3) Read and interpret commands sent over serial to control a relay on a digital output pin

Sketch.ino

Code:

#include "TwicReader.h"

int buttonPin = 4;
int relayPin = 5;
int ledPin = 13;

bool buttonDown = false;
char serialBuffer[256];

void setup() {
 
  pinMode(ledPin, OUTPUT);
  pinMode(buttonPin, INPUT_PULLUP);
  pinMode(relayPin, OUTPUT);
  Serial.begin(9600);
  Serial.print("|START|");
  TwicReader.begin();
}

void loop() {
  heartbeatLED();
  TwicReader.read();
  updateButtonState();
  checkForSerialCommands();
}

void heartbeatLED()
{
  // Heartbeat
  int ledSetting = millis() % 3000;
  if(ledSetting < 250 || (ledSetting > 500 && ledSetting < 750))
    digitalWrite(ledPin, HIGH);   // set the LED on
  else
    digitalWrite(ledPin, LOW);    // set the LED off
}

void updateButtonState()
{
  if(digitalRead(buttonPin) == LOW && !buttonDown)
  {
    delay(100);
    if(digitalRead(buttonPin) == LOW)
    {
      buttonDown = true;
      Serial.print("|BUTTON:CALL|");
    }
  }
  else if(digitalRead(buttonPin) == HIGH && buttonDown)
  {
    delay(100);
    if(digitalRead(buttonPin) == HIGH)
    {
      buttonDown = false;
    }
  }
}

void checkForSerialCommands()
{
  while(Serial.available())
  {
    if((char)Serial.read() == '|')
    {
      byte length = Serial.readBytesUntil('|', serialBuffer, 256);
      serialBuffer[length] = '\0';
     
      if(length > 0 && serialBuffer[length-1] != '\n')
      {
        int partsCount = 0;
        char* parts[5];
        char* index = serialBuffer;
       
        parts[partsCount++] = index;
        while(*index && partsCount < 5)
        {
          if(*index == ':')
          {
            *index = '\0';
            parts[partsCount++] = index + 1;
          }
         
          index++;
        }
       
        if(partsCount > 1 && strcmp(parts[0], "RELAY") == 0)
        {
          if(strcmp(parts[1], "OPEN") == 0)
          {
            digitalWrite(relayPin, HIGH);
          }
          else if(strcmp(parts[1], "CLOSE") == 0)
          {
            digitalWrite(relayPin, LOW);
          }
          else if(strcmp(parts[1], "PULSE") == 0 && partsCount > 2)
          {
            int pulse = atoi(parts[2]);
            digitalWrite(relayPin, HIGH);
            delay(pulse);
            digitalWrite(relayPin, LOW);
          }
        }
      }
    }
  }
}

TwicReader.h

Code:

#ifndef TWICREADER
#define TWICREADER

#include "Arduino.h"

class TwicReaderClass
{
  public:
    void begin();
    void read();
};

extern TwicReaderClass TwicReader;

#endif

TwicReader.cpp

Code:

#include "TwicReader.h"

volatile int interrupt_count;  // track interrupts from HID reader
volatile int data[100];        // store data sent from HID reader
char twicNumBuffer[15];

void data0ISR()
{
  if(interrupt_count < 100)
  {
    data[interrupt_count] = 0;
    interrupt_count++;
  }
}

void data1ISR()
{
  if(interrupt_count < 100)
  {
    data[interrupt_count] = 1;
    interrupt_count++;
  }
}

void TwicReaderClass::begin()
{
  interrupt_count = 0;
 
  // enable the interrupts for reading HID data
  attachInterrupt(0, data0ISR, FALLING);
  attachInterrupt(1, data1ISR, FALLING);
 
  interrupts();
}

void wiegandToTwic()
{
  // 14 agency code bits
  int agencyCode = 0;
  for(int i=1; i<15; i++)
      agencyCode |= data[i] << (14 - i);
     
  // 14 system code bits
  int systemCode = 0;
  for(int i=15; i<29; i++)
      systemCode |= data[i] << (28 - i);
 
  // 20 credential number bits
  int32_t credentialNumber = 0;
  for(int i=29; i<49; i++)
      credentialNumber |= (int32_t)data[i] << (48 - i);
 
  // write the formatted TWIC number to a string
  sprintf(twicNumBuffer, "%.4d%.4d%.6ld", agencyCode, systemCode, credentialNumber);
}

void wiegandToProx()
{
  // 16 card number bits
  int32_t cardNumber = 0;
  for(int i=9; i<25; i++)
      cardNumber |= (int32_t)data[i] << (24 - i);
 
  // write the formatted TWIC number to a string
  sprintf(twicNumBuffer, "%ld", cardNumber);
}

void TwicReaderClass::read()
{
  if(interrupt_count > 0)
  {
    delay(300); // Make sure the reader is done talking
    noInterrupts();  // turn off interrupts so nothing changes
   
    if(interrupt_count == 75) // Expect 75 bits from the reader for PIV Cards
    {
      // Do the conversion
      wiegandToTwic();
      Serial.print("|TWICNUM:");
      Serial.print(twicNumBuffer);
      Serial.print("|");
    }
   
    if(interrupt_count == 26) // Expect 26 bits from the reader for HID Prox
    {
      // Do the conversion
      wiegandToProx();
      Serial.print("|TWICNUM:");
      Serial.print(twicNumBuffer);
      Serial.print("|");
    }
   
    interrupt_count = 0;
    interrupts();
  }
}

TwicReaderClass TwicReader;

[Reply #1 on:]

Your serial read function has some built-in assumptions I would not make.

Serial.readBytesUntil('|', serialBuffer, 256); // this is going to block in some cases.

 

[Reply #2 on:]

You have a potential problem here:

Code:

   noInterrupts();  // turn off interrupts so nothing changes
   
    if(interrupt_count == 75) // Expect 75 bits from the reader for PIV Cards
    {
      // Do the conversion
      wiegandToTwic();
      Serial.print("|TWICNUM:");
      Serial.print(twicNumBuffer);
      Serial.print("|");
    }
   
    if(interrupt_count == 26) // Expect 26 bits from the reader for HID Prox
    {
      // Do the conversion
      wiegandToProx();
      Serial.print("|TWICNUM:");
      Serial.print(twicNumBuffer);
      Serial.print("|");
    }
   
    interrupt_count = 0;
    interrupts();
  }

Serial printing relies on interrupts, and with interrupts off the buffer may fill up and then it will just hang.

Instead of turning interrupts off like that, I would turn them off briefly, make a copy of anything you are worried might change, and then turn them back on again.

[Reply #3 on:]

Or even change those

Code:

if(interrupt_count ==

to

Code:

if(interrupt_count >=

and leave interrupts running.

[Reply #4 on:]

I really appreciate the code review. Great suggestions. I will implement them and see if they help clear up the problem.