Go Down

Topic: Program doesn't loop with Serial using switch and case (Read 681 times) previous topic - next topic

SpaceOne

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: [Select]
#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;
}

marco_c

Code: [Select]
          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?
Arduino libraries http://arduinocode.codeplex.com
Parola for Arduino http://parola.codeplex.com

SpaceOne

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


marco_c

And if you send 1/13/44/ nothing happens? Even if you send it as the first message?
Arduino libraries http://arduinocode.codeplex.com
Parola for Arduino http://parola.codeplex.com

SpaceOne

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

marco_c

#5
Oct 22, 2012, 12:20 pm Last Edit: Oct 22, 2012, 12:36 pm by marco_c Reason: 1
Code: [Select]
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.
Arduino libraries http://arduinocode.codeplex.com
Parola for Arduino http://parola.codeplex.com

PaulS

Code: [Select]
      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.

AWOL

If getSerial returns a value, why not use it, instead of assigning the return value to a global?
"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.

marco_c

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
Arduino libraries http://arduinocode.codeplex.com
Parola for Arduino http://parola.codeplex.com

PaulS

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.

SpaceOne

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: [Select]
#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

Go Up