lots of delays, any other way??

[system]

A friend is helping me write a plugin for a bus simulator, it captures the dashboard data (speed, rpm's, idiot lights etc) and sends it via (usb) serial to my arduino mega (1280) which works the gauges and lights in the real dashboard.

we are a little worried that we may be using too many delays, as after every serial read we need a 1millisecond delay, i know that's not much, but worried they may add up and cause problems,

Can someone look at our code and advise if there are better ways to do it please,
we have another 8 gauges to add, and a good 20 or more lights,

// Last updated: March 12th, 2012 - 7:39PM GMT

char kind_of_data;

void setup(){
  Serial.begin(115200);
  TCCR4B = (TCCR4B & 0xF8) | 0x01 ; //set timer 4 to 31Khz to silence singing coils in gauges
  pinMode(22, OUTPUT);
  pinMode(23, OUTPUT);
  pinMode(24, OUTPUT);
  pinMode(25, OUTPUT);
  pinMode(26, OUTPUT);
}

void loop()
{
  while(Serial.available() > 0)
  {
    kind_of_data = Serial.read();

    // Gauges
    if (kind_of_data == 'R' ) Read_Rpm();
    if (kind_of_data == 'o' ) Read_Oil();
    // Lights
    if (kind_of_data == 'S' ) Read_BusStopLight();
    if (kind_of_data == 'B' ) Read_Blinker();
    if (kind_of_data == 'H' ) Read_Highbeam();
    if (kind_of_data == 'b' ) Read_Battery();
    // Test routine - can be removed when necessary
    if (kind_of_data == 'I' ) digitalWrite(23,HIGH); //turns led on or off to show connection
    if (kind_of_data == 'O' ) digitalWrite(23,LOW);
   }
}

// Lights

void Read_BusStopLight(){
  delay(1);
  int haltewunsch = Serial.read() - '0';
  if (haltewunsch == 1) 
  {
  	digitalWrite(22,HIGH);
  }
  else
  {
	digitalWrite(22,LOW);
  }
}

void Read_Blinker(){
  delay(1);
  int lights_blinkgeber = Serial.read() - '0';
  if (lights_blinkgeber == 1) 
  {
  	digitalWrite(24,HIGH);
  }
  else
  {
	digitalWrite(24,LOW);
  }
}

void Read_Highbeam(){
  delay(1);
  int cockpit_fernlichthebel = Serial.read() - '0';
  if (cockpit_fernlichthebel == 1) 
  {
  	digitalWrite(25,HIGH);
  }
  else
  {
	digitalWrite(25,LOW);
  }
}

void Read_Battery(){
  delay(1);
  int battery = Serial.read() - '0';
  if (battery == 1) 
  {
  	digitalWrite(26,HIGH);
  }
  else
  {
	digitalWrite(26,LOW);
  }
}

// Gauges

void Read_Rpm(){
  delay(1);
  int rpm1000 = Serial.read()- '0';
  delay(1);
  int rpm100 = Serial.read()- '0';
  delay(1);
  int rpm10 = Serial.read()- '0';
  delay(1);
  int rpm1 = Serial.read()- '0';
  int rpm = 1000*rpm1000 + 100*rpm100 + 10*rpm10 + rpm1;
  tone(9, map(rpm,0,5700,55,423)); //50% duty variable frequency to run rev counter
}

void Read_Oil(){
  delay(1);
  int oil1 = Serial.read()- '0';
  delay(1);
  int oilm1 = Serial.read()- '0';
  int oil = 10*oil1 + oilm1;
  analogWrite(8,map(oil,0,50,0,255)); //PWM to imitate varying resistance to earth for oil gauge
}

[cyclegadget]

To understand how to remove the delays, we need to know why you put them there. You are using while(Serial.available() > 0) , so you should have serial data available but, then you delay right after that. Was the delay a result of failed tests? Have you tried run the sketch without the delay and if so, what happened?

Mark

[nickgammon]

gazz:
we are a little worried that we may be using too many delays, as after every serial read we need a 1millisecond delay, i know that's not much, but worried they may add up and cause problems,

You don't need any delays, but you do need to rework your serial reading.

Gammon Forum : Electronics : Microprocessors : How to process incoming serial...

Your 1 mS delays appear to be helping because it takes 86 uS for one byte to be sent at 115200 baud, and thus waiting 1000 uS gives it time to arrive. But this is not a good way of doing it. For one thing, a delay (like, an interrupt) at the sending end can cause it to take longer than 1 mS.

My link above suggests a couple of ways of doing it, and in your case the state machine method might be better.

[system]

I think you could just check Serial.available - with the appropriate number of characters - in each of your reading subroutines instead of delaying until the data arrives. (If every command string is the same length you could check this in loop).

Or try the state machine approach which is more elegant.

Also note that if your host is transmitting only "1" or "0", you could compress code like:

int haltewunsch = Serial.read() - '0';
  if (haltewunsch == 1) 
  {
  	digitalWrite(22,HIGH);
  }
  else
  {
	digitalWrite(22,LOW);
  }

to:

digitalWrite(22, Serial.read() - '0');

[system]

the delays seem'd to be needed or everything went haywire... flickering lights and gauge needles galore,

my friend has now re-written the code using the state machine method, so mucho thanks for the pointers (i did read your site about the state machine a few days ago Mr Gammon, it went over my head a little, but my friend had no problems with it, so our latest code is:

// Last updated: March 14th, 2012 - 21:40PM GMT
// Using state machine pattern - based on Nick Gammon code

typedef enum {  NONE, GOT_I, GOT_O, GOT_R, GOT_o, GOT_T, GOT_S, GOT_B, GOT_H, GOT_b, GOT_h } states;
states state = NONE;
unsigned int currentValue;

void setup ()
{
  Serial.begin (115200);
  TCCR4B = (TCCR4B & 0xF8) | 0x01 ;
  state = NONE;
  pinMode(22, OUTPUT);
  pinMode(23, OUTPUT);
  pinMode(24, OUTPUT);
  pinMode(25, OUTPUT);
  pinMode(26, OUTPUT);
  pinMode(27, OUTPUT);
} 

// Lights

void Read_BusStopLight(const unsigned int value)
{
  digitalWrite(22,Serial.read() - '0');
}

void Read_Blinker(const unsigned int value)
{
  digitalWrite(24,Serial.read() - '0');
}

void Read_Highbeam(const unsigned int value)
{
  digitalWrite(25,Serial.read() - '0');
}

void Read_Battery(const unsigned int value)
{
  digitalWrite(26,Serial.read() - '0');
}

void Read_Handbrake(const unsigned int value)
{
  digitalWrite(27,Serial.read() - '0');
}

// Gauges

void Read_Temperature(const unsigned int value)
{
  analogWrite(7,map(value,0,100,0,255));
}

void Read_Rpm(const unsigned int value)
{
  tone(9, map(value,0,5700,55,423));
}

void Read_Oil(const unsigned int value)
{
  analogWrite(8,map(value,0,50,0,255));
}

void handlePreviousState()
{
  switch (state)
  {
  case GOT_R:
    Read_Rpm(currentValue);
    break;
  case GOT_o:
    Read_Oil(currentValue);
    break;
  case GOT_T:
    Read_Temperature(currentValue);
    break;
  case GOT_S:
	Read_BusStopLight(currentValue);
	break;
  case GOT_B:
	Read_Blinker(currentValue);
	break;
  case GOT_H:
	Read_Highbeam(currentValue);
	break;
  case GOT_b:
	Read_Battery(currentValue);
	break;
  case GOT_h:
	Read_Handbrake(currentValue);
	break;
  case GOT_I:
	digitalWrite(23,HIGH);
	break;
  case GOT_O:
	digitalWrite(23,LOW);
	break;
  }
  currentValue = 0; 
}

void processIncomingByte (const byte c)
{
  if (isdigit (c))
  {
    currentValue *= 10;
    currentValue += c - '0';
  }
  else 
  {

    handlePreviousState ();
    switch (c)
    {
    case 'I':
      state = GOT_I;
      break;
    case 'O':
      state = GOT_O;
      break;
    case 'R':
      state = GOT_R;
      break;
    case 'o':
      state = GOT_o;
      break;
    case 'T':
      state = GOT_T;
      break;
    case 'S':
      state = GOT_S;
      break;
    case 'B':
      state = GOT_B;
      break;
    case 'H':
      state = GOT_H;
      break;
    case 'b':
      state = GOT_b;
      break;
    case 'h':
      state = GOT_h;
      break;
    default:
      state = NONE;
      break;
    }
  }  
  
}

void loop()
{
	if (Serial.available())
		processIncomingByte (Serial.read());
}