Serial.read with Pushbutton

Hello there,

once again I’m asking for your help:

I want to create a programm in which the following things happen:

  1. If “1-2-3” is entered via the Serial Port, a LED starts to light. If any other String is entered, it shouldn’t light.
  2. If I press (and release) the button, the activated LED should go out and stay out.

I already have the following programm.

My problem is, that the LED is lighting up again, after I release the button.

How do I have to adapt the code, that it stays out?

Thanks for your help in advance,

Peter

char c;
String Zeile;
String Feld[10];
int FeldAnz;
int LED=11;
int Button=10;


void setup() {
 Serial.begin(9600);
 pinMode(LED,OUTPUT);
 pinMode(Button,INPUT);
 digitalWrite(LED,LOW);
}

String ZeileLesen(){
  Zeile="";
  while(Serial.available()){
    c=Serial.read();
    Zeile+=c;
    delay(2);
  }
  return Zeile;
}

void PositionLesen(){
  FeldAnz=1;
  for(int i=0;i<10;i++){
    Feld[i]="";
     }
  int Len=7;
  int Pos=2;                            
  while((Pos<Len)){
    c=Zeile[Pos];
    Feld[FeldAnz-1]+=c;
    Pos++;
     }
 }


void loop() {
  while(Serial.available()){
    ZeileLesen();
    PositionLesen();
    for(int i=0;i<FeldAnz;i++){
      Serial.println(Feld[i]);
              }
        }
    for(int i=0;i<FeldAnz;i++){           
    if (Feld[i]=="1-2-3"){
    digitalWrite(LED,HIGH);
    
  while(digitalRead(Button)==HIGH){ 
       digitalWrite(LED,LOW); }
      }
  else{digitalWrite(LED,LOW);
      }
   }
}
const int LED=11;
const int Button=10;

void setup() {
 Serial.begin(9600);
 pinMode(LED,OUTPUT);
 pinMode(Button,INPUT);
 digitalWrite(LED,LOW);
}

void loop() {
  int control = readPushbutton();
  control += readSerial();// prevents simultaneous control conflict
  if (control == '1') digitalWrite(LED,LOW);
  else if (control == '2') digitalWrite(LED,HIGH);
}

int readPushbutton() {
  if (digitalRead(Button)==HIGH) return '1';
  return 0;
}

int readSerial() {
  static byte charactersRead = 0;
  static unsigned long stringTimestamp;
  if (charactersRead > 0) {
    if (millis() - stringTimestamp >= 200UL) charactersRead = 0;// cleans incomplete words after 200 ms
  }
  if (Serial.available()) {
    int c = Serial.read();
    if (charactersRead == 0 && c == '1') {
        stringTimestamp = millis();
        charactersRead++;
    }
    else if ((charactersRead == 1 && c == '-') || (charactersRead == 2 && c == '2') || (charactersRead == 3 && c == '-')) charactersRead++;
    else if (charactersRead == 4 && c == '3') {
        charactersRead = 0;
        return '2';
    }
    else charactersRead = 0;
  }
  return 0;
}

peter368:
Hello there,

once again I’m asking for your help:

I want to create a programm in which the following things happen:

  1. If “1-2-3” is entered via the Serial Port, a LED starts to light. If any other String is entered, it shouldn’t light.
  2. If I press (and release) the button, the activated LED should go out and stay out.

I already have the following programm.

My problem is, that the LED is lighting up again, after I release the button.

How do I have to adapt the code, that it stays out?

Thanks for your help in advance,

Peter

I can’t get the LED to light unless I enter “1-1-2-3” with this code.

Perehama:

const int LED=11;

const int Button=10;

void setup() {
Serial.begin(9600);
pinMode(LED,OUTPUT);
pinMode(Button,INPUT);
digitalWrite(LED,LOW);
}

void loop() {
  int control = readPushbutton();
  control += readSerial();// prevents simultaneous control conflict
  if (control == '1') digitalWrite(LED,LOW);
  else if (control == '2') digitalWrite(LED,HIGH);
}

int readPushbutton() {
  if (digitalRead(Button)==HIGH) return '1';
  return 0;
}

int readSerial() {
  static byte charactersRead = 0;
  static unsigned long readingTimestamp;
  if (charactersRead > 0) {
    if (millis() - readingTimestamp >= 200UL) charactersRead = 0;// cleans incomplete words after 200 ms
  }
  if (Serial.available()) {
    int c = Serial.read();
    if (charactersRead == 0 && c == '1') {
        stringTimestamp = millis();
        charactersRead++;
    }
    else if (charactersRead == 1 && c == '-') {
        charactersRead++;
    }
    else if (charactersRead == 2 && c == '2') {
        charactersRead++;
    }
    else if (charactersRead == 3 && c == '-') {
        charactersRead++;
    }
    else if (charactersRead == 4 && c == '3') {
        charactersRead = 0;
        return '2';
    }
    else {
      charactersRead = 0;
    }
  }
  return 0;
}

I can't get this code to compile...

ToddL1962:
I can't get this code to compile...

Sorry, mate. I've refactored the code a couple of times.... try the following

const int LED=11;
const int Button=10;

void setup() {
 Serial.begin(9600);
 pinMode(LED,OUTPUT);
 pinMode(Button,INPUT);
 digitalWrite(LED,LOW);
}

void loop() {
  int control = readPushbutton();
  control += readSerial();
  if (control == '1') digitalWrite(LED,LOW);
  else if (control == '2') digitalWrite(LED,HIGH);
}

int readPushbutton() {
  if (digitalRead(Button)==HIGH) return '1';
  return 0;
}

int readSerial() {
  static byte charactersRead = 0;
  static unsigned long stringTimestamp;
  if (charactersRead > 0) {
    if (millis() - stringTimestamp >= 200UL) charactersRead = 0;// cleans incomplete words after 200 ms
  }
  if (Serial.available()) {
    int c = Serial.read();
    if (charactersRead == 0 && c == '1') {
        stringTimestamp = millis();
        charactersRead++;
    }
    else if ((charactersRead == 1 && c == '-') || (charactersRead == 2 && c == '2') || (charactersRead == 3 && c == '-')) charactersRead++;
    else if (charactersRead == 4 && c == '3') {
        charactersRead = 0;
        return '2';
    }
    else charactersRead = 0;
  }
  return 0;
}

