Need help debugging arduino code

I have a below test code to test firing some arduino code at the send of a command instead of at the press of a button. Stuff is supposed to happen when I write '1' to serial, but I end up writing and sending many 1s and nothing happens, and then suddenly one stray time it does work. What's wrong?

int rnd;
unsigned long currentmillis;
bool SetCurrentMillis = false;

bool OperationStarted = false;

void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
}

void loop() {
  // put your main code here, to run repeatedly:

  if ((Serial.available() && Serial.read() == '1') || (OperationStarted == true)) {
    rnd = random(0, 1023);
    OperationStarted = true;

    if (SetCurrentMillis == false) {
      currentmillis = millis();
      SetCurrentMillis = true;
    }

    if ((millis() - currentmillis) <= 200) {
      Serial.print(50);
      Serial.print("\t");
      Serial.print(100);
      Serial.print("\t");
      Serial.print(150);
      Serial.print("\t");
      Serial.print(200);
      Serial.print("\t");
      Serial.print(250);
      Serial.print("\t");
      Serial.print(300);
      Serial.print("\t");
      Serial.print(300);
      Serial.print("\t");
      Serial.print(rnd);
      Serial.print("\t");
      Serial.print(55);
      Serial.print("\t");
      Serial.print(110);
      Serial.print("\t");
      Serial.print(175);
      Serial.print("\t");
      Serial.print(225);
      Serial.print("\t");
      Serial.print(268);
      Serial.print("\t");
      Serial.print(364);
      Serial.print("\t");
      Serial.print(398);
      Serial.print("\t");
      Serial.println(rnd + 57);
    }
  } else if (Serial.available() && Serial.read() == '0') {
    OperationStarted = false;
    SetCurrentMillis = false;
  }
} 

when you do this and have sent '0'

the read() "eats up" the '0' (it's removed from the incoming buffer)... so your next test

won't see it

➜ if something is available on Serial, then read it in a variable and then decide what to do


organise your code as a state machine, it will make your life easier, your operationStarted bool was a good idea to decide what state you are in

click to see the code
unsigned long lastPrinting;
bool operationStarted = false;

void setup() {
  Serial.begin(115200);
  Serial.println(F("type 1 to start printing every 200ms and 0 to stop"));
}

void loop() {
  int r = Serial.read(); // will be -1 if nothing is available to read
  if (!operationStarted && r == '1') {
    operationStarted = true;
    lastPrinting = millis() - 200;
    Serial.println(F("STARTING"));
  }
  else if (operationStarted && r == '0') {
    Serial.println(F("STOPING"));
    operationStarted = false;
  }

  if (operationStarted) {
    if ((millis() - lastPrinting) >= 200) {
      Serial.println("Tick");
      lastPrinting = millis();
    }
  }
}

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

void loop() {
  static unsigned long currentmillis = 0;
  static bool SetCurrentMillis = false;
  static bool OperationStarted = false;
  static char ch = 0;

  if (Serial.available() > 0) {
    char c = Serial.read(); 
    if (c == '1' || c == '0')ch = c;
  }
  if ( ch  == '1' ) {
    int rnd = random(0, 1023);

    if (SetCurrentMillis == false) {
      currentmillis = millis();
      SetCurrentMillis = true;
    }

    if (millis() - currentmillis <= 200) {
      Serial.print("50\t100\t150\t200\t250\t300\t300\t");
      Serial.print(rnd);
      Serial.print("\t55\t110\t175\t225\t268\t364\t398\t");
      Serial.println(rnd + 57);
    }
  }
  else if (ch == '0') {
    OperationStarted = false;
    SetCurrentMillis = false;
    ch = 0;
  }
}

Or, possibly? (building on @kolaha 's contribution).

void setup() {
  Serial.begin(115200);
}
void loop() {
  static unsigned long currentmillis = 0;
  static bool SetCurrentMillis = false;
  static bool OperationStarted = false;
  if (Serial.available() > 0) {
    switch (Serial.read()) {
      case'0':
        OperationStarted = false;
        SetCurrentMillis = false;
        break;
      case'1':
        int rnd = random(0, 1023);
        if (SetCurrentMillis == false) {
          currentmillis = millis();
          SetCurrentMillis = true;
        }
        if (millis() - currentmillis <= 200) {
          Serial.print("50\t100\t150\t200\t250\t300\t300\t");
          Serial.print(rnd);
          Serial.print("\t55\t110\t175\t225\t268\t364\t398\t");
          Serial.println(rnd + 57);
        }
        break;
    }
  }
}

you only print when you get the '1' and more than 200ms had passed. Although it matches his original code (non working) behavior, I'am not sure it was OP's intent - would need clarification

My understanding of OP's expectations was that sending 1 would get the arduino to log stuff out every 200ms and sending 0 would stop the logging. I might have been wrong.

no, he want only first 200ms of outputs if it not interrupted by receiving '0'.
and it will never interrupted.

it starts after '1'
sends every 200ms
stops after '0'
you'r welcome

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

void loop() {
  static unsigned long lasttmillis = 0;
  static bool OperationStarted = false;
  static char ch = 0;

  if (Serial.available() > 0) ch = Serial.read();
  if (ch == '1') OperationStarted = true;
  if (ch == '0') OperationStarted = false;
  if (millis() - lasttmillis >= 200 && OperationStarted) {
    lasttmillis = millis();
    int rnd = random(0, 1023);
    Serial.print("50\t100\t150\t200\t250\t300\t300\t");
    Serial.print(rnd);
    Serial.print("\t55\t110\t175\t225\t268\t364\t398\t");
    Serial.println(rnd + 57);
  }
}

So the code you guys did offer initially does not do that since the printing happens only when you get something on Serial and it’s a 1

yes, topic owner call it:

what I want to do is that when I press 1, for 200mS the code should print stuff. But if before the 200mS is passed I press 0, then the code should stop printing stuff.

ok problem solved, correct code below. Ignore the changes in the text being printed.

int rnd;
  unsigned long currentmillis;
bool SetCurrentMillis = false;

bool OperationStarted = false;

void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
}

void loop() {
  // put your main code here, to run repeatedly:
rnd = random(0, 1023);
if ((Serial.available() && Serial.read() == '1') || (OperationStarted == true)) 
{
    
    OperationStarted = true;
    if (SetCurrentMillis == false) 
    {
      currentmillis = millis();
      SetCurrentMillis = true;
    }

    if ((millis() - currentmillis) <= 5000) 
    {
      Serial.print("50");
      Serial.print("\t");
      Serial.print("100");
      Serial.print("\t");
      Serial.print("150");
      Serial.print("\t");
      Serial.print("200");
      Serial.print("\t");
      Serial.print("250");
      Serial.print("\t");
      Serial.print("300");
      Serial.print("\t");
      Serial.print("300");
      Serial.print("\t");
      Serial.print("x");
      Serial.print("\t");
      Serial.print("55");
      Serial.print("\t");
      Serial.print("110");
      Serial.print("\t");
      Serial.print("175");
      Serial.print("\t");
      Serial.print("225");
      Serial.print("\t");
      Serial.print("268");
      Serial.print("\t");
      Serial.print("364");
      Serial.print("\t");
      Serial.print("398");
      Serial.print("\t");
      Serial.println("y");
    }
    else
    {
      OperationStarted = false;
      SetCurrentMillis = false;
    }
  } 
    if (Serial.available() && Serial.read() == '0') 
    {
        OperationStarted = false;
        SetCurrentMillis = false;
    }
}

no you still have the issue of reading twice the Serial line

and so you depend on when the byte becomes available

As said in post #2

correct? ok, good luck.

got it. Will save it to variable and only then use it. Thanks.

This good?

int rnd;
  unsigned long currentmillis;
bool SetCurrentMillis = false;

bool OperationStarted = false;

void setup() {
  // put your setup code here, to run once:
  Serial.begin(115200);
}

void loop() {
  int r = Serial.read();
  // put your main code here, to run repeatedly:
rnd = random(0, 1023);
if ((r == '1') || (OperationStarted == true)) 
{
    
    OperationStarted = true;
    if (SetCurrentMillis == false) 
    {
      currentmillis = millis();
      SetCurrentMillis = true;
    }

    if ((millis() - currentmillis) <= 5000) 
    {
      //do stuff
    }
    else
    {
      OperationStarted = false;
      SetCurrentMillis = false;
      // do stuff
      
    }
  } 
    if (r == '0') 
    {
        OperationStarted = false;
        SetCurrentMillis = false;
    }
}

Better indeed

For clarity I would really separate the serial from the state machine rather than use the || (or) operator in the if - but that’s personal preference.

2 Likes

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