Method active when class not created

Those are the class definitions. There is third class but that seems very well behaved and only works the relays.

What is "this". What runs fine for two iterations? The StartComms method? I think that should only be called once in a sketch.

It would be much easier to see what you're doing if you'd put all your code into a single .ino file so we could put it into the IDE and compile / test it: class declarations, class definitions, object instantiation, setup() function, loop() function, everything!

As requested, post the minimal code that compiles and demonstrates the problem. No extra clutter.

The programme i.e. "Loop" goes around two iterations.

I will try and make a combined version.

I know this is not a safe way to handle timing. But the bug doesn't occur until the program is running for more than a month. Are you planning for this to be a long term program?

And then what happens? What are the symptoms? What should be happening and how does that compare with what actually happens.

The programme will operate for no more than a few days at a time.

There are two odd things happening. 1) If I create a myInterface object with new as above the programme does two loops and then just stops partway through another output line in the serial interface.
2) if I comment out the lines and don't create myInterface or call the StartComms method AND add the line Serial.begin(9600) then the programm will run quite happly AND I still get Serial output from the line myInterface->Snd(msgTxt) even though I have not created the object. Which is a bit mad.

Show me. This makes no sense. If you don't have anything called myInterface then you'd get a compile error on that line. So the object must exist somewhere. If it exists then you can call it's methods whether it was instantiated or not. If that method works then it works. I don't understand the confusion.

Or are you trying to say that myInterface is a null pointer in that case. If that's the case then it's undefined behavior. There's no telling what sort of nonsense might happen.

1 Like

Which Arduino board?

I start off with:

myOutputs* myInterface = NULL;

as a global. If I use "new" to create the object and assign it to "myInterface" with:

myInterface = new myOutputs(OUTPUT_SERIAL, OUTPUT_TCP, OUTPUT_UDP);

then the programme will do two loops and freeze. If don't use the above line and do add Serial.begin etc. then the not only does the programme work but I also get serial output from the myInferface method "Snd".

The board in an Uno v3

If you don't include the line with new and try to call a method on myInterface while it is still a null pointer then you have invoked undefined behavior. It could literally do anything. There is no predicting it and there is no explaining it.

Don't call methods on null pointers.

As for why your other code only works twice, I think you should focus on that and not why some undefined case behaves in a particular way.

Sorry, I forgot to mention that it stops getting valid responses from some of the 3 temperature sensors if the comms class is created. I don't see any connection between the two.

// Include the libraries:
#include <Arduino.h>
#include <avr/wdt.h>
#include <Adafruit_Sensor.h>
#include <DHT.h>
#include <SPI.h>
#include <Ethernet.h>
#include <EthernetUdp.h>

//Sets the physical limits for the Ardunio Uno and this programme
#define MAX_RELAYS 8
#define MAX_SENSORS 10
#define COMMAND_TEXT 20
#define errT -99  //Sensor error respoce


/************************  Code Types **************************/
struct myResultType {
  float value = 0.0;
  float mean = errT;
  float count = 0;
  float total = 0.0;
};

//Holds the information that describes an input thing
struct mySensorType {
  int channel;  //Any assocated Arduino channel.
  int dhtNum;   //The sensor number 1 to 3
  //DhtLib * dht = NULL;       //Sensor object pointer
  bool failed;               //Flag to show the sensor has failed
  myResultType Temperature;  //Any temperature data
  myResultType Humidity;     //Any humidity data
  bool state;                //Any digitial state
  char type;                 //Its type. 'S'=Sensor, 'D'=Digital
};

//Holds the information that describes a thing
struct myRelayType {
  int state;    //The current digital result
  int channel;  //The Arduino Channel Number to use
  float min;    //Its minimum valid value in engineering units
  float max;    //Its maximum valid value in engineering units
};

//Holds the information that describes a group of things
struct myGroupType {
  char type;   //Type of sensor group (Virtual or Actial)
  int inuse;   //The number of sensors in use
  int max;     //The maximum number of sensors
  int useing;  //The number of active elements i.e. temperature sensors included in the average
};

