Go Down

Topic: An error... (Read 1 time) previous topic - next topic

dxw00d

This compiles:
Code: [Select]
//Remote Vehicle Ignition
//State Machine begin
#include <SoftwareSerial.h>

char inchar;

SoftwareSerial cell(2,3);

// system states
#define STOPPED   0
#define STARTING  1
#define CRANKING  2
#define RUNNING   3

#define FUEL_PUMP_PRIME_TIME  5000
#define RUNNING_TIMEOUT 60000UL

const int ignitionPin = 12;
const int onPin = 11;
const int ledPin = 13;
const int onledPin = 9;
const int alternatorPin = 4;

int state = STOPPED;        // master variable for the state machine
unsigned long StartingTime; // fuel pump priming
unsigned long RunningTime;  //  ten minute timeout

void setup()
{
  pinMode(onPin, OUTPUT);
  pinMode(onledPin, OUTPUT);
  pinMode(ignitionPin, OUTPUT);
  pinMode(ledPin, OUTPUT);
  pinMode(alternatorPin,INPUT);
  Stop();
  cell.begin(9600);
  delay(30000);
  cell.println("AT+CMGF=1");
  delay(200);
  cell.println("AT+CNMI=3,3,0,0");
  delay(200);
}

void loop()
{
  switch(state)
  {
  case STOPPED:
    HandleStoppedState();
    break;
  case STARTING:
    HandleStartingState();
    break;
  case CRANKING:
    HandleCrankingState();
    break; 
  case RUNNING:
    if(millis()-RunningTime > RUNNING_TIMEOUT)
    {
      Serial.println("Running Timeout"); 
      Stop(); 
    }
    break; 
  default:
    Serial.print("Unknown State: ");
    Serial.println(state);
    Stop();
    break; 
  }
}

void Stop()
{
  Serial.println("TimeOut"); 
  digitalWrite(ignitionPin,LOW);
  digitalWrite(onPin,LOW);
  digitalWrite(onledPin,LOW);
  state=STOPPED;
}

void HandleStartingState()
{
  if(millis()-StartingTime > FUEL_PUMP_PRIME_TIME)
  {
    if(digitalRead(alternatorPin)==HIGH)
    {
      Serial.println("Abort cranking"); 
      Stop();
    }
    else
    { 
      Serial.println("Starting Vehicle"); 
      digitalWrite(onPin,HIGH);
      digitalWrite(ignitionPin,HIGH);
      state=CRANKING;
    } 
  }
}

void HandleStoppedState()
{
  if(cell.available() >0)
  {
    inchar=cell.read();
    if (inchar=='#')
    {
      delay(10);
      inchar=cell.read();
      if (inchar=='s')
      {

        delay(10);
        inchar=cell.read();
        if (inchar=='0')
        {

          digitalWrite(ignitionPin,LOW);
          digitalWrite(onPin,LOW);
        }
        else if (inchar=='1')
        {
          Serial.println("Priming Fuel Pump"); 
          StartingTime=millis();
          digitalWrite(ignitionPin,LOW);
          digitalWrite(onPin,HIGH);
          state=STARTING;
        }     
      }
    }
  }
}

void HandleCrankingState()
{
  if(digitalRead(alternatorPin)==HIGH)
  {   
    Serial.println("Vehicle Running"); 
    digitalWrite(ignitionPin,LOW);
    digitalWrite(onPin,HIGH);
    digitalWrite(onledPin,HIGH);
    RunningTime=millis();
    state=RUNNING;
  }
}


But I don't have your hardware, so I don't know if it works.

Aussie_V8


This compiles:
Code: [Select]
//Remote Vehicle Ignition
//State Machine begin
#include <SoftwareSerial.h>

char inchar;

SoftwareSerial cell(2,3);

// system states
#define STOPPED   0
#define STARTING  1
#define CRANKING  2
#define RUNNING   3

#define FUEL_PUMP_PRIME_TIME  5000
#define RUNNING_TIMEOUT 60000UL

const int ignitionPin = 12;
const int onPin = 11;
const int ledPin = 13;
const int onledPin = 9;
const int alternatorPin = 4;

int state = STOPPED;        // master variable for the state machine
unsigned long StartingTime; // fuel pump priming
unsigned long RunningTime;  //  ten minute timeout

void setup()
{
  pinMode(onPin, OUTPUT);
  pinMode(onledPin, OUTPUT);
  pinMode(ignitionPin, OUTPUT);
  pinMode(ledPin, OUTPUT);
  pinMode(alternatorPin,INPUT);
  Stop();
  cell.begin(9600);
  delay(30000);
  cell.println("AT+CMGF=1");
  delay(200);
  cell.println("AT+CNMI=3,3,0,0");
  delay(200);
}

