Millis() going off

Hi,

I've been working on my aquarium controller project for some time now, but i recently ran in to a problem that I can't seem to figure out.

I'm using an Arduino Mega, and an ESP8266 to control everything from my phone.
Everything seems to work alright, but when I change some value (and so far it seems that it doesn't matter which one) some of my Arduino processes started to slow down dramatically.
I found out that after changing some values my millis() where no longer accurate: instead of counting some 1000 millis for a 1000 loops, after the change only 60 something millis where counted for every 1000 loops, so processes that depend on an millis timer, are slowed down to.

I hope I can explain it better with an example.

Here is a stripped version of my code:

#include <Wire.h>

#define DS3231_I2C_ADDRESS 0x68
const byte Relay[4] = {48, 46, 44, 42};

byte second, minute, hour, dayOfWeek, dayOfMonth, month, year;
byte decToBcd(byte val) {
  return ( (val / 10 * 16) + (val % 10) );
}
byte bcdToDec(byte val) {
  return ( (val / 16 * 10) + (val % 16) );
}
long timeInSec;
int timeInMin;
unsigned long waitSomeSec;
char page;
char data;
long value;
char valueString[20];
bool newData = false;

float tempVal[3] = {20, 21, 22};
float maxTempVal[3] = {25, 25, 25};
float minTempVal[3] = {18, 18, 20};
float tempCalVal[3] = {0, 0, 0};
float heatOnOffTemp[2] = {20, 22};
float tempWarning[2] = {19, 23};
float pHval = 6.86;
float min_pHval = 6.1;
float max_pHval = 7.32;
byte activeLight = 9;
long RGB_startTimePhase[6] = {21600, 39600, 45000, 77400, 81000, 0};
long RGB_startTimeMode[6] = {25200, 43200, 48600, 79200, 84600, 1800};
int bottleSize[3] = {500, 500, 500};
int bottleLevel[3] = {467, 288, 325};
byte statusNo[3] = {0, 0, 0};

unsigned long myMillis;
unsigned long loopNr = 0;
int loopCount = 0;

void setup() {
  Wire.begin();
  Serial.begin(9600);
  Serial1.begin(115200);
  for (byte r = 0; r < sizeof(Relay); r++) {
    digitalWrite(Relay[r], HIGH); pinMode(Relay[r], OUTPUT);
  }
  waitSomeSec = millis();
  myMillis = millis();
}

void readDS3231time(byte * second, byte * minute, byte * hour, byte * dayOfWeek, byte * dayOfMonth, byte * month, byte * year) {
  Wire.beginTransmission(DS3231_I2C_ADDRESS);
  Wire.write(0);
  Wire.endTransmission();
  Wire.requestFrom(DS3231_I2C_ADDRESS, 7);
  *second = bcdToDec(Wire.read() & 0x7f);
  *minute = bcdToDec(Wire.read());
  *hour = bcdToDec(Wire.read() & 0x3f);
  *dayOfWeek = bcdToDec(Wire.read());
  *dayOfMonth = bcdToDec(Wire.read());
  *month = bcdToDec(Wire.read());
  *year = bcdToDec(Wire.read());
}

void loop() {
  if (loopCount == 1000){
    Serial.print(F("loop number "));
    Serial.print(loopNr); Serial.print(F("   -    1000 loops took "));
    Serial.print(millis() - myMillis); 
    Serial.println(F(" millis"));
    myMillis = millis();
    loopCount = 0;
  }
  readDS3231time(&second, &minute, &hour, &dayOfWeek, &dayOfMonth, &month, &year);

  serialCommunication();
  if (newData) processData();
  
  if (millis() - waitSomeSec > 2000) {
    if (dayOfMonth < 10) Serial.print(F("0"));
    Serial.print(dayOfMonth);
    Serial.print(F("/"));
    if (month < 10) Serial.print(F("0"));
    Serial.print(month);
    Serial.print(F("/"));
    Serial.print(year);
    Serial.print(F(" "));
    if (hour < 10) Serial.print(F("0"));
    Serial.print(hour);
    Serial.print(F(":"));
    if (minute < 10) Serial.print(F("0"));
    Serial.print(minute);
    Serial.print(F(":"));
    if (second < 10) Serial.print(F("0")); 
    Serial.println(second);
    waitSomeSec = millis();
  }
  loopNr++;
  loopCount++;
}

void serialCommunication() {
  char recieved;
  static char currentLine[30];
  static byte charPos = 0;
  while (Serial1.available()) {
    recieved = Serial1.read();
    if (recieved == '\n') {
      if (!strncmp(currentLine, "GET /", 5)) {
        page = currentLine[5];
        data = currentLine[6];
        if (currentLine[7] == '=') {
          sscanf(currentLine, "GET /%c%c=%ld:", page, data, &value);
        }
        else if (currentLine[7] == '

) {
          sscanf(currentLine, “GET /%c%c$%20[^:]”, page, data, valueString);
        }
        newData = true;
      }
      charPos = 0;
    }
    else if (recieved != ‘\r’) {  // if you got anything else but a carriage return character,
      currentLine[charPos] = recieved;      // add it to the end of the currentLine
      charPos++;    // and go to the next char posistion
      currentLine[charPos] = ‘\0’;    // null terminate the string
    }
  }
}

void processData() {
  switch (page) {
    case ‘A’: Serial1.print(F(“Home_Data|”));
      for (byte i = 0; i < sizeof(tempVal) / sizeof(float); i++) {
        Serial1.print(tempVal[i]);
        Serial1.print(F("|"));
        Serial1.print(minTempVal[i]);
        Serial1.print(F("|"));
        Serial1.print(maxTempVal[i]);
        Serial1.print(F("|"));
      }
      Serial1.print(pHval);
      Serial1.print(F("|"));
      Serial1.print(min_pHval);
      Serial1.print(F("|"));
      Serial1.print(max_pHval);
      Serial1.print(F("|"));
      for (byte i = 0; i < sizeof(Relay); i++) {
        Serial1.print(digitalRead(Relay[i]));
        Serial1.print(F("|"));
      }
      Serial1.print(activeLight);
      Serial1.print(F("|"));
      byte nextLL;
      if (activeLight >= sizeof(RGB_startTimeMode) / sizeof(long)) {
        nextLL = activeLight - (sizeof(RGB_startTimeMode) / sizeof(long)) + 1;
        if (nextLL > sizeof(RGB_startTimeMode) / sizeof(long)) nextLL = 0;
        Serial1.print(RGB_startTimePhase[nextLL]);
        Serial1.print(F("|"));
      }
      else if (activeLight < sizeof(RGB_startTimePhase) / sizeof(long)){
        Serial1.print(RGB_startTimeMode[activeLight]);
        Serial1.print(F("|"));
      }
      for (byte i = 0; i < sizeof(bottleSize) / sizeof(int); i++) {
        Serial1.print(round((float)100 / (float)bottleSize[i] * bottleLevel[i]));
        Serial1.print(F("|"));
      }
      for (byte i = 0; i < sizeof(statusNo); i++) {
        Serial1.print(statusNo[i]);
        Serial1.print(F("|"));
      } Serial1.println();
      reset();
      break;
    case ‘E’: switch (data) { //Temperatuur Instellingen
        case ‘Z’:
          Serial1.print(F(“Temp_Settings_Data|”));
          for (byte i = 0; i < sizeof(tempVal) / sizeof(float); i++) {
            Serial1.print(tempVal[i]);
            Serial1.print(F("|"));
            Serial1.print(tempCalVal[i]);
            Serial1.print(F("|"));
          }
          for (byte i = 0; i < sizeof(heatOnOffTemp) / sizeof(float); i++) {
            Serial1.print(heatOnOffTemp[i]);
            Serial1.print(F("|"));
            Serial1.print(tempWarning[i]);
            Serial1.print(F("|"));
          } Serial1.println();
          reset();
          break;
        case ‘Y’:
          Serial1.print(F(“Temp_Data|”));
          for (byte i = 0; i < sizeof(tempVal) / sizeof(float); i++) {
            Serial1.print(tempVal[i]);
            Serial1.print(F("|"));
          } Serial1.println();
          reset();
          break;
        case ‘A’: if (value == 1) {
            minTempVal[0] = tempVal[0];
            Serial1.println(F(“tOk”));
          }
          reset();
          break;
        case ‘B’: if (value == 1) {
            maxTempVal[0] = tempVal[0];
            Serial1.println(F(“tOk”));
          }
          reset();
          break;
        case ‘C’: if (value == 1) {
            minTempVal[1] = tempVal[1];
            Serial1.println(F(“tOk”)); //Serial.println(F(“tOk”));
          }
          reset();
          break;
        case ‘D’: if (value == 1) {
            maxTempVal[1] = tempVal[1];
            Serial1.println(F(“tOk”));
          }
          reset();
          break;
        case ‘E’: if (value == 1) {
            minTempVal[2] = tempVal[2];
            Serial1.println(F(“tOk”));
          }
          reset();
          break;
        case ‘F’: if (value == 1) {
            maxTempVal[2] = tempVal[2];
            Serial1.println(F(“tOk”));
          }
          reset();
          break;
        case ‘G’:
          tempCalVal[0] = (float) value / 100;
          Serial1.println(F(“tOk”));
          reset();
          break;
        case ‘H’:
          tempCalVal[1] = (float) value / 100;
          Serial1.println(F(“tOk”));
          reset();
          break;
        case ‘I’:
          tempCalVal[2] = (float) value / 100;
          Serial1.println(F(“tOk”));
          reset();
          break;
        case ‘J’:
          heatOnOffTemp[0] = (float) value / 100;
          Serial1.println(F(“tOk”));
          reset();
          break;
        case ‘K’:
          heatOnOffTemp[1] = (float) value / 100;
          Serial1.println(F(“tOk”));
          reset();
          break;
        case ‘L’:
          tempWarning[0] = (float) value / 100;
          Serial1.println(F(“tOk”));
          reset();
          break;
        case ‘M’:
          tempWarning[1] = (float) value / 100;
          Serial1.println(F(“tOk”));
          reset();
          break;
      }
  }
}

void reset() {
  page = ’ ';
  data = ’ ';
  value = 0;
  newData = false;
}

Serial monitor:

07/06/19 22:55:25
loop number 44000   -    1000 loops took 1056 millis
loop number 45000   -    1000 loops took 1052 millis
07/06/19 22:55:27
loop number 46000   -    1000 loops took 1053 millis
loop number 47000   -    1000 loops took 1052 millis
07/06/19 22:55:30
loop number 48000   -    1000 loops took 1052 millis
loop number 49000   -    1000 loops took 1053 millis
07/06/19 22:55:32
loop number 50000   -    1000 loops took 1053 millis
loop number 51000   -    1000 loops took 75 millis
loop number 52000   -    1000 loops took 65 millis
loop number 53000   -    1000 loops took 66 millis
loop number 54000   -    1000 loops took 66 millis
loop number 55000   -    1000 loops took 66 millis
loop number 56000   -    1000 loops took 65 millis
loop number 57000   -    1000 loops took 66 millis
loop number 58000   -    1000 loops took 65 millis
loop number 59000   -    1000 loops took 67 millis
loop number 60000   -    1000 loops took 65 millis
loop number 61000   -    1000 loops took 66 millis
loop number 62000   -    1000 loops took 66 millis
loop number 63000   -    1000 loops took 65 millis
loop number 64000   -    1000 loops took 67 millis
loop number 65000   -    1000 loops took 65 millis
loop number 66000   -    1000 loops took 66 millis
loop number 67000   -    1000 loops took 65 millis
loop number 68000   -    1000 loops took 66 millis
loop number 69000   -    1000 loops took 66 millis
loop number 70000   -    1000 loops took 66 millis
07/06/19 22:55:54
loop number 71000   -    1000 loops took 65 millis
loop number 72000   -    1000 loops took 66 millis
loop number 73000   -    1000 loops took 66 millis

Notice the time difference between the last two times on the monitor, according to the amount of millis, it should be 2 seconds, but its 20sec.
The only thing I did was changing the tempCalVal[0] with my phone (case 'E', case 'G' in processData).

Anyone has any idea of what is going wrong?

I must be missing it but what part of your code indicates the beginning of a proper serial sentence?
Here is a task I use to capture incoming serial data.

void fReceiveSerial_LIDAR( void * parameters  )
{
  bool BeginSentence = false;
  sSerial.reserve ( StringBufferSize300 );
  char OneChar;
  for ( ;; )
  {
    EventBits_t xbit = xEventGroupWaitBits (eg, evtReceiveSerial_LIDAR, pdTRUE, pdTRUE, portMAX_DELAY);
    if ( LIDARSerial.available() >= 1 )
    {
      while ( LIDARSerial.available() )
      {
        OneChar = LIDARSerial.read();
        if ( BeginSentence )
        {
          if ( OneChar == '>')
          {
            if ( xSemaphoreTake( sema_ParseLIDAR_ReceivedSerial, xSemaphoreTicksToWait10 ) == pdTRUE )
            {
              xQueueOverwrite( xQ_LIDAR_Display_INFO, ( void * ) &sSerial );
              xEventGroupSetBits( eg, evtParseLIDAR_ReceivedSerial );
            }
            BeginSentence = false;
            break;
          }
          sSerial.concat ( OneChar );
        }
        else
        {
          if ( OneChar == '<' )
          {
            sSerial = ""; // clear string buffer
            BeginSentence = true; // found begining of sentence
          }
        }
      } //  while ( LIDARSerial.available() )
    } //if ( LIDARSerial.available() >= 1 )
    xSemaphoreGive( sema_ReceiveSerial_LIDAR );
  }
  vTaskDelete( NULL );
} //void fParseSerial( void * parameters  )

The ESP32 task does not start saving the received serial data till a beginning of sentence marker has been received. In this way I do not process partial or incomplete sentences.

I have the same feeling as Idahowalker.
The problem could be a stack overflow or overwriting other memory locations. I can see that happening, because the code is not safe.

In the function "serialCommunication()" you set "newData" to true, even if there was not complete or wrong data. The data is incoming serial data. When the function "serialCommunication()" exits, then the data might not be complete. There could still be data coming in. Therefor, the "page" or "data" could have something that you are not expecting.
The "currentLine" is filled without limit, when there was no '\n' and no '\r' then it will overflow.
If the "page" or the "data" had wrong data, then the function "processData()" will never call the reset() function.

What is this:

long value;
char page;
char data;
char valueString[20];

sscanf(currentLine, "GET /%c%c=%ld:", page, data, &value);
sscanf(currentLine, "GET /%c%c$%20[^:]", page, data, valueString);

I suggest to make a small test sketch to get the sscanf right. The "page" and "data" should be pointers and I don't understand the "%20[^:]". Perhaps it is better not to use sscanf, and use (a lot of) code to extract the data from the text.

@Coos, do you really need to check the content of currentLine while a message is being received?

My strong preference would be to receive a complete message and only parse the content after a full message has arrived. That allows the receiving code and the parsing code to be simpler. That's how things work in the examples in Serial Input Basics

...R

Koepel:
What is this:

long value;

char page;
char data;
char valueString[20];

sscanf(currentLine, "GET /%c%c=%ld:", page, data, &value);
sscanf(currentLine, "GET /%c%c$%20[^:]", page, data, valueString);



I suggest to make a small test sketch to get the sscanf right. The "page" and "data" should be pointers and I don't understand the "%20[^:]". Perhaps it is better not to use sscanf, and use (a lot of) code to extract the data from the text.

The compiler should be giving you warning about the sscanf statements. I believe the correct format would be:

   sscanf(currentLine, "GET /%c%c=%ld:", &page, &data, &value);
   sscanf(currentLine, "GET /%c%c$%20[^:]", &page, &data, valueString);

You have also declared valueString as a 20 character array, you will need to be careful how you use that, since there is no space for a terminating character.

sscanf returns an integer value of the number of fields successfully matched, you may want to check that as verification that your input was valid.

"%20[^:]" will match up to 20 characters that are not a ":" , presumably the input data terminates with the ":" character.

david_2018:
"%20[^:]" will match up to 20 characters that are not a ":"

I see, I found this: scanf - C++ Reference
But the third parameter 'valueString' is then not used because there is no '%s' ? or is the [^:] used as a specifier for a string as well ?

Koepel:
I see, I found this: scanf - C++ Reference
But the third parameter ‘valueString’ is then not used because there is no ‘%s’ ? or is the [^:] used as a specifier for a string as well ?

I’m not that familiar with the specifics, but testing on an arduino shows the %20[^:] producing the correct results in valueString, and the results don’t vary if you use %20[^:]s or %20[^:]c

One thing I did notice is that sscanf is placing a null at the end of the characters it places into valueString, so it will be necessary to declare valueString with 21 elements.

Thanks a lot guys. Using pointers with sscanf seems to be the solution indeed.

I will implement the other suggestions, because they make a lot of sense.

Right now I have this:

void serialCommunication() {
  char recieved;
  static bool beginFound;
  char startMarker = '

I also changed the declaration of valueString to

char valueString[21];

Thanks for solving and improving my code.;
  char endMarker = '*';
  const byte numChars = 30;
  static char currentLine[numChars];
  static byte charPos = 0;
  while (Serial1.available() && !newData) {
    recieved = Serial1.read();
    if (beginFound){
      if (recieved == endMarker){
        page = currentLine[0];
        data = currentLine[1];
        Serial.print("page: "); Serial.print(page);
        Serial.print("      data: "); Serial.print(data);
        if (currentLine[2] == '=') {
          sscanf(currentLine, "%c%c=%ld:", &page, &data, &value);
          Serial.print("      value: "); Serial.print(value);
        }
        else if (currentLine[2] == '|') {
          sscanf(currentLine, "%c%c$%20[^:]", &page, &data, valueString);
        } Serial.println();
        beginFound = false;
        newData = true;
      } else {
        currentLine[charPos] = recieved;
        charPos++;
        if (charPos >= numChars) {
          charPos = numChars - 1;
        }
        currentLine[charPos] = '\0';
      }
    }
    else if (recieved == startMarker){
      beginFound = true;
      charPos = 0;
    }
  }
}


I also changed the declaration of valueString to 

§DISCOURSE_HOISTED_CODE_1§


Thanks for solving and improving my code.

Suppose the begin-marker was detected and that the Wire.available() does not see any character in the buffer, but characters are still being received (it is a serial bus).
The while-statement will end, and then the loop() will run again. The variables will reset, and the rest of the characters are written at the begin of 'currentLine'.

The Serial1 is 115200 baud, that is fast. But you are hoping that the sketch is slow enough, so the whole incoming string is handled inside the while-statement. It does not have to be so.

For myself, I prefer to check Wire.available() and then read just one character, see my millis_serial_timeout.ino example.

I'm using static variables where needed, so they will not reset if the while loop ends and start again, so that shouldn't be a problem, right?

Okay, I missed that, sorry :blush:
So the 'newData' is global and 'charPos' and 'beginFound' are static. Yes, that will work :smiley:

When a lot of wrong characters are read, for example when the baudrate is wrong, then that data will not be written beyond the array. That is good. Suppose one character of the wrong data is accidently a '$', but there is never a ''. Then the 'currentLine' will be always fully filled. Suppose after that the right baudrate is set and normal data is send. Then the sketch does not read the first command. Only after a '' it will start to read the incoming characters in a normal way.

I suggest to call reset() once at the end of processData(). Otherwise a wrong page (for example 'Q') stops reading new data. Just ignore the wrong data.

What if the data is only "$*" ? You could add an extra check.

You already use the 'const' keyword. The 'startMarker' and 'endMarker' are also constants :wink:

Maybe these things are far-fetched, but I like to keep the sketch going, regardless of the incoming data.
I think an improvement can be make by splitting the different functionality into different parts.
First parts reads the serial data, fills the 'currentLine' buffer and restarts with every '$' and stops with ''. Put the '' in the buffer as well (easier to debug when printed to the serial monitor). Everything that is wrong is ignored.
Second part, checks if there is data and converts it. If there is no real data or the data is wrong, it is ignored.
Third part processes the command/data. If the command/data is unknown (for example page 'Q'), then it is ignored.

You have now the first part and the second part in one piece of code.

Maybe those are improvements for later.
Thank you that I learned a new thing about the "%20[^:]" sscanf format 8)

Koepel:
Okay, I missed that, sorry :blush:
So the 'newData' is global and 'charPos' and 'beginFound' are static. Yes, that will work :smiley:

When a lot of wrong characters are read, for example when the baudrate is wrong, then that data will not be written beyond the array. That is good. Suppose one character of the wrong data is accidently a '$', but there is never a ''. Then the 'currentLine' will be always fully filled. Suppose after that the right baudrate is set and normal data is send. Then the sketch does not read the first command. Only after a '' it will start to read the incoming characters in a normal way.

Good point, although in my case that is not a problem because my phone while simple resend the request until it got the response it wants...

I suggest to call reset() once at the end of processData(). Otherwise a wrong page (for example 'Q') stops reading new data. Just ignore the wrong data.

What if the data is only "$*" ? You could add an extra check.

You already use the 'const' keyword. The 'startMarker' and 'endMarker' are also constants :wink:

Will do that as well. Putting a reset() at the end of processData, actually makes the other resets() unnecessary... I like that.

Thanks again.