//Manages the command data
struct myCommandType {
  char command[COMMAND_TEXT];  //The command text
  int number;                  //The command number
  int param1;                  //The first paramiter
  float param2;                //The second paramiter
};

/************ RELAY CLASS ********************************************/
//Holds all of the information for a relay
class myRelay {
public:
  myRelay();
  void Set(int digital, float min, float max);
  void Operate(int Mode, unsigned long myNow);                       //changes the relay state
  void FindState(float tempAv, int Manual);                          //Decides what state the relay should be in
  unsigned long FindTime(unsigned long myNow, unsigned long Count);  //Decides when the relay needs to change
  int Lock;                                                          //0 = Not locked. 1 = ON, 2 = OFF
  int Required;                                                      //The stage the relay should be in at the contained in "Change"
  unsigned long Change;                                              //When the change should be made
  myRelayType Info;

private:
  unsigned long _lastchange;  //The time the last change was made
};

/************ ALL RELAYS CLASS ***************************************/
//The collection of relays
class myAllRelays {
public:
  myAllRelays(int numof);
  void SetRelays(unsigned long myNow);     //Tells the relays to change state
  void FindState(float tempAv);            //Sets the future state of the relays when they change
  void FindTime(unsigned long myNow);      //Sets the times the next changes need to occure
  void LockRelay(int Relay, int Locking);  //Sets a command lock
  myRelay* Relay(int i);                   //Returns a pointer to a specific relay
  void Print(bool full, char* msgTxt);     //Test for all of the relays
  int ManualLock;                          //Set by command.  Default 0 (none), 1 = Locked ON, 2 = Locked OFF
  myGroupType Info;                        //The group level information

private:
  myRelay* _relay[MAX_RELAYS];  //The relays
};

/************ DHT CLASS ***************************************/
#define DHTTYPE DHT22  // DHT 22  (AM2302)

class DhtLib {
private:
  DHT dht;

public:
  DhtLib(int dhtPin)
    : dht(dhtPin, DHTTYPE) {}
  void begin();
  float getTemperature();
  float getHumidity();
};

/*************** OUTPUTS CLASS ********************************************/
//Manages Printing to serial and sending to IP
class myOutputs {
public:
  myOutputs(bool SerialSt, bool TCPSt, bool UPDSt);
  void Reset(int Change, bool Mode);
  bool StartComms(char* myName, char* myVersion);
  void Snd(char* msgTxt);
  void Command(myCommandType* Command);

private:
  void Prt(char* SomeText);
  void SendIP(char* msgTxt);   //Sends an IP message
  void SendUDP(char* msgTxt);  //Sends an UDP message

  //Comms check flags
  bool _sOK;
  bool _ipOK;
  bool _helloSent = false;  //Hello message
  bool _SerialSt;           //True = stream the messagge to Serial
  bool _TCPSt;              //True = stream the messagge to TCP
  bool _UDPSt;              //True = stream the messagge to UDP
};

//This EPCS
#define NAME "Box N"

//System delay times
#define StartDelay 1000   //The startup delay in ms.
#define SensorDelay 2000  //ms the delay between sampeling the DHT sensors. Must be at least 2000 ms due to sensor design
#define CycleDelay 500    //The operating timestep in ms. Updates the sensors, inputs & outputs
#define Hysteresis 10000  //ms The min delay before reversing a relay and the temperature averaging time for sensor inputs
#define StepDelay 2000    //ms the delay between starting any two relays

//Indicator LED
#define LED_FLASH 19  //Set to -1 if not used.  This means digital 19 can be used for other things

//Sets the outputs to produce
#define OUTPUT_SERIAL true
#define OUTPUT_TCP true
#define OUTPUT_UDP true
#define VERBOSE true

/**********************************************************************************************************************
* Arduino Uno has 20 digital (channels 0 to 19) if you make use of the analogues as digitals
* However: 0 and 1 are used for serial communications.  Attachining anything to them prevents the use of the bootloader
* Also: the Ethernet Shield makes use of 4 and 10 for its communications.
* There for 0, 1, 4 and 10 cannot be used.
* NOTE: The digitial channels are NOT all the same.  The temperature sensors must not be attached to A0-A5 (14-19)
***********************************************************************************************************************/

//Configure the Relays - NOTE: Relay names must be <10 char long
#define N_RELAYS 8
//              Channel, Min,   Max
#define RELAY_1 2, 0.0, 22.0
#define RELAY_2 3, 0.0, 23.0
#define RELAY_3 18, 0.0, 24.0
#define RELAY_4 5, 0.0, 25.0
#define RELAY_5 14, 0.0, 26.0
#define RELAY_6 15, 0.0, 27.0
#define RELAY_7 16, 0.0, 28.0
#define RELAY_8 17, 0.0, 29.0

/*
//Set up Sensors
#define NUMBER_INPUTS 8         //The total of all type of input.  Sensors + Digitials + virtual 
#define SAMPLES_TO_AVERAGE 10   //The number of readings included in the average

//The input types. These are either 'S' = SENSOR or 'D' = DIGITIAL INPUT or 'V' = VIRTUAL SENSOR
#define INPUT_TYPES 'S', 'S', 'S', 'D', 'V', 'V', 'V', 'V'

//The input channels to use.  In the same order as above.  Virtual sensors can have any number
#define INPUT_CHANNELS 6, 7, 8, 9, 1, 2, 3, 4
*/

//Set up Sensors
#define NUMBER_INPUTS 7        //The total of all type of input.  Sensors + Digitials + virtual
#define SAMPLES_TO_AVERAGE 10  //The number of readings included in the average

//The input types. These are either 'S' = SENSOR or 'D' = DIGITIAL INPUT or 'V' = VIRTUAL SENSOR
#define INPUT_TYPES 'S', 'S', 'D', 'V', 'V', 'V', 'V'

//The input channels to use.  In the same order as above.  Virtual sensors can have any number
#define INPUT_CHANNELS 6, 7, 9, 1, 2, 3, 4

//Network Settings
#define MAC_ADDRESS \
  { 0xA8, 0x61, 0x0A, 0xAE, 0x95, 0x48 }
#define IP_ME 192, 168, 6, 177
#define IP_DEST 192, 168, 6, 155
#define IP_DNS 192, 168, 6, 1
#define IP_GATEWAY 192, 168, 6, 1
#define IP_SUBNET 255, 255, 0, 0

#define UDP_PORT 8888
#define TCP_PORT 80

//These are the project veriables
#define VERSION "0.r2.1"

//Create the global pointers and other global veriables
myAllRelays* RList = NULL;
mySensorType SList[MAX_SENSORS];
int inputChannels[MAX_SENSORS] = { INPUT_CHANNELS };
char inputTypes[MAX_SENSORS] = { INPUT_TYPES };
myOutputs* myInterface = NULL;
DhtLib* dht[3];

unsigned long runNow;     //The next time to get the average temperature will be processed etc and generate output
unsigned long sampleNow;  //The next time to get DHT sensor values
float useTemp = errT;     //The current temperature to control the relays
bool Flash = false;       //LED to indicate board is alive
unsigned long FlashNow;   //The next time to flash
bool CommsOK;             //Flag to track the status of the comms links
bool Verbose = VERBOSE;   //Set the lots of output flag

/************************  Code  ****************************/

//User Set Network & IP Settings
byte mac[] = MAC_ADDRESS;  //The mac adderess of the Ethernet shield
IPAddress ip1(IP_ME);      //The static IP address of this EPCS unit
IPAddress ip2(IP_DEST);    //The Network IP Address of the device to recive the UDP
IPAddress myDns(IP_DNS);
IPAddress gateway(IP_GATEWAY);
IPAddress subnet(IP_SUBNET);
unsigned int localPort = UDP_PORT;  //The UDP port to use
EthernetClient client;              //The Ethernet client object
EthernetServer server(TCP_PORT);    //The Ethernet port to use
EthernetUDP Udp;                    //The Udp instance