void loop()
{
  switch(state)
  {
  case STOPPED:
    HandleStoppedState();
    break;
  case STARTING:
    HandleStartingState();
    break;
  case CRANKING:
    HandleCrankingState();
    break; 
  case RUNNING:
    if(millis()-RunningTime > RUNNING_TIMEOUT)
    {
      Serial.println("Running Timeout"); 
      Stop(); 
    }
    break; 
  default:
    Serial.print("Unknown State: ");
    Serial.println(state);
    Stop();
    break; 
  }
}

void Stop()
{
  Serial.println("TimeOut"); 
  digitalWrite(ignitionPin,LOW);
  digitalWrite(onPin,LOW);
  digitalWrite(onledPin,LOW);
  state=STOPPED;
}

void HandleStartingState()
{
  if(millis()-StartingTime > FUEL_PUMP_PRIME_TIME)
  {
    if(digitalRead(alternatorPin)==HIGH)
    {
      Serial.println("Abort cranking"); 
      Stop();
    }
    else
    { 
      Serial.println("Starting Vehicle"); 
      digitalWrite(onPin,HIGH);
      digitalWrite(ignitionPin,HIGH);
      state=CRANKING;
    } 
  }
}

void HandleStoppedState()
{
  if(cell.available() >0)
  {
    inchar=cell.read();
    if (inchar=='#')
    {
      delay(10);
      inchar=cell.read();
      if (inchar=='s')
      {

        delay(10);
        inchar=cell.read();
        if (inchar=='0')
        {

          digitalWrite(ignitionPin,LOW);
          digitalWrite(onPin,LOW);
        }
        else if (inchar=='1')
        {
          Serial.println("Priming Fuel Pump"); 
          StartingTime=millis();
          digitalWrite(ignitionPin,LOW);
          digitalWrite(onPin,HIGH);
          state=STARTING;
        }     
      }
    }
  }
}

void HandleCrankingState()
{
  if(digitalRead(alternatorPin)==HIGH)
  {   
    Serial.println("Vehicle Running"); 
    digitalWrite(ignitionPin,LOW);
    digitalWrite(onPin,HIGH);
    digitalWrite(onledPin,HIGH);
    RunningTime=millis();
    state=RUNNING;
  }
}


But I don't have your hardware, so I don't know if it works.


Thanks! shall test shortly.
But why are there so many braces at the end?

AWOL

Yes, paying attention to indentation and matching braces as you go along will pay dividends.
"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.

PaulS

Quote
As soon as i add this, it fails.

We're only going to tell you about 14 more times. You absolutely MUST have EXACTLY ONE { for every }. That last block of code has 9 curly braces - some open, some close. It must have an even number. Period. You can try anything you like but you can NOT get around that fact.

Aussie_V8

Thank you Paul for the sarcastic reply.
I'm fairly sure i now understand.

Regards.

Nick Gammon

You can reduce the number of braces by reversing the logic, to a certain extent, eg. instead of:

Code: [Select]
void HandleCrankingState()
{
  if(digitalRead(alternatorPin)==HIGH)
  {   
    Serial.println("Vehicle Running"); 
    digitalWrite(ignitionPin,LOW);
    digitalWrite(onPin,HIGH);
    digitalWrite(onledPin,HIGH);
    RunningTime=millis();
    state=RUNNING;
  }
}


You can do this:

Code: [Select]
void HandleCrankingState()
{
  if(digitalRead(alternatorPin)==LOW)
    return;

  Serial.println("Vehicle Running"); 
  digitalWrite(ignitionPin,LOW);
  digitalWrite(onPin,HIGH);
  digitalWrite(onledPin,HIGH);
  RunningTime=millis();
  state=RUNNING;
}


Now you don't have as many nested braces.

A good rule of thumb when coding is to always write the closing brace as you write the opening one. Eg. writing your original code:

Code: [Select]
void HandleCrankingState()
{
   <---- write the closing brace and up-arrow to here to write the function
}  // end of HandleCrankingState


The comment says why the brace is there.

Then:

Code: [Select]
void HandleCrankingState()
{
  if(digitalRead(alternatorPin)==HIGH)
  {   
      <---- write the closing brace and up-arrow to inside the "if"
  }  // if alternatorPin is HIGH
}  // end of HandleCrankingState


Get into the habit of doing that and these problems will go away.
Please post technical questions on the forum, not by personal message. Thanks!

More info:
http://www.gammon.com.au/electronics

Andy R

You fixed it, nice job!

Aussie_V8

Well, no i didn't fix it, you kind people did.
And along the way, i learnt some good lessons, cheers everyone!

It compiles, uploads and works perfect.

Go Up