Sniffer Code Crashing on Serial

I’ve got some code which I’m using to detect the DCC packets sent on a model railroad track bus, and activate layout functions in response to the commands being sent to the locomotives. I’m doing it this way so that it acts without having to be called up by a specific address. An optical coupler chip is used to connect the Arduino input pin to the track.

I originally downloaded this code from a site that used it as a packet sniffer, which simply sensed the commands being given to the loco and output it to the serial monitor (DCC Sniffer – Packet Analyser with Arduino | rudysmodelrailway). However, I’ve removed all the commands that use the serial function, so that I can call the appropriate animations (sound effects, servos, etc.) at the appropriate points.

My problem is that whenever I’m running this code, and I try to use a serial output, be it hardware or software, the Arduino crashes. I need to use the serial to control the MP3 player module that runs the sound effects. Given that I’ve removed all references to serial output (aside from comments), I’m at a loss as to what’s causing this. I’ve included the code below. Can someone please give me a second opinion on this?

///////////////////////////////////////////////////////
//
// DCC packet analyze: Ruud Boer, October 2015
// DCC packet capture: Robin McKay, March 2014
//
// The DCC signal is detected on Arduino digital pin 2
//
//
////////////////////////////////////////////////////////

//Below variables are used for the DCC sniffer code

/* General notes and observations:
 *  - Based on testing, the code seems to behave as follows:
 *    - Will update based on the refreshTime
 *    - Will update immediately if a function packet is detected
 * 
 */

byte refreshTime = 1; // Time between DCC packets buffer refreshes in s
byte packetBufferSize = 32; // DCC packets buffer size

#define TIMER_PRESCALER 64
#define DccBitTimerCount (F_CPU * 80L / TIMER_PRESCALER / 1000000L)
// 16000000 * 80 / 64 / 1000000 = 20; 20 x 4usecs = 80us

boolean packetEnd;  //Tracks if we've reached the end of the packet
boolean preambleFound; //Tracks if a DCC packet preamble has been found

const byte bitBufSize = 50; // number of slots for bits
volatile byte bitBuffer[bitBufSize]; //Defines buffer to hold bits
volatile byte bitBuffHead = 1;
volatile byte bitBuffTail = 0;

byte pktByteCount=0; //Counts number of bytes in a packet
byte packetBytesCount;
byte preambleOneCount;
byte dccPacket[6]; // buffer to hold packet data
byte instrByte1;
byte decoderType; //0=Loc, 1=Acc
byte bufferCounter=0;
byte isDifferentPacket=0;
byte showLoc=1;
byte showAcc=1;


unsigned int decoderAddress;
unsigned int packetBuffer[64];
unsigned int packetNew=0;

unsigned long timeToRefresh = millis() + refreshTime*1000;
//Above could be altered - change the multiplier to 500
//if you want half-second resolution for refresh.  May
//not be needed.  Seemingly would only affect the
//detection of which loco is running, which would
//be for longer than 1 second anyway.

//========================

void getPacket() {
  //Helper function
  //Gets next byte until the end of DCC packet
  preambleFound = false;
  packetEnd = false;
  packetBytesCount = 0;
  preambleOneCount = 0;
  while (! packetEnd) {
    if (preambleFound) getNextByte();
    else checkForPreamble();
  }
}

//========================

void checkForPreamble() {
  //Helper function
  //Checks that a preamble is found and sets
  //preambleFound to true if so
   byte nextBit = getBit();
   if (nextBit == 1) preambleOneCount++;
   if (preambleOneCount < 10 && nextBit == 0) preambleOneCount = 0;
   if (preambleOneCount >= 10 && nextBit == 0) preambleFound = true;
}

//========================

void getNextByte() {
  //Helper function
  //Gets next byte of packet
  byte newByte = 0;
  for (byte n = 0; n < 8; n++) newByte = (newByte << 1) + getBit();
  packetBytesCount ++;  
  dccPacket[packetBytesCount] = newByte;
  dccPacket[0] = packetBytesCount;
  if (getBit() == 1) packetEnd = true;
}

//========================