/*************** OUTPUTS CLASS ********************************************/
//Manages Printing to serial and sending to IP
myOutputs::myOutputs(bool SerialSt, bool TCPSt, bool UDPSt)
  : _SerialSt(SerialSt), _TCPSt(TCPSt), _UDPSt(UDPSt) {
  _sOK = false;
  _ipOK = false;
}

void myOutputs::Reset(int Change, bool Mode) {
  switch (Change) {
    case 1:
      _SerialSt = Mode;
      break;
    case 2:
      _TCPSt = Mode;
      break;
    case 3:
      _UDPSt = Mode;
      break;
  }
}

//Start the verious comms options
bool myOutputs::StartComms(char* myName, char* myVersion) {
  //Start the serial comms
  unsigned long Timeout = millis();
  char msgTxt[80] = { '\0' };

  //Enables the serial port for information output, if its connected
  if (!_sOK) {
    Serial.begin(9600);
    while (!Serial) {  //Wait for the serial to start.  if it does not then disable the serial
      if (millis() - Timeout > 1000) break;
    }
    if (Serial) _sOK = true;
  }

  //Enables the ethernet if its connected and cabled
  if (!_ipOK) {
    //Start the common Ethernet elements
    Ethernet.init(10);                                 // Most Arduino shields
    Ethernet.begin(mac, ip1, myDns, gateway, subnet);  // start the Ethernet connection and the server:
    _ipOK = true;
    // Check for Ethernet hardware present
    if (Ethernet.hardwareStatus() == EthernetNoHardware) {
      sprintf(msgTxt, "%s: %s\r", myName, "Error, Ethernet shield not found");
      _ipOK = false;
    }
    if (Ethernet.linkStatus() == LinkOFF) {
      sprintf(msgTxt, "%s: %s\r", myName, "Error, Ethernet cable is not connected");
      _ipOK = false;
    }
    if (_ipOK) {
      server.begin();        //Start TCP server
      Udp.begin(localPort);  //Start UDP
    }
  }
  if (strlen(msgTxt) > 1) Snd(msgTxt);

  //Send the hello
  if (!_helloSent) {
    sprintf(msgTxt, "%s, %s, %s", myName, myVersion, "Starting");
    Snd(msgTxt);
    _helloSent = true;
  }

  return _ipOK;
}

void myOutputs::Prt(char* SomeText) {
  if (_sOK && _SerialSt) Serial.println(SomeText);
}

void myOutputs::Snd(char* msgTxt) {
  Prt(msgTxt);
  SendUDP(msgTxt);
  SendIP(msgTxt);
}

//Sends and recieves TCP messages
void myOutputs::SendIP(char* msgTxt) {
  //Looks for a client and then sends what it has stored in rolling buffer
  if (_ipOK && _TCPSt) {
    EthernetClient client = server.available();  // listen for incoming clients
    if (client) {
      if (client.connected()) client.print(msgTxt);  // output the current dataline
    }
  }
}

//Write a line of UDP
void myOutputs::SendUDP(char* msgTxt) {
  if (_ipOK && _UDPSt) {
    Udp.beginPacket(ip2, localPort);
    Udp.write(msgTxt);
    Udp.endPacket();
  }
}

