System freeze help

Hi, ive been through a few iterations of this code making improvements, I recently removed strings and swapped for char[] although serial disconnect time has improved every few days it will completely stop responding to serial requests.

Can any one see any code issues that could be leading to this?

#include <OneWire.h>
#include <DallasTemperature.h>

#define RELAY_OFF 1
#define RELAY_ON 0

const int maxChars = 80;
char receivedChars[maxChars];
int ndx = 0;

void relayControl(int pin, unsigned char state) {
  pinMode(pin, OUTPUT);
  digitalWrite(pin, state);
  Serial.print("RELAY,PIN,");
  Serial.print(pin);
  Serial.print(",");
  Serial.println(state);
}

void processTemperature(int pin) {
  float temperature = 0.0;
  int deviceCount = 0;

  OneWire OneWireBus(pin);
  DallasTemperature Sensor(&OneWireBus);
  Sensor.begin();
  Sensor.requestTemperatures();

  deviceCount = Sensor.getDeviceCount();

  if (deviceCount > 0) {
    for (int i = 0; i < deviceCount; i++) {
      temperature = Sensor.getTempCByIndex(i);
      Serial.print("TEMP,");
      Serial.print(pin);
      Serial.print(",");
      Serial.println(temperature);
    }
  } else {
    Serial.print("TEMP,");
    Serial.print(pin);
    Serial.println(",NO_DATA");
  }
}

const int cylPin = 52;

void cylStatState() {
  pinMode(cylPin, INPUT_PULLUP);
  int pinState = digitalRead(cylPin);
  Serial.print("CYLSTAT,");
  Serial.println(pinState);
}

void processInput() {
  char cmd[6];
  int pin = 0;
  char opt[3];

  strncpy(cmd, receivedChars, 5);
  cmd[5] = '\0';
  pin = atoi(receivedChars + 5);
  strncpy(opt, receivedChars + 7, 2);
  opt[2] = '\0';

  if (strncmp(cmd, "relay", 5) == 0) {
    if (strncmp(opt, "on", 2) == 0) {
      relayControl(pin, RELAY_ON);
    } else {
      relayControl(pin, RELAY_OFF);
    }
  } else if (strncmp(cmd, "temps", 5) == 0) {
    processTemperature(pin);
  } else if (strncmp(cmd, "cstat", 5) == 0) {
    cylStatState();
  }
}

void processSerialData() {
  char startMarker = '<';
  char endMarker = '>';

  while (Serial.available() > 0 && ndx < maxChars) {
    char c = Serial.read();

    if (c == startMarker) {
      ndx = 0;
    } else if (c == endMarker) {
      receivedChars[ndx] = '\0';
      processInput();
      ndx = 0;  // Reset index after processing
    } else {
      receivedChars[ndx++] = c;
    }
  }
}

void setup() {
  Serial.begin(115200);
}

void loop() {
  processSerialData();
}

here you did not check if ( ndx >= 80 ){}
the limit for your char array

Try this:
receivedChars[ndx++%maxChars] = c;

wow thanks both for your speedy replies, I will add that edit and see how it goes.

I was/am worried it could be the way I'm dynamically setting and using pins and I suppose I'm at the stage where I could start hard coding these things but the repetitive code is ugly :slight_smile:

I will report back.

Thanks

Edit:

I'm not sure I understand what that's doing though, from stack overflow

"modulus Operator gives you remainder"

how does it work if you don't mind helping me understand.

Edit:

if char index is 81 % 80 = 1

char index = 1

so we wrap around and reset the buffer.

As others have pointed out, this should be maxChars-1 as it could potentially be 79 at start of loop and would be incremented to 80 which overflows the array..
Might want to add a check to see if ndx is getting to high,do a serial print for debugging, obviously something in the data is not correct..

good luck.. ~q

1 Like

Thanks, my project works great for a few days so its got to be something in this area.

Edit:

why didn't I test this before...

My code works fine with text lower than maxChars but if i send a massive long string my buffer hits + 80 then everything stops.

make it this..

while (Serial.available() > 0 && ndx < maxChars-1)

see how long it runs now.. :slight_smile:

~q

1 Like

If I sent serial in longer than buffer it still kills my output until I reset.

Wish id done these checks sooner :slight_smile: easier to fix locally that a remote device.

I notice you don't have a check for this potential..
Be nice if you have another serial port to debug print too..

try something like this..

void processSerialData() {
  char startMarker = '<';
  char endMarker = '>';

  while (Serial.available() > 0 && ndx < maxChars-1) {
    char c = Serial.read();

    if (c == startMarker) {
      ndx = 0;
    } else if (c == endMarker) {
      receivedChars[ndx] = '\0';
      processInput();
      ndx = 0;  // Reset index after processing
    } else {
      receivedChars[ndx++] = c;
    }
  }
  //check here for too much wrong data, reset things..
  if (ndx == maxChars-1){
    ndx = 0;
    memset(receivedChars,0,sizeof(receivedChars));//emtpy
  }
}

it adds a check to see if ndx is sitting at 79 with no end marker in sight you will not enter while loop anymore..
so we reset back to ndx 0 and empty the buffer..
still not sure why the incoming is getting mucked up..

~q

1 Like

I was just looking at how i could get that check, thank you I think that's the one.

1 Like

Thank you so much, I am testing locally with a spare mega, that's why I cant believe I hadn't done it until now :slight_smile:

I see expected behaviour now on the serial monitor.

I can upload to the remote mega via ssh now, strange something in my node logic must be randomly sending a large packet and killing it randomly.

You could print the mess to see what it is..
How is the device getting data from a program or you using a secure shell telnet connection to it??
the mega's got a few serial ports..

~q

The Arduino is plugged into raspberry pi 4 at a family members house, which is their smart heating system so when the temperature data stops the system stops :confused:

I ssh in to work remotely and the serial is over USB, all seems to be working now so time will tell but the local testing I did showed me the error straight away, cant believe I hadn't checked sooner.

I may consider changing my code to a single request temp command

"<22,23,24,25>"

and have the Arduino probe all pins asked for and send a single array back,

"[22.7,45.1,....]"

I don't think the amount of serial requests I'm doing is good design.

nah, probably not, just make sure you stay in sync somehow..
ok, makes more sense, on the pi is it your program talking to the mega, might want to add some transmission control..
have the mega send a nak back when it starts to overflow, when you recv a nak reset the sending side of things..

AckMe

did this demo a while back, simple ack or nak controls flow..

fun stuff.. ~q

Thank you.

I've enjoyed getting back into the Arduino its been fun and completely unforgiving :), I've been hobby coding in python for a while and its a different world, low level and low power so much more to consider with the Arduino.

I'm using node-red on the pi to send and receive serial, I will look into the link I want to make this system bulletproof.

1 Like

Thinking about this some more..
Serial is anything but bullet proof..
Could just be some corruption..
Might be a good idea to add a simple checksum of sorts to the data to make sure it's valid..
Look at the UDPSend and UDPRecv in my git it adds a simple checksum to each packet and then checks it to make sure data is received valid..
To bullet proof, each packet should have a checksum that is checked and a ACK is sent if checksum fails send a NAK back, have your sender resend..
Can log the NAKs back on pi to see if line is dirty but expect some bad packets as they will happen..
~q

Thank you, I will look into this later on today. The commands I send are fairly trivial and I expect the event to fail if the command was damaged in any way but it could occur that the command was damaged in a way it could trigger the wrong event, definitely need it to work as professionally as possible so I don’t get headache from the user :slight_smile:

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.