byte getBit() {
  // gets the next bit from the bitBuffer
  // if the buffer is empty it will wait indefinitely for bits to arrive
  //Helper function
  byte nbs = bitBuffHead;
  while (nbs == bitBuffHead) byte nbs = nextBitSlot(bitBuffTail); //Buffer empty
  bitBuffTail = nbs;
  return (bitBuffer[bitBuffTail]);
}

//========================

void beginBitDetection() {
  //Starts detection of bits.  Needs to be called in Setup.
  TCCR0A &= B11111100;
  attachInterrupt(0, startTimer, RISING);
}

//========================

void startTimer() {
  //Helper function
  //Used to start interrupt timers
  OCR0B = TCNT0 + DccBitTimerCount;
  TIMSK0 |= B00000100;
  TIFR0  |= B00000100;
}

//========================

ISR(TIMER0_COMPB_vect) {
  //Calls interrupt
  byte bitFound = ! ((PIND & B00000100) >> 2); 
  TIMSK0 &= B11111011;
  byte nbs = nextBitSlot(bitBuffHead);
  if (nbs == bitBuffTail) return;
  else {
    bitBuffHead = nbs;
    bitBuffer[bitBuffHead] = bitFound;
  }
}

//========================

byte nextBitSlot(byte slot) {
  //Helper function
  //Provides next bit slot
  slot ++;
  if (slot >= bitBufSize) slot = 0;
  return(slot);
}

//========================

void refreshBuffer() {
  //refreshes the Packet Buffer
  timeToRefresh = millis() + refreshTime*1000;  //Resets refresh timer
  //Will need to change the 1000 to 500 for half-second resolution, if using.
  for (byte n=0; n<packetBufferSize; n++) packetBuffer[n]=0;
  bufferCounter=0;
}

//========================

void setup() {
  //Below sets up the DCC packet sniffer.
  beginBitDetection(); //Do not call delay() after this line!!  Including body of loop() or any funcs!!!! 
}

//====================

void loop() {
  getPacket(); //Uncomment this line when on DCC !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
  //above line loads next DCC packet
  byte speed;
  byte checksum = 0;
  
  if (millis() > timeToRefresh) refreshBuffer(); //Refreshes buffer when timer runs out

  pktByteCount = dccPacket[0];
  if (!pktByteCount) return; // No new packet available
  //If there is a packet, then dccPacket[0] will have more than 0 in it.
  //dccPacket[0] will contain the length of the packet.
  /* Based on reading the code below, for locomotive packets
   *  dccPacket[1] is the loco short address
   *  dccPacket[1 & 2] are the loco long address
   *  dccPacket[2] is the first data byte for short address
   *  dccPacket[3-] are data bytes
   * 
   */

  for (byte n = 1; n <= pktByteCount; n++) checksum ^= dccPacket[n];
  //Above checks that bytes have valid checksum against
  if (checksum) return; // Invalid Checksum
  
  else { // There is a new packet with a correct checksum
    isDifferentPacket=1;
    for (byte n=0; n<packetBufferSize ; n++) {// Check if packet is not already in the buffer. 
    // The checksum byte is used for the test, not ideal, some new packets may not show (only 256 different checksums)
      if (dccPacket[pktByteCount]==packetBuffer[n]) isDifferentPacket=0; 
    }
    
    if (isDifferentPacket) {  // packet does not yet exist in the packet buffer
      packetBuffer[bufferCounter] = dccPacket[pktByteCount]; // add new packet to the buffer
      bufferCounter = (++bufferCounter)&(packetBufferSize-1);
      
      if (dccPacket[1]==B11111111) { //Idle packet
        return;
      }
    
      if (!bitRead(dccPacket[1],7)) { //bit7=0 -> Loc Decoder Short Address
        decoderAddress = dccPacket[1]; //Sets decoderAddress to short address
        instrByte1 = dccPacket[2];
        decoderType = 0;
      }
      else {
        if (bitRead(dccPacket[1],6)) { //bit7=1 AND bit6=1 -> Loc Decoder Long Address
          decoderAddress = 256 * (dccPacket[1] & B00011111) + dccPacket[2]; //Corrected as per comment on website
          //Sets decoderAddress to long address
          instrByte1 = dccPacket[3];
          decoderType = 0;
        }
        else { //bit7=1 AND bit6=0 -> Accessory Decoder
          //Not interested in Accessory decoders for this application - can just return;
          //Do this in the if(decoderType) statement below
          decoderAddress = dccPacket[1]&B00111111;
          instrByte1 = dccPacket[2];
          decoderType = 1;
        }
      }
      //By this point, we have the decoder address
      //Do address reading/checking here
      if (decoderType) { // Accessory Basic
        if (showAcc) {
          if (instrByte1&B10000000) { // Basic Accessory
            decoderAddress = (((~instrByte1)&B01110000)<<2) + decoderAddress;
            byte port = (instrByte1&B00000110)>>1;
          }
          else { // Accessory Extended NMRA spec is not clear about address and instruction format !!!
            decoderAddress = (decoderAddress<<5) + ((instrByte1&B01110000)>>2) + ((instrByte1&B00000110)>>1);
          }
        }
      }
      else { // Loc / Multi Function Decoder
        //showLoc is one of the variables set via the Serial prompt.
        //Is 1 (true) by default.  Can just remove serial prompt
        //code and leave it true.
        if (showLoc) {
          byte instructionType = instrByte1>>5; //Shifts bits of instruction byte to the right by 5 bits
          //Have figured out and commented code up to here - need to figure out the rest below.
          //Mainly need to figure out how to determine if function 9 is triggered or not.
          switch (instructionType) {

            case 0:
              //Do nothing
            break;

            //For cases 1, 2 and 3 below, the 'Serial.print' lines ending in B01111111 are where
            //we take the speed measurement from.  Serial.print lines have been removed.

            case 1: // Advanced Operations
              if (instrByte1==B00111111) { //128 speed steps
                byte speed = dccPacket[pktByteCount-1]&B01111111;
                //Check/read speed here
              }
              else if (instrByte1==B00111110) { //Speed Restriction
              //Do nothing
              }
            break;

            case 2: // Reverse speed step
              speed = ((instrByte1&B00001111)<<1) - 3 + bitRead(instrByte1,4);
              //Check/read speed here
            break;

            case 3: // Forward speed step
              speed = ((instrByte1&B00001111)<<1) - 3 + bitRead(instrByte1,4);
              //Check/read speed here
            break;

            case 4: // Loc Function L-4-3-2-1
              //Do nothing
              //Check functions 1-4 with bitwise ops here
              //1 is 1, 2 is 10, 3 is 100 and 4 is 1000
            break;

            case 5: // Loc Function 8-7-6-5
              if (bitRead(instrByte1,4)) {
                //Do nothing
                //Check functions 5-8 with bitwise ops here
                //5 is 1, 6 is 10, 7 is 100 and 8 is 1000
              }
              else { // Loc Function 12-11-10-9
                //Check functions 9-12 with bitwise ops here
                //9 is 1, 10 is 10, 11 is 100 and 12 is 1000
              }
            break;

            case 6: // Future Expansions
              switch (instrByte1&B00011111) {
                case 0: // Binary State Control Instruction long form
                  //Do nothing
                break;
                case B00011101: // Binary State Control
                  //Do nothing
                break;
                case B00011110: // F13-F20 Function Control
                  //Do nothing
                break;
                case B00011111: // F21-F28 Function Control
                  //Do nothing
                break;
              }
            break;
            
            //No code for case 7 - will never see programming on the main.
            case 7:
            break;
          }
        }
      }
    }
  }
}

//=====================

Crashes are usually caused by memory problems, for example writing to memory outside the bounds of an array, using Strings on an AVR-based Arduino, etc.

Does sending a command to serial cause a write to memory? That seems to be what triggers the crash.

I don’t know what you mean by “sending a command to serial”. Post the code fragment you suspect, using code tags.

In the first post you stated you have removed all commands that use the serial function. What is the program supposed to do, and with what input?

while not impossible, it’s much much easier to debug code with the serial interface.

it would be easier to add the serial interface and disable parts of the “other” code in order to isolate what is causing the problem

i’m familar with DCC and don’t believe using the Serial interface should interfere with the use of an interrupt to capture the DCC bit-stream. (not sure how this approach works without measuring the time between edges, 50-100 usec)

No, using Strings on AVR boards is very very safe. The malloc in the AVR boards always keeps 128bytes free so the program can keep running if the String uses up all the other available memory. Makes it easy to debug because print()s still work.