//Recives commands of TCP and enacts them
void myOutputs::Command(myCommandType* Command) {

  char myMessage[COMMAND_TEXT];                                                                          //The command recieved
  char *a, *b, *c;                                                                                       //The parts of the command
  char* myCommands[9] = { "LONG", "SHORT", "REBOOT", "SERIAL", "TCP", "UDP", "POWER", "RELAY", "SET" };  //These zare the valid commands
  int myNumCommands = 9, ib, ic;
  //float fc = errT;
  bool Gotit = false;
  char myNum[7];
  char* end;
  int i = 0, j = 0;

  Command->number = -1;
  //Get the command message.  Up the max number allowed
  if (_ipOK) {
    EthernetClient client = server.available();  //Listen for incoming clients
    while (client.available() > 0) {
      // read the bytes incoming from the client:
      if (i < COMMAND_TEXT) myMessage[i] = client.read();
      else client.read();
      i++;
    }
    myMessage[i] = '\0';
    if (strlen(myMessage)) {
      Snd(myMessage);  //Echo the message recieved

      //Look for a command
      i = 0;
      while (!Gotit && i < myNumCommands) {
        a = strstr(myMessage, myCommands[i]);
        if (a != NULL) {
          Gotit = true;
          strcpy(Command->command, a);
          Command->number = i + 1;
        }
        i++;
      }

      //Get the paramiters
      ib = -1;
      ic = -1;
      if (Gotit && i > 3) {
        b = strchr(a, ',');
        if (b != NULL) {
          b++;
          Command->param1 = (int)*b - '0';
        }
        c = strchr(b, ',');
        if (c != NULL) {
          c++;
          for (j = 0; j < 6; j++) {
            myNum[j] = *c;
            c++;
          }
          myNum[j - 1] = '\0';
          Command->param2 = (float)strtod(myNum, &end);
        }
      }
    }
  }
}

/**************** END OF COMMS ***************/

/**************** EPCS OF COMMS ***************/
void DhtLib::begin() {
  dht.begin();
}

float DhtLib::getTemperature() {
  float value;
  value = dht.readTemperature();
  //Serial.println(value);
  if (isnan(value)) return errT;
  else return value;
}

float DhtLib::getHumidity() {
  float value;
  value = dht.readHumidity();
  if (isnan(value)) return errT;
  else return value;
}
/**************** END OF EPCS ***************/

/**************** RELAYS ***************/
/************ RELAY CLASS *********************************************/
//The Relay Class, holds all of the information for a single relay
myRelay::myRelay()
  : Required(HIGH), Change(0), _lastchange(0), Lock(0) {}

//setup the user paramiters
void myRelay::Set(int digital, float min, float max) {
  Info.channel = digital;
  pinMode(Info.channel, OUTPUT);
  Info.min = min;
  Info.max = max;
  Info.state = LOW;
}

// Make sure a relay is set to the requested state. Return True if a change was made
void myRelay::Operate(int Mode, unsigned long myNow) {
  if (Info.state != Mode) {  //Change only if we need to
    digitalWrite(Info.channel, Mode);
    Info.state = Mode;
    Change = 0;
    _lastchange = myNow;
  }
}

// Find out what state the relay should be in NOW according to the current temperature
void myRelay::FindState(float tempAv, int Manual) {

  if (Manual + Lock > 0) {
    if ((Manual == 1) || (Lock == 1)) Required = HIGH;
    else Required = LOW;
  } else {
    if ((tempAv > Info.min) && (tempAv < Info.max)) Required = HIGH;
    else Required = LOW;
  }
}

//Decide when a relay needs to change
unsigned long myRelay::FindTime(unsigned long myNow, unsigned long Last) {
  if (Required != Info.state) {                  // A change has been requested
    if (Change == 0) {                           // and we dont already have a time set for that change
      if ((myNow - _lastchange) > Hysteresis) {  // and the last time we changed this relay was more than the set time ago then ..
        if (Last == 0) Change = myNow;
        else Change = Last + StepDelay;  // set a time in the future and make sure we dont start two relays at the same time
      }
    }
  }
  return Change;
}

/************  ALL RELAYS CLASS **************************************/
//The All Relays Class, hold sthe information for a collecton of relays
myAllRelays::myAllRelays(int numof) {
  if (numof > MAX_RELAYS) Info.inuse = MAX_RELAYS;
  else Info.inuse = numof;
  for (int i = 0; i < Info.inuse; i++) _relay[i] = new myRelay();
  ManualLock = 0;
}

//Returns a specific relay
myRelay* myAllRelays::Relay(int i) {
  if (i < Info.inuse) return _relay[i];
  else return NULL;
}

// Go through all of the relays if we have hit their change time operate them
void myAllRelays::SetRelays(unsigned long myNow) {
  myRelay* current;
  for (int i = 0; i < Info.inuse; i++) {
    current = _relay[i];
    if (current != NULL) {
      if (current->Change != 0) {
        if (myNow >= current->Change) current->Operate(current->Required, myNow);
      }
    }
  }
}

// Find out what state the relays should be in NOW according to the current temperature
void myAllRelays::FindState(float tempAv) {
  myRelay* current;

  for (int i = 0; i < Info.inuse; i++) {
    current = _relay[i];
    if (current != NULL) current->FindState(tempAv, ManualLock);
  }
}

//Set a relay as manually locked
void myAllRelays::LockRelay(int Relay, int Locking) {

  myRelay* current;
  if (Relay > 0 && Relay < N_RELAYS + 1) {
    if (Locking > -1 && Locking < 4) {

      Serial.print("Locking: ");
      Serial.print(Relay);
      Serial.print(", ");
      Serial.println(Locking);
      current = _relay[Relay - 1];
      if (current != NULL) current->Lock = Locking;
    }
  }
}

// Decide when all of the relays need to change
void myAllRelays::FindTime(unsigned long myNow) {
  unsigned long Last = 0;
  myRelay* current;
  for (int i = 0; i < Info.inuse; i++) {
    current = _relay[i];
    if (current != NULL) Last = current->FindTime(myNow, Last);
  }
}

//Provide the text output for all of the relays
void myAllRelays::Print(bool full, char* msgTxt) {
  char subTxt[15];

  if (full) {
    sprintf(subTxt, ", R%d", Info.inuse);
    strcat(msgTxt, subTxt);
  }

  for (int i = 0; i < Info.inuse; i++) {
    if (full) sprintf(subTxt, ", R%d %d", i + 1, _relay[i]->Info.state);
    else sprintf(subTxt, ", %d", _relay[i]->Info.state);
    strcat(msgTxt, subTxt);
  }
}
/***************** END of Relays *************/

/***************** Main section *************/

//Configure the Relays as defined in Config.h
void initR() {
  RList = new myAllRelays(N_RELAYS);
  if (RList->Info.inuse > MAX_RELAYS) RList->Info.inuse = MAX_RELAYS;

  if (N_RELAYS > 0) RList->Relay(0)->Set(RELAY_1);
  if (N_RELAYS > 1) RList->Relay(1)->Set(RELAY_2);
  if (N_RELAYS > 2) RList->Relay(2)->Set(RELAY_3);
  if (N_RELAYS > 3) RList->Relay(3)->Set(RELAY_4);
  if (N_RELAYS > 4) RList->Relay(4)->Set(RELAY_5);
  if (N_RELAYS > 5) RList->Relay(5)->Set(RELAY_6);
  if (N_RELAYS > 6) RList->Relay(6)->Set(RELAY_7);
  if (N_RELAYS > 7) RList->Relay(7)->Set(RELAY_8);
}

//Configure the inputs as defined in Config.h
void initS() {
  int j = 0;

  for (int i = 0; i < NUMBER_INPUTS; i++) {
    SList[i].channel = inputChannels[i];
    SList[i].type = inputTypes[i];
    SList[i].state = false;
    SList[i].failed = false;
    if (SList[i].type == 'S') {
      SList[i].dhtNum = j;
      //dht[j] = new DhtLib(SList[i].channel);
      //dht[j]->begin();
      j = j + 1;
      if (j == 3) j = 0;  //Stop the array from going out of bounds
    }
    if (SList[i].type == 'D') {
      pinMode(SList[i].channel, INPUT);
    }
  }
}

void setup() {
  unsigned long delayT = millis();

  //Set the indicator LED
  if (LED_FLASH > 0) {
    pinMode(LED_FLASH, OUTPUT);
    FlashNow = millis();
  }

  while ((millis() - delayT) < StartDelay) {}  //Do Nothing
  runNow = millis();                           //Sets the starting cycle time in ms

  //Create the output class but does not initilise any of the streams.  Arduino hangs if the recevers are not ready!
  myInterface = new myOutputs(OUTPUT_SERIAL, OUTPUT_TCP, OUTPUT_UDP);

  //Setup the I/O ports, configure the veriables etc.
  initR();
  initS();

  //Make the sensor storage
  //Serial.begin(9600);
  for (int i = 0; i < 3; i++) dht[i] = new DhtLib(i + 6);
  for (int i = 0; i < 3; i++) dht[i]->begin();
}

//Find the current average for all of the temperature values
float AverageOfAverages() {

  float av = 0.0;
  float count = 0;

  for (int i = 0; i < NUMBER_INPUTS; i++) {
    if (SList[i].Temperature.mean != errT) {
      count++;
      av += SList[i].Temperature.mean;
    }
  }
  if (count > 0) av = av / count;
  else av = errT;
  return av;
}

//Find the rolling average for the sensor values
void Average(myResultType* info) {

  if (info->value > errT) {
    info->count++;
    info->total += info->value;
    //If we have enough samples find the average.  Average start as errT and then be the last valid reading
    if (info->count == SAMPLES_TO_AVERAGE) {
      info->mean = info->total / info->count;
      info->count = 0;
      info->total = 0.0;
    }
  }
}

void GetReadings() {

  //Sample the seneors and find the folling average for each sensor
  for (int i = 0; i < NUMBER_INPUTS; i++) {

    if (SList[i].type == 'S') {
      SList[i].Temperature.value = dht[SList[i].dhtNum]->getTemperature();
      if (SList[i].Temperature.value == errT) SList[i].failed = true;
      else {
        Average(&SList[i].Temperature);
        SList[i].Humidity.value = dht[SList[i].dhtNum]->getHumidity();
        ;
        Average(&SList[i].Humidity);
      }
    }

    if (SList[i].type == 'D') {
      SList[i].state = digitalRead(SList[i].channel);
    }
  }
}

void SetReading(int Sensor, float Reading) {

  //Sets the reading for a virtual sensor
  if (Sensor > 0 && Sensor < NUMBER_INPUTS + 1) {
    if (SList[Sensor].type == 'V') {
      SList[Sensor].Temperature.mean = Reading;
    }
  }
}

void AddNumber(float num, char* units, char* txt) {
  char numTxt[8];

  strcat(txt, ",");
  dtostrf(num, 2, 1, numTxt);
  strcat(txt, numTxt);
  if (Verbose) strcat(txt, units);
}

void ShowReadings(float temperature) {

  char msgTxt[200] = { '\0' };
  char subTxt[100] = { '\0' };

  strcat(msgTxt, NAME);
  AddNumber(temperature, "C", msgTxt);
  for (int i = 0; i < NUMBER_INPUTS; i++) {
    if (Verbose) {
      sprintf(subTxt, ",%d%c", i + 1, SList[i].type);
      strcat(msgTxt, subTxt);
    }

    if (SList[i].failed) strcat(msgTxt, ",FAIL");
    else {
      if (SList[i].type == 'S') {
        if (Verbose) AddNumber(SList[i].Temperature.value, "C", msgTxt);
        AddNumber(SList[i].Temperature.mean, "aC", msgTxt);
        if (Verbose) AddNumber(SList[i].Humidity.value, "%", msgTxt);
        AddNumber(SList[i].Humidity.mean, "a%", msgTxt);
      }

      if (SList[i].type == 'D') {
        sprintf(subTxt, ",%d", SList[i].state);
        strcat(msgTxt, subTxt);
      }

      if (SList[i].type == 'V') {
        AddNumber(SList[i].Temperature.mean, "aC", msgTxt);
      }
    }
  }
  subTxt[0] = '\0';
  RList->Print(Verbose, subTxt);
  strcat(msgTxt, subTxt);
  myInterface->Snd(msgTxt);
}

//Causes an LED on the board to flash every second.
void FlashLED(unsigned long myNow) {
  if (LED_FLASH > 0) {
    if (myNow >= FlashNow) {
      FlashNow += 500;
      if (Flash) {
        digitalWrite(LED_FLASH, LOW);
        Flash = false;
      } else {
        digitalWrite(LED_FLASH, HIGH);
        Flash = true;
      }
    }
  }
}

//Carry out the commends
void DoCommand(myCommandType* Command) {

  switch (Command->number) {
    case 1:  //Long message command
      Verbose = true;
      break;
    case 2:  //Short message command
      Verbose = false;
      break;
    case 3:                   //Reboot command
      wdt_enable(WDTO_15MS);  //Turn on the WatchDog and don't stroke it.
      for (;;) {}             //do nothing and wait for the eventual...
      break;
    case 4:  //Serial message command
      myInterface->Reset(1, Command->param1);
      break;
    case 5:  //TCP message command
      myInterface->Reset(2, Command->param1);
      break;
    case 6:  //UDP message command
      myInterface->Reset(3, Command->param1);
      break;
    case 7:  //All relays message command
      if (Command->param1 > -1 && Command->param1 < 4) RList->ManualLock = Command->param1;
      break;
    case 8:  //Single Relay message command
      RList->LockRelay(Command->param1, Command->param2);
      break;
    case 9:  //Sets the virtual sensor reading
      SetReading(Command->param1, Command->param2);
      break;
  }
}

void loop() {
  unsigned long myNow = millis();
  char myTxt[80];
  myCommandType ThisCommand;

  //Flashes the indicator to show we are running
  FlashLED(myNow);

  //This gets sensor readings at the rate speicified.  It can be faster or slower (more likely) than the relay update reat
  if (myNow >= sampleNow) {
    sampleNow += SensorDelay;

    //If the comms are not running try and start them
    if (!CommsOK) CommsOK = myInterface->StartComms(NAME, VERSION);

    // Reading temperature or humidity takes about 250 milliseconds!
    // Sensor readings may also be up to 2 seconds 'old' (its a very slow sensor)
    GetReadings();
    useTemp = AverageOfAverages();
  }

  //Controls the relays etc.
  if (myNow >= runNow) {
    runNow += CycleDelay;

    //Operate the relays.  If there is a valid temperature reading
    if (useTemp != errT) {
      RList->SetRelays(myNow);    //See which relays need to change in this time step, and change them
      RList->FindState(useTemp);  //Find the state a relay should be in according to the current temperature
      RList->FindTime(myNow);     //If the relay is in a diffent state to the one it should be in, set a time for it to change
    }
    ShowReadings(useTemp);  //Provides output
    myInterface->Command(&ThisCommand);
    if (ThisCommand.number > 0) DoCommand(&ThisCommand);
  }
}

This is the version that locks up after a couple of loops. I have tried to remove some chunks of code but it seems to start working as soon as I take anything out. For example, it works if I comment out:
if (ThisCommand.number > 0) DoCommand(&ThisCommand);
If i put this back and comment out the unrelated LRelay class is also works.

Unfortunately I need both.

When you compile, what does it say for memory usage?

Why do you create all your objects dynamically instead of creating them globally as static objects?

What happens if 'Command()' doesn't find a valid command? Then 'Command()' doesn't write anything to 'ThisCommand'. As a local variable, 'ThisCommand' contains garbage until valid data is written to it. You use it to call 'DoCommand' regardless.

it says:

Sketch uses 23952 bytes (74%) of program storage space. Maximum is 32256 bytes.
Global variables use 1201 bytes (58%) of dynamic memory, leaving 847 bytes for local variables. Maximum is 2048 bytes.