Unexpected program behaviour

Projects Programming
1

Hello,

I've spent few hours trying to identify the issue, but no luck.. Code stop/halt on line 163, sometimes it cycling loop() again from the start.

I'm getting rid of using Strings in my program (as it is freezing after a few days and need a restart). So, probably I messed up with char arrays just can't figure out where and why :frowning: :slight_smile:

Any help/advice is highly appreciated! Thanks!

Arduino IDE v.1.8.13
Board: Arduino Nano (ATMega328 (Old Bootloader))

const int inpBuffMaxLen = 200;
char inputBuffer[inpBuffMaxLen];

int minTemp = 0;
int maxTemp = 0;
int obrMinTemp = 0;
int obrMaxTemp = 0;

char smsPhoneStr[12] = "";
char smsPhoneStr2[12] = "";

bool bEnableSms = false;
bool bObratkaEnableSms = false;
int smsIntervalMins;
long lastSmsSentMSec = -9999000;

class CommonA
{
  public:
    static int indexOf(const char* source, const char* find)
    {
      int srcLen = strlen(source);
      int fndLen = strlen(find);

      if (srcLen == 0 || fndLen == 0 || fndLen > srcLen)
      {
        return -1;
      }

      const char* ptr = strstr(source, find);
      if (ptr) {
        int index = ptr - source;

        if (index >= 0)
        {
          return index;
        }
      }

      return -1;
    }

    static void strRemove(char* str, int pos)
    {
      int len = strlen(str);
      int ind = 0;
      for (int i = pos; i < len; i++)
      {
        str[ind] = str[i];
        ind++;
      }
      str[ind] = '\0';
    }
};

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

void loop(void)
{
  httpGetSMSConfig();
  delay(300000);
}

bool httpGetSMSConfig()
{
  bool bRes = false;
  bool bConnect = true;

  if (bConnect)
  {
    inputBuffer[0] = '\0';
    strcat(inputBuffer, "HTTPCODE:200|L:54|D:\r\nsmsdet=40,55,0,30,60,1,81234567861 81234567861,9,999,");

    char* resStr = &inputBuffer[0];
    Serial.println(resStr);

    int index = -1;
    if ((index = CommonA::indexOf(resStr, "smsdet=")) > -1)
    {
      bRes = true;
      CommonA::strRemove(resStr, index + 8);

      //mintemp
      index = CommonA::indexOf(resStr, ",");
      char minTempStr[5] = "";
      memcpy(minTempStr, resStr, index);
      minTempStr[index] = '\0';
      Serial.print(F("Mintemp: "));
      Serial.println(minTempStr);
      CommonA::strRemove(resStr, index + 1);
      minTemp = atoi(minTempStr);

      //maxtemp
      index = CommonA::indexOf(resStr, ",");
      char maxTempStr[5] = "";
      memcpy(maxTempStr, resStr, index);
      maxTempStr[index] = '\0';
      Serial.print(F("Maxtemp: "));
      Serial.println(maxTempStr);
      CommonA::strRemove(resStr, index + 1);
      maxTemp = atoi(maxTempStr);

      //bEnableSMS
      index = CommonA::indexOf(resStr, ",");
      char enableSmsStr = resStr[index - 1];
      Serial.print(F("enableSMS: "));
      Serial.println(enableSmsStr);
      CommonA::strRemove(resStr, index + 1);
      if (enableSmsStr == '1')
      {
        bEnableSms = true;
      }
      else
      {
        bEnableSms = false;
      }

      //obrMinTemp
      index = CommonA::indexOf(resStr, ",");
      char obrMinTempStr[5] = "";
      memcpy(obrMinTempStr, resStr, index);
      obrMinTempStr[index] = '\0';
      Serial.print(F("ObrMin: "));
      Serial.println(obrMinTempStr);
      CommonA::strRemove(resStr, index + 1);
      obrMinTemp = atoi(obrMinTempStr);

      //obrMaxTemp
      index = CommonA::indexOf(resStr, ",");
      char obrMaxTempStr[5] = "";
      memcpy(obrMaxTempStr, resStr, index);
      obrMaxTempStr[index] = '\0';
      Serial.print(F("ObrMax: "));
      Serial.println(obrMaxTempStr);
      CommonA::strRemove(resStr, index + 1);
      obrMaxTemp = atoi(obrMaxTempStr);

      //bObratkaEnableSMS
      index = CommonA::indexOf(resStr, ",");
      char obratkaEnableSmsStr = resStr[index - 1];
      Serial.print(F("ObrEnabled: "));
      Serial.println(obratkaEnableSmsStr);
      CommonA::strRemove(resStr, index + 1);
      if (obratkaEnableSmsStr == '1')
      {
        bObratkaEnableSms = true;
      }
      else
      {
        bObratkaEnableSms = false;
      }

      //phone
      index = CommonA::indexOf(resStr, ",");
      char phones[25];
      memcpy(phones, resStr, index);
      phones[index] = '\0';

      Serial.print(F("phoneWhole "));
      Serial.println(phones);      
      int ind2 = -1;
      if ((ind2 = CommonA::indexOf(phones, " ")) > -1)
      {
        //second num
        memcpy(smsPhoneStr, phones, index);
        smsPhoneStr[index] = '\0';

        CommonA::strRemove(phones, index + 1);
        memcpy(smsPhoneStr2, phones, strlen(phones));
        smsPhoneStr2[strlen(phones)] = '\0';
      }
      else
      {
        memcpy(smsPhoneStr, phones, strlen(phones));
        smsPhoneStr[strlen(phones)] = '\0';

        smsPhoneStr2[0] = '\0';
      }

      Serial.print(F("Phone: "));
      Serial.println(smsPhoneStr);
      CommonA::strRemove(resStr, index + 1);

      //smsIntervalMins
      index = CommonA::indexOf(resStr, ",");
      char smsIntervalMinsStr[10];
      memcpy(smsIntervalMinsStr, resStr, index);
      smsIntervalMinsStr[index] = '\0';
      Serial.print(F("SMS Interval (mins): "));
      Serial.println(smsIntervalMinsStr);
      CommonA::strRemove(resStr, index + 1);
      smsIntervalMins = atoi(smsIntervalMinsStr);

      //lastSmsSentMins
      index = CommonA::indexOf(resStr, ",");
      char smsLastSentStr[5];
      memcpy(smsLastSentStr, resStr, index);
      Serial.print(F("SMS Last sent (mins): "));
      Serial.println(smsLastSentStr);
      CommonA::strRemove(resStr, index + 1);
      int val = atoi(smsLastSentStr);
      if (val <= 0)
      {
        lastSmsSentMSec = -1;
      }
      else
      {
        lastSmsSentMSec = val;
      }
    }

  }
}
2

The obvious suspect is that the program is writing outside of array bounds. For example, in this code you never check for the case that index>4, which could be fatal.

      char minTempStr[5] = "";
      memcpy(minTempStr, resStr, index);
      minTempStr[index] = '\0';

It helps to use the "n" versions of the string handling routines, where possible. For example, strncpy(), strncat(), etc.

3

why do you create a class for your helper functions... just use functions

int strIndexOf(const char* source, const char* find)
{
  int srcLen = strlen(source);
  int fndLen = strlen(find);

  if (srcLen == 0 || fndLen == 0 || fndLen > srcLen)
  {
    return -1;
  }

  const char* ptr = strstr(source, find);
  if (ptr) {
    int index = ptr - source;

    if (index >= 0)
    {
      return index;
    }
  }
  return -1;
}

void strRemove(char* str, int pos)
{
  int len = strlen(str);
  int ind = 0;
  for (int i = pos; i < len; i++)
  {
    str[ind] = str[i];
    ind++;
  }
  str[ind] = '\0';
}

Your issue is likely there: you allocate 12 bytes for

char smsPhoneStr[12] = "";
char smsPhoneStr2[12] = "";

then in the code you allocate 25 bytes for

     char phones[25];

which will be initialized with "81234567861 81234567861"

and then you do

     if ((ind2 = strIndexOf(phones, " ")) > -1)
      {
        //second num
        memcpy(smsPhoneStr, phones, index); 
        smsPhoneStr[index] = '\0';

        strRemove(phones, index + 1);
        memcpy(smsPhoneStr2, phones, strlen(phones));
        smsPhoneStr2[strlen(phones)] = '\0';
      }
      else
      {
        memcpy(smsPhoneStr, phones, strlen(phones));
        smsPhoneStr[strlen(phones)] = '\0';

        smsPhoneStr2[0] = '\0';
      }

==> if the if condition is true, you SHOULD use ind2, not index (that's your bug)
==> if the condition is false, you assume that there is only one phone number and you copy possibly up to 25 bytes into 12 which could lead to disaster. you should check for bounds.

here is a quick rewrite in a test script:

const int inpBuffMaxLen = 200;
char inputBuffer[inpBuffMaxLen];

char smsPhoneStr[12] = "";
char smsPhoneStr2[12] = "";

int strIndexOf(const char* source, const char* find)
{
  int srcLen = strlen(source);
  int fndLen = strlen(find);

  if (srcLen == 0 || fndLen == 0 || fndLen > srcLen)
  {
    return -1;
  }

  const char* ptr = strstr(source, find);
  if (ptr) {
    int index = ptr - source;

    if (index >= 0)
    {
      return index;
    }
  }
  return -1;
}

void strRemove(char* str, int pos)
{
  int len = strlen(str);
  int ind = 0;
  for (int i = pos; i < len; i++)
  {
    str[ind] = str[i];
    ind++;
  }
  str[ind] = '\0';
}


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

  inputBuffer[0] = '\0';
  strcat(inputBuffer, "HTTPCODE:200|L:54|D:\r\nsmsdet=40,55,0,30,60,1,81234567861 81234567000,9,999,");

  char* resStr = inputBuffer;
  Serial.println(resStr);
  int index = strIndexOf(resStr, "smsdet=");
  if (index > -1)  {
    strRemove(resStr, index + 7); // SHOULD THAT BE 7 ?
    Serial.println(resStr);

    index = strIndexOf(resStr, ",");
    char minTempStr[5] = "";
    memcpy(minTempStr, resStr, index);
    minTempStr[index] = '\0';
    Serial.print(F("Mintemp: "));
    Serial.println(minTempStr);
    strRemove(resStr, index + 1);
    int minTemp = atoi(minTempStr);

    //maxtemp
    index = strIndexOf(resStr, ",");
    char maxTempStr[5] = "";
    memcpy(maxTempStr, resStr, index);
    maxTempStr[index] = '\0';
    Serial.print(F("Maxtemp: "));
    Serial.println(maxTempStr);
    strRemove(resStr, index + 1);
    int maxTemp = atoi(maxTempStr);

    //bEnableSMS
    index = strIndexOf(resStr, ",");
    char enableSmsStr = resStr[index - 1];
    Serial.print(F("enableSMS: "));
    Serial.println(enableSmsStr);
    strRemove(resStr, index + 1);
    bool EnableSms = (enableSmsStr == '1');

    //obrMinTemp
    index = strIndexOf(resStr, ",");
    char obrMinTempStr[5] = "";
    memcpy(obrMinTempStr, resStr, index);
    obrMinTempStr[index] = '\0';
    Serial.print(F("ObrMin: "));
    Serial.println(obrMinTempStr);
    strRemove(resStr, index + 1);
    int obrMinTemp = atoi(obrMinTempStr);

    //obrMaxTemp
    index = strIndexOf(resStr, ",");
    char obrMaxTempStr[5] = "";
    memcpy(obrMaxTempStr, resStr, index);
    obrMaxTempStr[index] = '\0';
    Serial.print(F("ObrMax: "));
    Serial.println(obrMaxTempStr);
    strRemove(resStr, index + 1);
    int obrMaxTemp = atoi(obrMaxTempStr);

    //bObratkaEnableSMS
    index = strIndexOf(resStr, ",");
    char obratkaEnableSmsStr = resStr[index - 1];
    Serial.print(F("ObrEnabled: "));
    Serial.println(obratkaEnableSmsStr);
    strRemove(resStr, index + 1);
    bool bObratkaEnableSms = (obratkaEnableSmsStr == '1');


    //phone
    index = strIndexOf(resStr, ",");
    char phones[25];
    memcpy(phones, resStr, index);
    phones[index] = '\0';

    Serial.print(F("phoneWhole "));
    Serial.println(phones);
    int ind2 = strIndexOf(phones, " ");
    if (ind2 > -1) {
      //second num
      memcpy(smsPhoneStr, phones, ind2);
      smsPhoneStr[ind2] = '\0';
      Serial.print(F("Phone: "));
      Serial.println(smsPhoneStr);

      strRemove(phones, ind2 + 1);
      memcpy(smsPhoneStr2, phones, strlen(phones));
      smsPhoneStr2[strlen(phones)] = '\0';
      Serial.print(F("Phone2: "));
      Serial.println(smsPhoneStr2);
    }
    else
    {
      memcpy(smsPhoneStr, phones, strlen(phones));
      smsPhoneStr[strlen(phones)] = '\0';

      smsPhoneStr2[0] = '\0';
    }
    strRemove(resStr, index + 1);

    //smsIntervalMins
    index = strIndexOf(resStr, ",");
    char smsIntervalMinsStr[10];
    memcpy(smsIntervalMinsStr, resStr, index);
    smsIntervalMinsStr[index] = '\0';
    Serial.print(F("SMS Interval (mins): "));
    Serial.println(smsIntervalMinsStr);
    strRemove(resStr, index + 1);
    int smsIntervalMins = atoi(smsIntervalMinsStr);

    //lastSmsSentMins
    index = strIndexOf(resStr, ",");
    char smsLastSentStr[5];
    memcpy(smsLastSentStr, resStr, index);
    Serial.print(F("SMS Last sent (mins): "));
    Serial.println(smsLastSentStr);
    strRemove(resStr, index + 1);
    int val = atoi(smsLastSentStr);
    int lastSmsSentMSec = (val <= 0) ? -1 : val;
  }
}

void loop() {}

Serial Monitor (@ 115200 bauds) will show

[color=purple]
[color=red]HTTPCODE:200|L:54|D:
smsdet=40,55,0,30,60,1,81234567861 81234567000,9,999,[/color]
40,55,0,30,60,1,81234567861 81234567000,9,999,
Mintemp: 40
Maxtemp: 55
enableSMS: 0
ObrMin: 30
ObrMax: 60
ObrEnabled: 1
phoneWhole 81234567861 81234567000
Phone: 81234567861
Phone2: 81234567000
SMS Interval (mins): 9
SMS Last sent (mins): 999
[/color]

seems to be ok now but you should be super careful when mem-copying stuff into buffers to ensure that the size of what you copy fits the buffer size

PS/ parsing your cString could be done way more simply with strtok() and without rearranging the string all the time, just scan things as you go through the buffer. consider this code:

const int inpBuffMaxLen = 200;
char inputBuffer[inpBuffMaxLen];
const char* keyword = "smsdet=";

void setup() {
  Serial.begin(115200);
  inputBuffer[0] = '\0';
  strcat(inputBuffer, "HTTPCODE:200|L:54|D:\r\nsmsdet=40,55,0,30,60,1,81234567861 81234567000,9,999,");

  char* resStr = strstr(inputBuffer, keyword);
  resStr = strtok(resStr + strlen(keyword), ", ");
  while (resStr) {
    Serial.println(resStr);
    resStr = strtok(NULL, ", ");
  }
}

void loop() {}

Serial Monitor (@ 115200 bauds) will show

[color=purple]
40
55
0
30
60
1
81234567861
81234567000
9
999
[/color]

so you get all your data, it's just a matter of storing them as you go

4

J-M-L, thanks a lot for addressing my issue!
Bug is trivial when it's disclosed, but so stupid for me being unable to find out for so long :slight_smile:

5

we have all been there, don't worry!