Pages: 1 [2]   Go Down
Author Topic: An error...  (Read 866 times)
0 Members and 1 Guest are viewing this topic.
Gosport, UK
Offline Offline
Faraday Member
**
Karma: 19
Posts: 3114
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

This compiles:
Code:
//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.
Logged

Seaford, Victoria, Australia
Offline Offline
Jr. Member
**
Karma: 0
Posts: 51
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

This compiles:
Code:
//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?
Logged

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

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

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

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.
Logged

Seaford, Victoria, Australia
Offline Offline
Jr. Member
**
Karma: 0
Posts: 51
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Regards.
Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 452
Posts: 18694
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Code:
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:
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:
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:
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.
Logged

NYC
Offline Offline
Full Member
***
Karma: 0
Posts: 125
The singularity is near!
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

You fixed it, nice job!
Logged

Seaford, Victoria, Australia
Offline Offline
Jr. Member
**
Karma: 0
Posts: 51
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

Pages: 1 [2]   Go Up
Jump to: