My program keeps crashing and looping. Does anyone know why!!?

Using Arduino Programming Questions
1

Hi there,

I'm writing a class template for a small coding project of mine. Once working, the class should allow me to parse data along different serial channels using the same functions on the Arduino Nano/Uno.

Expectation:
Program should call the setup function and then proceed to the loop and behave as normal.

The Bug: Keeps crashing
Last output to Serial Monitor depends on where I put flushes and delays. The program must crash somewhere after the last print line and then reboots (well sometimes it doesn't reboot and just crashes) this goes into a cycle. However, I know that the last output to the serial monitor is not a good indication of the exact line that the program fails.

Maybe someone here could look at my code and give me a hand? Any help would be much appreciated!

I have used the memory free library already and it tells me that there is still about 1.4K bytes left in the SRAM at the point of crash. So this bug is not due to a catastrophic collision between the stack and heap.
I don't think the crash is due to trying to access an index that is out of bounds. Neither is it caused by any memory leaks due to passing pointers that no longer point to allocated memory.

The Arduino IDE compiles the whole program without errors and no warnings from my code.

The crash maybe happening upon the call to AltSoftSerial::available() when the Comms.read() function is evaluated. This happens on both the nano and uno. Can someone tell me what how to solve it?

This is my sketch.

#include <Arduino.h>
#include <Comms1_2.h>        //written by me
#include <AltSoftSerial.h>
//#include <MemoryFree.h>

//Declare and instantiate

SerialCommsClass<AltSoftSerial> Comms ("Bytes", 500);             //Bluetooth
SerialCommsClass<HardwareSerial> Comms1 ("Bytes", 500);       //Hardware Serial

void setup() {
  // put your setup code here, to run once:

//Call begin() for both channels
  Comms1.begin(9600); 
  Comms.begin(9600);
  Serial.flush();

  //SETUP PINS AND STUFF;
}

void loop() {
  //Serial.print("treeMemory1()=");
  //Serial.println(freeMemory());
  if (Comms1.read()) {/*do stuff*/}
  if (Comms.read()) {/*do even more stuff*/}
}

And here is the Comms1_2.h class with its corresponding cpp file.

//Allows reading of both hardware Serial and altsoftserial.
//incoming info can be interpretted as either strings or bytes
//Will also write bytes or strings if necessary. 

//must be instantiated with SerialCommsClass<ReplaceWithMyChannel> Comms()

#ifndef Comms1_2_h
#define Comms1_2_h

#ifndef OPERAND_SIZE
#define OPERAND_SIZE 3
#endif

#include "AltSoftSerial.h"
#if ARDUINO >= 100
#include "Arduino.h"
#else
#include "WProgram.h"
#endif
#include "EEPROM.h"

template <typename T>//Allows you to give it instantiations of class for different forms of communication

class SerialCommsClass {					//SerialCommsClass<AltSoftSerial> Comms()
public:
	SerialCommsClass (const String& arg, unsigned int timeOut);  //Constructor "Bytes" or "Strings" and timeOut
	bool read();	//function will call the relevant methods read the appropriate buffer as either Bytes or strings
	bool write(char opcode, const unsigned long operand[OPERAND_SIZE]);		//passes array by reference
                                                                                  //format data to be sent and then write to the output buffer
                                                                                //if bytes used, input parameters must be between 0-255
	bool begin(unsigned long baud);	//just like all the other libraries
	bool flush();
private:
	bool _bytesOrStrings;						//0 for bytes, 1 for strings
	bool extractBytes();
	const unsigned int _Time_Out;		//Time the program waits before it decides that
                          				//it hasn't received the expected termination
	const char _Terminating_Char;		//character marking end of command is currently a space 
	T* _Channel;			//stores pointer to object

	char _opcode;										
	unsigned long _operand[OPERAND_SIZE];	//template
	String _command;  //received chars stored here
	unsigned int _timeStamp; 
};

#endif
2

Part 2:
Here is the Cpp file:

//takes only  operands, terminating char is ' '

#include "MemoryFree.h"
#include "Comms1_2.h"
#include "AltSoftSerial.h"
template <typename T>
SerialCommsClass<T>::SerialCommsClass (const String& arg, unsigned int timeOut)   //constructor

: _Time_Out (timeOut),    //upto 32 seconds
 _Terminating_Char (' ')
 {
    T channel;
    _Channel = &channel;
    if (arg == "Bytes") {
      _bytesOrStrings = 0;    //try to make sure that realloc() is not called. Try to avoid fragmenting the heap
      _command.reserve(OPERAND_SIZE + 2);
    } else if (arg == "Strings") {
      _bytesOrStrings = 1;      //try to make sure that realloc() is not called. Try to avoid fragmenting the heap
      _command.reserve(OPERAND_SIZE * 5 + 2);
    } else {
      Serial.println(F("Invalid arg"));
      Serial.flush();
    }
  }

template <>
SerialCommsClass<HardwareSerial>::SerialCommsClass (const String& arg, unsigned int timeOut)   //explicit specialization of constructor
: _Time_Out (timeOut),
  _Terminating_Char (' ')
  {
    _Channel = &Serial;
    if (arg == "Bytes") {
      _bytesOrStrings = 0;    //try to make sure that realloc() is not called. Try to avoid fragmenting the heap
      _command.reserve(OPERAND_SIZE + 2);
    } else if (arg == "Strings") {
      _bytesOrStrings = 1;      //try to make sure that realloc() is not called. Try to avoid fragmenting the heap
      _command.reserve(OPERAND_SIZE * 5 + 2);
    } else {
      Serial.println(F("Invalid arg"));
      Serial.flush();
    }
  }

template <typename T>
bool SerialCommsClass<T>::read () {  //Takes either Serial or an instantiation of AltSerial 
                                                              //arg is either "Bytes" or "Strings"
  Serial.println("read");
  Serial.println(_command.length());
  if (_command.length() == 0) {     //time out tested
    //Serial.flush();
    _timeStamp = (unsigned int)millis();    //equivalent to mod 65536
    Serial.println("1");
    Serial.flush();
    //resets every call
  } /*else if ((unsigned int)((unsigned int)millis() - _timeStamp) > _Time_Out) {
    //if it doesn't receive the terminating char after expected time
    Serial.println(F("Received incomplete command")); //No termination
    Serial.flush();
    Serial.println(_command);
    _command = ""; //gets rid of data
    //perhaps should ask to resend command
  } */
delay(5000);    //all the serial prints around here are for debugging purposes
Serial.println("available");    //note this line will not print if you replace with Serial.println(_Channel->template available())
Serial.flush();
  while (_Channel->template available()) {
    char c = _Channel->template read();
    _command += c;      //add to command
  }
Serial.println("3");
return _bytesOrStrings ? extractStrings() : extractBytes(); //return appropriate interpretation
}

template <typename T>
bool SerialCommsClass<T>::extractBytes () { 
      Serial.println ("got here");
  Serial.flush();
  Serial.print("treeMemory1()=");
  Serial.println(freeMemory());
  Serial.flush();
  //takes the command and turns into an opcode and operand
  if (_command.length() == 0) {
    //if the string is empty or is not terminated, 
    //a complete command hasn't been received yet
    return false;
  } else if (_command[_command.length() - 1] != _Terminating_Char) {
    //not terminated
    return false;
  } else if (_command.length() != OPERAND_SIZE + 2) {    //2 bytes for opcode and delimiter
    //if something weird is received
    Serial.println(F("Invalid data length"));
    Serial.flush();
    _command = "";
    return false;
  }
  //must be a valid command
  Serial.println(F("Something Received"));
  _opcode = _command[0]; //expects the first character to be an opcode
  for (unsigned int i = 0; i < OPERAND_SIZE; i++) {
    _operand[i] = _command[i + 1];
  }
  _command = ""; //Finished using command, so delete it. All info is in operand and opcode

  return true;
}

template <typename T>
bool SerialCommsClass<T>::write (char opcode, const unsigned long operand[OPERAND_SIZE]) {
  //Specifying array size makes no difference
  static String buffer;       //reserves space
  buffer = "";      //empties buffer
  if(_bytesOrStrings) {
//allows for delimiters and average 4 chars per number
    buffer.reserve(OPERAND_SIZE * 5 + 3);
  } else {
//accounts for 1 byte per input, delimiters and char
    buffer.reserve(OPERAND_SIZE + 3);
  }
  buffer += '?';
  buffer += opcode;
  if (_bytesOrStrings) {        //for strings
    for (unsigned int i = 0; i < OPERAND_SIZE; i++) {
      buffer += String(operand[i]);        //constructor used to convert between ulong and string
      buffer += ",";
    }
    buffer[OPERAND_SIZE + 2] = '!';        //gets rid of extra comma at the end
  } else { //must be bytes
    if (operand[0] > 255 || operand[1] > 255 || operand[2] > 255) {
      return false;
    }
    for (unsigned int i = 0; i < OPERAND_SIZE; i++) {
      buffer += (byte)operand[i];
    }
    buffer += '!';
  }
  flush();
  for (unsigned int i = 0; i < OPERAND_SIZE + 3; i++) {
    _Channel->template write(buffer[i]);       //read byte by byte necessary fo compatibility between the 2 different 
                                                //implementations of altsoftserial and HardwareSerial
  }
  //flush();
  return true;
}

template <typename T>
bool SerialCommsClass<T>::begin (unsigned long baud) {
  _Channel->template begin(baud);
  Serial.println(F("Channel opened"));
  Serial.flush();
  return true;
}

template <typename T>
bool SerialCommsClass<T>::flush() {
  _Channel->template flush();
  return true;
}

//instantiated here
template class SerialCommsClass<AltSoftSerial>;
template class SerialCommsClass<HardwareSerial>;

Here is a sample from the Serial Monitor

Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
available
Channel opened
Channel opened
read
0
1
3

Why use a class template? You are really handling two types of classes.

You can just overload the constructor...

example:

#include <SoftwareSerial.h>

//#include <Stream.h>

#define MESSAGE_LENGTH 64

class SerialMessenger{
  public:
    SerialMessenger( HardwareSerial& device) {hwStream = &device;}
    SerialMessenger( SoftwareSerial& device) {swStream = &device;}
    void begin(uint32_t baudRate);
    char* checkForNewMessage(const char endMarker, bool errors);
    
  private:
    HardwareSerial* hwStream;
    SoftwareSerial* swStream;
    Stream* stream;
    char incomingMessage[MESSAGE_LENGTH];
    size_t idx = 0;
};

char* SerialMessenger::checkForNewMessage(const char endMarker, bool errors = false)
{
  stream = !hwStream? (Stream*)swStream : hwStream;
  if (stream->available())
  {
    if (stream->peek() == '\r')
    {
      (void)stream->read();
      return nullptr;
    }
    incomingMessage[idx] = stream->read();
    if (incomingMessage[idx] == endMarker)
    {
      incomingMessage[idx] = '\0';
      idx = 0;
      return incomingMessage;
    }
    else
    {
      idx++;
      if (idx > MESSAGE_LENGTH - 1)
      {
        if (errors)
        {
          stream->print(F("{\"error\":\"message too long\"}\n"));
        }
        while (stream->read() != '\n')
        {
          delay(50);
        }
        idx = 0;
        incomingMessage[idx] = '\0';
      }
    }
  }
  return nullptr;
}

void SerialMessenger::begin(uint32_t baudRate)
{
  if (hwStream)
  {
    hwStream->begin(baudRate);
  }
  else
  {
    swStream->begin(baudRate);
  }
}

SoftwareSerial softSerial(2, 3);

SerialMessenger vera(Serial);
SerialMessenger electron(softSerial);

void setup()
{
  vera.begin(115200);
  electron.begin(57600);
}


void loop()
{
  if(const char* veraMessage = vera.checkForNewMessage('\n'))
  {
    char newMessage[MESSAGE_LENGTH];
    strcpy(newMessage, veraMessage);
    Serial.println(newMessage);
  }
  if(const char* electronMessage = electron.checkForNewMessage('\n'))
  {
    char newMessage[MESSAGE_LENGTH];
    strcpy(newMessage, electronMessage);
    Serial.println(newMessage);
  }
}
4 
 if (hwStream)
  {
    hwStream->begin(baudRate);
  }
  else
  {
    swStream->begin(baudRate);
  }

Can you explain how you can use a pointer to an object as the condition in an if statement?

5

kajob:

 if (hwStream)

{
    hwStream->begin(baudRate);
  }
  else
  {
    swStream->begin(baudRate);
  }




Can you explain how you can use a pointer to an object as the condition in an if statement?

The class will initialize both pointers. Either with the initialization that is provided in the constructor, or as nullptr.

Null Pointers will return false.

6

Oh ok thanks. Anyway, I found the original problem.

SerialCommsClass<T>::SerialCommsClass (const String& arg, unsigned int timeOut)   //constructor

: _Time_Out (timeOut),    //upto 32 seconds
 _Terminating_Char (' ')
 {
    T channel;
    _Channel = &channel;
    if (arg == "Bytes") {
      _bytesOrStrings = 0;    //try to make sure that realloc() is not called. Try to avoid fragmenting the heap
      _command.reserve(OPERAND_SIZE + 2);
    } else if (arg == "Strings") {
      _bytesOrStrings = 1;      //try to make sure that realloc() is not called. Try to avoid fragmenting the heap
      _command.reserve(OPERAND_SIZE * 5 + 2);
    } else {
      Serial.println(F("Invalid arg"));
      Serial.flush();
    }
  }

After looking more closely, there is a memory leak in this constructor.