5

I made a simple mobile application to send LED_ON or LED_OFF when a button is clicked. In the below code the while loop is not fully completed while execution but the controller is going to the void loop() and continuing from there.

char command;
String my_final="LED_OFF";
#define led 9

void setup(){
  Serial.begin(9600);
  pinMode(led, OUTPUT);
  Serial.println("ready");
}

void loop(){
  if(Serial.available() > 0){
    my_final = "";
    while(Serial.available() > 0){
      command = (byte)Serial.read();
      my_final += command;
      Serial.println("test");
    }
    Serial.println(my_final);
    }

  if(my_final == "LED_ON"){
    Serial.println(my_final);
    analogWrite(led, 255);
    my_final == "LED_ON";
  }

  if(my_final == "LED_OFF"){
    Serial.println(my_final);
    analogWrite(led, 0);
    my_final == "LED_OFF";
  }
}

The main problem happens in my_final="" as i have to do this to accept new input from the bluetooth. i just cant seem to find a way around this problem.

EDIT

This is what im getting in the serial monitor. test L test E test D test _ test O test N.

  • What you describe in your question is pretty much impossible. Have you really seen in the debugger that the execution exits the `while` loop with condition `Serial.available()` still being true? – Dmitry Grigoryev May 07 '18 at 09:36
  • this is what im getting in the serial monitor.. `test L test E test D test _ test O test N` – ganesh vicky May 07 '18 at 09:41
  • 1
    Perhaps you should state *what* you're getting in the serial monitor in your question then. Currently, your question reads as if you have seen the execution jump out of the `while` loop, which is impossible to see with a serial monitor. – Dmitry Grigoryev May 07 '18 at 09:43
  • 1
    What is `my_final == "LED_ON";` supposed to do? (at the end of the `if(my_final == "LED_ON"){}` block, and likewise for `OFF`) – AaronD May 07 '18 at 20:52
  • How do you know the while loop is fully completed? – user253751 May 07 '18 at 22:49
  • I forgot to remove those two lines which I used before this for some reason. – ganesh vicky May 08 '18 at 01:37

4 Answers4

9

Serial communication is transmitting data one byte at a time. Your code is fast enough to read and process one byte before the next one is received.

There are many possible solutions to this, and using delays is not a good one.

You could send a marker character at the end of each command, e.g. LED_ON!. Then you know you have to keep adding characters to your array until you see an exclamation mark. A common candidate for a marker is the newline character.

Another solution is to make sure all commands have the same length (e.g. 6 bytes). Then you could simply wait until you receive so many characters, that is

if(Serial.available() >= 6) ...

Beware that in the second solution, if you lose a single byte you will not be able to receive commands correctly until you re-sync. In order to re-sync, you could for example throw away the contents of Serial after a timeout, if an incomplete command is sitting there for too long.

Dmitry Grigoryev
  • 25,576
  • 5
  • 45
  • 106
  • 2
    Another option: shift characters into a buffer one at a time, dropping the oldest one for each new one, and check to see if the content of the buffer is a valid command. Or use single character commands. – Jules May 07 '18 at 13:03
  • 1
    @Jules using a circular buffer would require a guarantee that commands cannot contains other commands as sub-strings. – Dmitry Grigoryev May 07 '18 at 13:14
  • 1
    A more extensible protocol might be to reserve the first few (1 to 4, depending upon the max expected command/message length) bytes for encoding the length of the message. Then your algorithm is basically 'read the message size -> read the message'. – aroth May 08 '18 at 02:54
  • @aroth Sure, that is also possible. Re-sync algorithm would still be needed though. – Dmitry Grigoryev May 08 '18 at 09:22
5

@9600 Baudrate, you are receiving data at 10 bits per millisecond. One character is 8 bits + start and stop bits in a frame. You are reading the buffer faster than it. So a simple solution would be to put a delay of > 1 ms or so after serial read(), inside the loop.

Mitu Raj
  • 10,843
  • 6
  • 23
  • 46
  • 4
    This is perhaps the simplest thing the OP could try to confirm the source of their problem. In the long run however, I wouldn't recommend it. There are so many ways in which delay-based code can fail... – Dmitry Grigoryev May 07 '18 at 13:00
  • 1
    Yes true... data loss can occur when operating at higher baud rate with continuous data stream... – Meenie Leis May 07 '18 at 13:02
  • Yep. Buffer Overflow in that case. Agreed. – Mitu Raj May 07 '18 at 13:05
  • 1
    avoiding buffer overflow can be done by only `delay(1)`ing after the `read()` when `Serial.available() == 0` – ratchet freak May 07 '18 at 13:20
  • 2
    @MITURAJ I actually meant something else. When you write a string like "LED_OFF" to a COM port on a computer, there's no guarantee that all characters will be sent without delay. The same is true for microcontrollers where the UART ISR doesn't have the highest priority. – Dmitry Grigoryev May 07 '18 at 13:22
2

Buffering until a linefeed might look like this (my arduino code is a bit rusty, so please forgive simple errors - the algorithm should be correct):

finished = false;
command = "";
while (!finished) {
   if (Serial.available() > 0) {
      c = (byte)Serial.read();
      if (c == '\n') {
         finished = true;
      }
      else {
         command += c;
      }
   }
}

This loop will just keep reading characters and adding these characters to the variable command.

This assumes that the communications link is perfect, in that it will not drop or mangle characters. To make this more robust, you could put a check in to make sure that the time to receive the command isn't too long.

You also might want to respond to each command with either an ack response or a huh? (nack) response, letting the server know what's going on.

Happy making!

NomadMaker
  • 121
  • 3
1

What happens when the incoming datastream looks like this:

LE..Td..D_O..Td..N?

Td is a random small delay.

If your while(Serial.available() > 0){ is faster than Td, you will have reset my_final = ""; with only LE in it. Meaning, next you receive D_O, and then you reset it again.

Jeroen3
  • 21,976
  • 36
  • 73