Here is a very straightforward implementation using a state machine.

  1. If 1-2-3 is entered it will turn on the LED. If at any during serial input an incorrect character for the sequence is entered the program looks for the sequence to start over (i.e. begins looking for a '1' again.

  2. The LED will remain on until either you enter more serial data OR you press the button.

If you want the LED to remain on until the button is pressed then remove the digitalWrite(LED, LOW) from case 0:.

const int LED = 11;
const int Button = 10;
int state = 0;

void setup() 
{
  Serial.begin(9600);
  pinMode(LED, OUTPUT);
  pinMode(Button, INPUT);
  digitalWrite(LED, LOW);
}

void loop() 
{
  int c;
  while (!Serial.available()) 
  {
    if (digitalRead(Button)==HIGH) digitalWrite(LED, LOW);
  }
  c = Serial.read();
  Serial.print((char) c);
  switch (state)
  {
    case 0:
      digitalWrite(LED, LOW);
      if (c == '1') state++;
      break;
      
    case 1:
      if (c == '-') state++;
      else state = 0;
      break;

    case 2:
      if (c == '2') state++;
      else state = 0;
      break;

    case 3:
      if (c == '-') state++;
      else state = 0;
      break;

    case 4:
      state = 0;
      if (c == '3') 
      {
        digitalWrite(LED, HIGH);
      }
      break;
  }
}

ToddL1962:
Here is a very straightforward implementation using a state machine.

This is not straightforward. The "while (!Serial.available())" is a confusing backwards if statement. There is only one state of this machine. Using a switch case for what you are doing is a confusing backwards if... else statement. The breaks and cases don't improve readability of the code and naming your count "state" only further obfuscates the purpose.

1. Fig-1 describes your hardware setup.
sw1led1.png
Figure-1:

2. Fig-2 describes the solution of your problem in the form of Flow Chart.
sw1led1FC.png
Figure-2:

3. The following sketch is the the coding of Fig-2 (at conceptual/understanding level).

char myData[10] = "";    //array to haold string ("1-2-3") coming from Serial Monitor

void setup() 
{
  Serial.begin(9600);   //L1:
  pinMode(11, OUTPUT);  //L1:
  digitalWrite(11, LOW); //LED is OFF; L1
  pinMode(10, INPUT_PULLUP); //L1: K1 is at DPin-10
}

void loop() 
{
  byte n = Serial.available();  //L2:
  if (n !=0 ) //L2:
  {
    byte m = Serial.readBytesUntil('\n', myData, 10); //L2:  blocking code; keep reading until 
    //timeout happens of 1-sec (default) period or Newline ('\n') is detected or 10 charcaters are received.
    myData[m] = '\0';  //insert null-byte //L2:
    if(strcmp(myData, "1-2-3") == 0) //L2:  when two strings are equal, the function returns 0
    {
      digitalWrite(10, HIGH);   //L3: ; LED is ON
      while(digitalRead(10) != LOW)  //L4:
      {
        ;    //wait   L4:
      }
      digitalWrite(13, LOW);  //L5: LED is OFF
    }
  } //goto L2:
}

4. Upload the sketch of Step-3.

5. Bring in the Serial Monitor (Fig-3) with the selection of Newline option in the Line ending tab.


Figure-3:

(1) Enter 1-2-3 in the InputBox of the Serial Monitor and then click on the Send button.
(2) Check that LED is ON.
(3) Press down the button K1.
(4) Check that LED is OFF.
(5) Release the button K1.
(6) Check that LED is still OFF.

6. Goto Step-5 and repeat the process.

Note: You may find reading the post of this link useful.

sw1led1.png

sw1led1FC.png

Perehama:
This is not straightforward. The "while (!Serial.available())" is a confusing backwards if statement. There is only one state of this machine. Using a switch case for what you are doing is a confusing backwards if... else statement. The breaks and cases don't improve readability of the code and naming your count "state" only further obfuscates the purpose.

LOL. Have you ever written a state machine? This is a textbook implementation of a pattern-matching state machine. But hey, what do I know...

Perehama:
This is not straightforward. The "while (!Serial.available())" is a confusing backwards if statement. There is only one state of this machine. Using a switch case for what you are doing is a confusing backwards if... else statement. The breaks and cases don't improve readability of the code and naming your count "state" only further obfuscates the purpose.

Also, Golam's implementation is reasonable as well. It depends on how you interpret the requirements. My implementation takes in a continuous seral input looking for a pattern whereas Golam's expects an entire string at once that matches the required pattern. Two different interpretations of the requirement.

while (!Serial.available()) is a perfectly acceptable way of waiting for serial input and you will see many examples of that in this forum. Alternatively, I could have coded it this way:

const int LED = 11;
const int Button = 10;
int state = 0;

void setup() 
{
  Serial.begin(9600);
  pinMode(LED, OUTPUT);
  pinMode(Button, INPUT);
  digitalWrite(LED, LOW);
}

void loop() 
{
  int c;
  if(!Serial.available()) 
  {
    if (digitalRead(Button)==HIGH) digitalWrite(LED, LOW);
    return;
  }

  c = Serial.read();
  Serial.print((char) c);
  switch (state)
  {
    case 0:
      digitalWrite(LED, LOW);
      if (c == '1') state++;
      break;
      
    case 1:
      if (c == '-') state++;
      else state = 0;
      break;

    case 2:
      if (c == '2') state++;
      else state = 0;
      break;

    case 3:
      if (c == '-') state++;
      else state = 0;
      break;

    case 4:
      state = 0;
      if (c == '3') 
      {
        digitalWrite(LED, HIGH);
      }
      break;
  }
}

Or this way:

const int LED = 11;
const int Button = 10;
int state = 0;

void setup()
{
  Serial.begin(9600);
  pinMode(LED, OUTPUT);
  pinMode(Button, INPUT);
  digitalWrite(LED, LOW);
}

void loop()
{
  int c;
  if (Serial.available())
  {
    c = Serial.read();
    Serial.print((char) c);
    switch (state)
    {
      case 0:
        digitalWrite(LED, LOW);
        if (c == '1') state++;
        break;

      case 1:
        if (c == '-') state++;
        else state = 0;
        break;

      case 2:
        if (c == '2') state++;
        else state = 0;
        break;

      case 3:
        if (c == '-') state++;
        else state = 0;
        break;

      case 4:
        state = 0;
        if (c == '3')
        {
          digitalWrite(LED, HIGH);
        }
        break;
    }
  }
  else
  {
    if (digitalRead(Button) == HIGH) digitalWrite(LED, LOW);
  }
}

There are always multiple ways of coding the same thing.

Perehama:
This is not straightforward. The "while (!Serial.available())" is a confusing backwards if statement. There is only one state of this machine. Using a switch case for what you are doing is a confusing backwards if... else statement. The breaks and cases don't improve readability of the code and naming your count "state" only further obfuscates the purpose.

Or maybe you can understand this better:

const int LED = 11;
const int Button = 10;
int state = 0;

void setup()
{
  Serial.begin(9600);
  pinMode(LED, OUTPUT);
  pinMode(Button, INPUT);
  digitalWrite(LED, LOW);
}

void loop()
{
  int c;
  if (Serial.available())
  {
    c = Serial.read();
    Serial.print((char) c);
    switch (state)
    {
      case 0:
        digitalWrite(LED, LOW);
        if (c == '1') state = 1;
        break;

      case 1:
        if (c == '-') state = 2;
        else state = 0;
        break;

      case 2:
        if (c == '2') state = 3;
        else state = 0;
        break;

      case 3:
        if (c == '-') state = 4;
        else state = 0;
        break;

      case 4:
        state = 0;
        if (c == '3')
        {
          digitalWrite(LED, HIGH);
        }
        break;
    }
  }
  else
  {
    if (digitalRead(Button) == HIGH) digitalWrite(LED, LOW);
  }
}

Perehama:
This is not straightforward. The "while (!Serial.available())" is a confusing backwards if statement. There is only one state of this machine. Using a switch case for what you are doing is a confusing backwards if... else statement. The breaks and cases don't improve readability of the code and naming your count "state" only further obfuscates the purpose.

And if you want to talk about not straightforward how about the following function?

int readPushbutton() {
  if (digitalRead(Button)==HIGH) return '1';
  return 0;
}

In one case it returns a character and another returns 0?

And this is more obvious than my simple state machine?

int readSerial() {
  static byte charactersRead = 0;
  static unsigned long stringTimestamp;
  if (charactersRead > 0) {
    if (millis() - stringTimestamp >= 200UL) charactersRead = 0;// cleans incomplete words after 200 ms
  }
  if (Serial.available()) {
    int c = Serial.read();
    if (charactersRead == 0 && c == '1') {
        stringTimestamp = millis();
        charactersRead++;
    }
    else if ((charactersRead == 1 && c == '-') || (charactersRead == 2 && c == '2') || (charactersRead == 3 && c == '-')) charactersRead++;
    else if (charactersRead == 4 && c == '3') {
        charactersRead = 0;
        return '2';
    }
    else charactersRead = 0;
  }
  return 0;
}

ToddL1962:
LOL. Have you ever written a state machine? This is a textbook implementation of a pattern-matching state machine. But hey, what do I know...

I was trying to help you, not bruise your ego. I have written actual state machines in use professionally. You can go through my old posts and find textbook examples of state machines in my code. It's clear that by your continued misuse of the term "state machine" for a case switch argument and your use of a presumed "state machine" to achieve a count of bytes received, that you have no clue what a textbook state machine really is. To be clear, a state machine is not just the use of a switch case argument. A state machine is a mathematical model. If you look critically at the application from a flow-cart example and compare it to a diagram of a Mealy machine or a Moore machine, which are textbook implementations, you will discover there is only 1 state of the machine. Some might argue that because the LED is on and off, it is two states, and I will argue that despite the state of the output, the machine can still receive and respond to both serial and push-button, and therefore, is 1 state.
You are welcome to criticize my code, but I didn't claim it to be "very straightforward" Criticizing my code, does nothing to explain your code and the reasoning behind it. I can give you my reasoning for returning characters or counting the way that I did if you really want to know. You code fails to do 2 things my code did. 1) my code receives both inputs at all times, and prevents them from lighting or extinguishing the LED simultaneously. 2) My serial reception times out, so the operator can't type "1-" today, then tomorrow "2-" then come back in the evening and type "3", which I would argue is not valid, as it is not conceptually the same as "1-2-3". My code is also non-blocking, which yours and Golem's isn't. I am not posting with the goal to defend my code, ego or honor. I am not posting to attack, argue or belittle. The reason for my post was to educate; the reason for this post is to educate. And, if the tone of the discussion is reactionary and emotional, I will end this discussion.

Perehama:
My code is also non-blocking, which yours and Golem's isn't.

Because, the LED will become ON once the UNO receives this string "1-2-3" from the Serial Monitor. Therefore, the UNO has to wait until the string is received; so, I don't see any benefits of non-blocking codes. Moreover, this is a single tasking session -- the MCU is not required to poll any other inputs while waiting to receive the said string.

When @ToddL1962 has taken my solution as a "reasonable one", @Perehama has placed it in the side basket due to its blocking feature.

GolamMostafa:
you have placed it in the side basket due to its blocking feature bug.

Blocking code doesn't deserve to be in even the side basket. It should be way down under the basket below the bottom-most basket. It's just as easy to write code that doesn't block, and there's always the chance that the requirement could change. So just make it non-blocking and be done with that.

My question though is of the OP: why do you have input like 1-2-3, when on the face of it it seems you could just type an A or a P or a Z? Easier to check 1 character than 5- but I guess you could think of the 1-2-3 as a sort of password? Just curious.

evadne:
Blocking code doesn't deserve to be in even the side basket. It should be way down under the basket below the bottom-most basket.

Teaching/Learning methodology/strategy totally contradicts with your opinion. Please, post the discrete level non-blocking codes for byte m = Serial.readBytesUntil('\n', myData, 10); and then visualize why non-blocking codes are not good examples for the learners like the OP -- the OP might just get lost in the nested if-else structures.

GolamMostafa:
@Perehama has placed it in the side basket due to its blocking feature.

Mate, I was not criticizing your use of blocking code, or placing it in the side basket. You presented a very well laid out post. I was only explaining the thought behind my code and the reason it's not more straightforward.

GolamMostafa:
Teaching/Learning methodology/strategy totally contradicts with your opinion.

If by that you mean it's better to teach bad coding habits becasue they're easier, and then fix the bad habits when the student gets more advanced, that's bollox. It's just as easy to teach things the right way first.

GolamMostafa:
the OP might just get lost in the nested if-else structures.

Only if you're a bad teacher.

post the discrete level non-blocking codes [singular]

Maybe at the weekend: I've been at my desk since 0300gmt and it's 1500 now, so I'm a bit tired right now. And tomorrow's more of the same, but hey, it pays the rent.

Debate will continue without any concrete decisions. I am talking from field (both Engineering and Academic). There is none such as good or bad teacher; the teacher applies his all apparent true/false (right/wrong) tricks just to keep the pupils in the class; at the end he makes few thought provoking/inspiring quizzes/questions and it is done.

AE says that a wise man starts with a simple thing and then gradually moves to the complex one. (He has not said to start with a right thing?) What is a simple thing? Is it a basic thing? Is it an elementary thing?

We (the non-natives) use "testicles(আন্ডা)" and not "bollox" to win.

Perehama:
Mate, [...]

:slight_smile: