Pages: [1]   Go Down
Author Topic: Program doesn't loop with Serial using switch and case  (Read 555 times)
0 Members and 1 Guest are viewing this topic.
Offline Offline
Newbie
*
Karma: 0
Posts: 24
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Hi

I got an Arduino Mega, the program bellow should change the state of an led according with the received data.
If I send 1/13/33/ with the Serial Monitor it turns ON the LED, but if I send 1/13/44/ it doesn't turn OFF  the LED, it gets stuck.

Code:
#include <Servo.h>

unsigned long serialdata;
int inbyte;
int pinNumber;
int digitalState;
int led = 13;
void setup()
{
  pinMode(led, OUTPUT);  
  Serial.begin(9600);

}

void loop()
{
  getSerial();
  switch(serialdata)
  {
  case 1:
    {
           getSerial();
          pinNumber = serialdata;
          getSerial();
          digitalState = serialdata;
          Serial.print(" pin number ");
          Serial.print(pinNumber);
          Serial.print(" state ");
          Serial.print(digitalState);
          Serial.print("end\n");
          
          if (digitalState==33)
          {            
            digitalWrite(led, HIGH);
          }
          if (digitalState==44)
          {
            digitalWrite(led, LOW);
          }
          break;
        }
        
     }
    }
  
long getSerial()
{
Serial.println(1);
  serialdata = 0;

 while (inbyte != '/')
  {
    inbyte = Serial.read();
 
    if (inbyte > 0 && inbyte != '/')
    {
    
      serialdata = serialdata * 10 + inbyte - '0';
    }
  }
  inbyte = 0;
  return serialdata;
}
Logged

Sydney, Australia
Offline Offline
Edison Member
*
Karma: 33
Posts: 1257
Big things come in large packages
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
          Serial.print(" pin number ");
          Serial.print(pinNumber);
          Serial.print(" state ");
          Serial.print(digitalState);
          Serial.print("end\n");
What is your debug printout telling you?

And why are you using 'led' and not 'pinnumber' in your digital output?
Logged

Arduino libraries http://arduinocode.codeplex.com
Parola hardware & library http://parola.codeplex.com

Offline Offline
Newbie
*
Karma: 0
Posts: 24
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

The result with led or the real pin number 13 is the same, I am using "pinNumber" to see the send data.

The print sends good data: "pin number 13 state 33 end", only once.
After that only "1" that is generated by getSerial() when the program requests the function

Logged

Sydney, Australia
Offline Offline
Edison Member
*
Karma: 33
Posts: 1257
Big things come in large packages
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

And if you send 1/13/44/ nothing happens? Even if you send it as the first message?
Logged

Arduino libraries http://arduinocode.codeplex.com
Parola hardware & library http://parola.codeplex.com

Offline Offline
Newbie
*
Karma: 0
Posts: 24
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

The first command 1/13/33/ gets executed but the next commands it doesn't.
If I send 1/13/44/ to turn off the led, nothing happens only "1" is printed, the pin number and the state don't get printed and the led doesn't turn off
Logged

Sydney, Australia
Offline Offline
Edison Member
*
Karma: 33
Posts: 1257
Big things come in large packages
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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

void loop()
{
  int pinNumber;
  int digitalState;

  switch(getSerial())
  {
  case 1:
    {
      pinNumber = getSerial();
      digitalState = getSerial();
      Serial.print("Pin: ");
      Serial.print(pinNumber);
      Serial.print("; S: ");
      Serial.println(digitalState);

      pinMode(pinNumber, OUTPUT);
      switch (digitalState)
      {
        case 33:
          digitalWrite(pinNumber, HIGH);
          break;
         
        case 44:
          digitalWrite(pinNumber, LOW);
          break;
      }
      break;
    }
  }
}

int getSerial()
{
  int d = 0;
  int b;

  while (b != '/')
  {
    b = Serial.read();
   
    if (b == -1)  // no char available
    {
      delay(1);  // just long enough to get a char in?
    }
    else if ((b >= '0') && (b <= '9'))
    {
      d = (d * 10) + (b - '0');
    }
  }

  return d;
}

This works for me. The problem was in the getSerial() function - your tests for validity of the character received were not correct:
- What is returned by the Serial.read() when there are no characters waiting?
- You need to check if the characters read was between '0' and '9' or the formula will not work.

I have also restructured to code to be in line with more standard programming practices.

What is also clear is that if you want to be able to specify the pin number you need to set the mode to output when you know the pin number from the serial stream, not just in setup(), unless you are restricting the pin number to a few pins.
« Last Edit: October 22, 2012, 05:36:19 am by marco_c » Logged

Arduino libraries http://arduinocode.codeplex.com
Parola hardware & library http://parola.codeplex.com

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 601
Posts: 48543
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Code:
      delay(1);  // just long enough to get a char in?
No. This is useless, time wasting activity.

First, Serial.available() should be called in getSerial(), and no attempt to read data should happen unless there is something to read.

Second, getSerial() should return as soon as there is no (more) data to read. It should return or set a flag indicating whether there was a complete packet read.

The code that calls getSerial() should do nothing until that flag/return value is true.
Logged

Global Moderator
UK
Offline Offline
Brattain Member
*****
Karma: 290
Posts: 25795
I don't think you connected the grounds, Dave.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

If getSerial returns a value, why not use it, instead of assigning the return value to a global?
Logged

"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

Sydney, Australia
Offline Offline
Edison Member
*
Karma: 33
Posts: 1257
Big things come in large packages
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Agree on what you say, PaulS, but the OP structured the code expecting it to block on getSerial() until it gets a number.

Short of changing the entire structure of the program, this is the easiest way to get OPs code working so that he can progress. What is there now will become a limitation later IF he needs to do other things
Logged

Arduino libraries http://arduinocode.codeplex.com
Parola hardware & library http://parola.codeplex.com

Seattle, WA USA
Offline Offline
Brattain Member
*****
Karma: 601
Posts: 48543
Seattle, WA USA
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Quote
Agree on what you say, PaulS, but the OP structured the code expecting it to block on getSerial() until it gets a number.

Short of changing the entire structure of the program, this is the easiest way to get OPs code working so that he can progress.
It can be done without the delay(). The delay() contributes absolutely nothing, except delay. Which isn't needed.

Does it matter whether there is no serial data to read 1 microsecond later? Find that there is nothing to read 1 time, 10 times, or 18378765 times. It doesn't matter. As soon as there IS something to read, read it. Don't keep diddling around for the rest of the 1000 microseconds.
Logged

Offline Offline
Newbie
*
Karma: 0
Posts: 24
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

marco_c thank you for the help, the program works as expected, I will use the code to control several servomotors.

For those who have the same problem here is the full code:
Code:
#include <Servo.h>

unsigned long serialdata;
int inbyte;
int pinNumber;
int digitalState;
int led = 13;
void setup()
{
  Serial.begin(9600);
}

void loop()
{
  int pinNumber;
  int digitalState;

  switch(getSerial())
  {
  case 1:
    {
      pinNumber = getSerial();
      digitalState = getSerial();
      Serial.print("Pin: ");
      Serial.print(pinNumber);
      Serial.print("; S: ");
      Serial.println(digitalState);
     
      pinMode(pinNumber, OUTPUT);
      if(digitalState==1){
        digitalWrite(pinNumber, HIGH);
          break;
      }
     
      if(digitalState==0){
        digitalWrite(pinNumber, LOW);
          break;
      }
     
      break;
    }
  }
}

int getSerial()
{
  int d = 0;
  int b;

  while (b != '/')
  {
    b = Serial.read();
   
    if (b == -1)  // no char available
    {
      delay(1);  // just long enough to get a char in?
    }
    else if ((b >= '0') && (b <= '9'))
    {
      d = (d * 10) + (b - '0');
    }
  }

  return d;
}

To send data use 1/13/1/
 where the first 1/ is the case 1
 13 is the pin number
 the last 1 is the digitalState
Logged

Pages: [1]   Go Up
Jump to: