I got control flow issues

For back ground reference you can check out the main project thread here:

Ok so lets get right into it, the project is a automotive glow plug relay controller for a diesel engine. I have built the hardware and I have used a dumb AI bot to help build out the boiler plate code. Now I have been able to clean up quite abit of it and it works for the most part, but I am not a programmer and I am at my limit of knowledge. I need help, but not entirely clueless.

Inputs:
Coolant Temp Sensor (Bosch 026 NTC)
Starter Input, 12v from ignition key switch. Signals engine cranking
Alternator Input, 12v from L terminal. Once engine starts, this goes live 12v.

Output:
External Relay Coil, Negative Ground Signaling from IRL B8721P N Channel Mosfet

Microcontroller:
Adafruit Qt Py with samd21 chipset

For those that actually would like to know how this system works I have written a 2 page overview of operations. I have also included the factory Mitsubishi Glow Plug Operations manual that the overview was based on. This controller is in fact an attempt at replicating the Mitsubishi Glow Plug Controller.

Controller_Overview.pdf (29.8 KB)
Mitsu_4M40_GlowPlug_Ops.pdf (157.4 KB)

My problem with the code is at the afterglowMode, it will turn on the relay, but then instead of checking the conditions, it bypasses that and returns to the beginning of the loop.

This is how it should be working:
Once it enters afterglowMode it should start the millis() timer and query the temp sensor and constantly check for either temperature to go above 140*F OR reach 3 minutes. Once either one of these conditions is met, it will turn off the relay and end operations.

Like I said it is working in the fact that it is turning the relay on and turning it off based on other conditions in the loop. When I first started the relay was not coming on or it would come on and simply just stay on forever so I have made some improvements to the code.

Thank YOU so much if you have read this far and are able to assist me!

#include <Arduino.h>

const int temperaturePin = A0;
const int alternatorPin = A1;
const int starterPin = A2;
const int relayControlPin = 6;

int relayOnTime = 0; // Variable to hold the relay "on" time

bool controlSequenceComplete = false; // Flag to indicate if the control sequence is complete
bool afterglowMode = false; // Flag to indicate if the control sequence is in afterglow mode

unsigned long afterglowStartTime = 0; // Start time of afterglow mode
const unsigned long afterglowDuration = 180000; // Duration of afterglow mode in milliseconds (3 Min)


void setup() {
  pinMode(relayControlPin, OUTPUT);
  pinMode(starterPin, INPUT_PULLUP);
  pinMode(alternatorPin, INPUT_PULLUP);
  // Additional setup code if needed
}

void loop() {
  if (!controlSequenceComplete) {
    float voltage = calculateVoltage(analogRead(temperaturePin));
  
    // Check if voltage is within valid range for temperature sensor
    if (voltage >= 0.0 && voltage <= 3.2) {
      // Switch case statement based on voltage ranges
      switch (int(voltage * 100)) {
        case 282 ... 320:
          relayOnTime = 24;
          break;
        case 208 ... 281:
          relayOnTime = 16;
          break;
        case 123 ... 207:
          relayOnTime = 10;
          break;
        default:
          relayOnTime = 0;
          break;
      }
    
      // Activate the relay circuit with the calculated relayOnTime
      digitalWrite(relayControlPin, HIGH);
      delay(relayOnTime * 1000);
      digitalWrite(relayControlPin, LOW);

      if (digitalRead(starterPin) == HIGH) {
      // Starter signal detected during the initial operation
      // Override and cancel the operation
      controlSequenceComplete = true;
      } else {
        // Control sequence completed successfully
        controlSequenceComplete = true;
      }
    } else {
      // Failsafe warning signal, if we lose temp sensor R2 then voltage 
      // should go to 3.3, anything above 3.2v(-30C(-22F) will trigger.
      for (int i = 0; i < 5; i++) {
        digitalWrite(relayControlPin, HIGH);
        delay(1000);
        digitalWrite(relayControlPin, LOW);
        delay(2000);
      }
      digitalWrite(relayControlPin, HIGH);
      delay(10000);
      digitalWrite(relayControlPin, LOW);
      controlSequenceComplete = true;
    }
   
  } else {
    if (digitalRead(starterPin) == HIGH) {
      // Starter signal detected, activate the relay control pin
      digitalWrite(relayControlPin, HIGH);
    } else {
      digitalWrite(relayControlPin, LOW); // Deactivate the relay circuit
      // Starter signal went low, stop the control sequence
      controlSequenceComplete = true; // Set control sequence complete flag
    }
  }

  if ((!afterglowMode) && (digitalRead(alternatorPin)) == HIGH) {
  // Alternator signal detected, engine has started
  // Enter afterglow mode
    afterglowMode = true; 
    digitalWrite(relayControlPin, HIGH); // Activate the relay control pin
    unsigned long afterglowCurrentTime = millis(); // Start afterglow mode timer
    float voltage = calculateVoltage(analogRead(temperaturePin)); 
    
  // Check if afterglow mode should continue
  if ((voltage <= 1.23) || (afterglowCurrentTime - afterglowStartTime >= afterglowDuration)) {
    // Coolant temperature reached 60°C or three minutes have elapsed
    digitalWrite(relayControlPin, LOW); // De-activate the relay circuit
    afterglowMode = false; // Exit afterglow mode
    controlSequenceComplete = true; // Set control sequence complete flag
  } else {
    controlSequenceComplete = false; // Set control sequence not complete flag
  }
}
}
float calculateVoltage(int sensorValue) {
  float voltage = sensorValue * (3.3 / 1024.0); // Convert the analog value to voltage
  return voltage;
}


This requirement cries for using a state-machine.
A state-machine is a concept build with the switch-case-break; - structure which does mutually exclusive execute multiple parts of code each part of the code executing mutual exclusive.

This gives an easy to follow code-structure with a minimised number of if-conditions and flag-variables can be avoided most of the time.

I'm not willing to transform or to modify AI-generated code. It will take me twice the time to analyse the code than to write it new.

My recommendation is that you learn how state-machines work.

You can imagine these states as modes of operation
You have a certain sequence of steps that should run through.
Which means you start with a state 1

  1. switch-on-glow-plugs and switch on dashboard-control LED change to state 2
  2. wait predetermined time. If time has passed by turn switch off glow-plugs change to state 3
    etc.

here is a tutorial how state-machines work

best regards Stefan

this is too wordy

and your asking us to debug a solution which you believe is appropriate, without explaining what the purpose of the code is

i'm guessing the code it trying to control glow plug temperature on a diesel engine by controlling the duty-cycle of a relay energizing the plugs

i would isolate the control of the relay to one part of the code that is executed independent of the rest of the code. that section would cycle the relay on based on a on time and an off time and would only execute if the on time is non-zero

the glow plug temperature (not voltage) would be determined in just one place and used as needed.

other parts of the code would read the temperature and any other conditions, to determine the on time

i don't understand afterglowMode. looks like it's initialized to false like controlSequenceComplete so both the cases are executed, at least initially. but once afterglowMode is set true that part of the code will never execute again, requiring the processor be reset

i believe the code can be much simpler by reducing the redundant code controlling the relay and restructuring the controlling parts

it would help if you could explain more clearly "what" the code needs to do rather that "how" you think you code works

1 Like

Thank You Stefan,

Interesting take on the situation. This will require some research on my part to understand the mechanics of it. I have seen simple pedestrian/traffic signal examples of this in the past. I will have to read the link you posted when I get a moment from work.

You input is appreciated, Thanks,
Robert

You can do it section by section in the setup() part of your code. loop would be void loop{}; When it finishes setup it will stall at loop and do nothing.

trying to understand.

looks like the following code is executed repeatedly and turns the relay on for a varying amount of time. there are numerous places in the code where this happens repeatedly ... hence duty cycle == period on / (period on + period off)

i don't see delay() for the off period, so it looks like it turns it on, delays, before turning it off, but since loop() repeats immediately, the relay is turned on again

i said duty cycle because it looks like the relay is turned on and off for different periods based on temperure

please explain "what" each mode is intended to accomplish

I tried to condense down the description to the keywords of conditions and parts that shall be controlled:
Driver turns ignition key to position "pre-glowing" => microcontroller is powered up

immidiately
code switches ON

  • relay that gives power to the glow-plugs
  • dashboard signal light

both relay and dashboard-signal-light stay switched on for a pre-programmed time

if pre-programmed time has passed by switch OFF

  • glow-plugs-relay
  • dashboard signal light

driver turns ignition-key to "start motor-position" =>

  • glow-plugs-relay is switched ON again

if driver releases the ignition-key so the ignition-key turns back to mode
"power is on / engine running"

  • glow-plugs-relay is switched off

if starting the engine was successful the alternator creates a 12V signal
if the code detects this 12V-signal this initiates the after-glow-mode =>
glow-plugs-relay is switched ON and observing time and engine temperature starts

condition 1: time is checked have 180 seconds passed by?
condition 2: engine-temperature is checked temperature above 140°F?

if condition 1 becomes true or condition 2 becames true =>
glow-plugs-relay is switched OFF

microcontroller goes into standby-mode -doing absolutely nothing

You should read my description carefully if this is correct.
especially switchingthe glow-plugs.relay off and on again is something that might should be different
It makes sense to not keep the glow-plug-relay switched on for a long time
switching it on again if engine did really start makes sense too.

best regards Stefan

can describe the section of you code that correspond to each of the 3 modes: Pre Glow, Start Glow and After Glow

and what you expect that code to do

i'm trying to reverse engineer what you're trying to do.

this is what i see

  • both controlSequenceComplete and afterglowMode are false allowing both cases to execute

  • the ! controlSequenceComplete case executes and for each possible sub-condition, controlSequenceComplete is set true, preventing further execution of this case

  • the ! afterglowMode case executes when the alternatorPin goes HIGH.

  • afterglowMode is set true by default, preventing further execution of this case

  • it is reset to false is voltage < 1.2

  • there a timer that may also reset afterglowMode to false , allowing further execution of this case, but if the case isn't enabled, the timer won't be executed (???)

  • otherwise controlSequence is set to false allowing the controlSequenceComplete case to execute once before it disables itself

i'm not sure what the code is expected to do.

!controlSequenceComplete: voltage 1.42
 !controlSequenceComplete - voltage
    set relay Control Pin 
 controlSequenceComplete = 1
controlSequenceComplete:
controlSequenceComplete:
controlSequenceComplete:
.
.
.
controlSequenceComplete:
controlSequenceComplete:
controlSequenceComplete:
controlSequenceComplete:
controlSequenceComplete:   << alternatorPin goes HIGH
!afterglowMode:
 !afterglowMode:
 controlSequenceComplete = 0
!controlSequenceComplete: voltage 1.42
 !controlSequenceComplete - voltage
    set relay Control Pin 
 controlSequenceComplete = 1
controlSequenceComplete:
controlSequenceComplete:
controlSequenceComplete:

#include <Arduino.h>

const int temperaturePin = A0;
const int alternatorPin = A1;
const int starterPin = A2;
const int relayControlPin = 6;

int relayOnTime = 0; // Variable to hold the relay "on" time

bool controlSequenceComplete = false; // Flag to indicate if the control sequence is complete
bool afterglowMode = false; // Flag to indicate if the control sequence is in afterglow mode

unsigned long afterglowStartTime = 0; // Start time of afterglow mode
const unsigned long afterglowDuration = 180000; // Duration of afterglow mode in milliseconds (3 Min)

void setup() {
    Serial.begin (9600);
    pinMode(relayControlPin, OUTPUT);
    pinMode(starterPin, INPUT_PULLUP);
    pinMode(alternatorPin, INPUT_PULLUP);
    // Additional setup code if needed
}


void loop()
{
    if (!controlSequenceComplete) {
        float voltage = calculateVoltage(analogRead(temperaturePin));
        Serial.print   ("!controlSequenceComplete: voltage ");
        Serial.println (voltage);

        // Check if voltage is within valid range for temperature sensor
        if (voltage >= 0.0 && voltage <= 3.2) {
            Serial.println (" !controlSequenceComplete - voltage");
            // Switch case statement based on voltage ranges
            switch (int(voltage * 100)) {
            case 282 ... 320:
                relayOnTime = 24;
                break;
            case 208 ... 281:
                relayOnTime = 16;
                break;
            case 123 ... 207:
                relayOnTime = 10;
                break;
            default:
                relayOnTime = 0;
                break;
            }

            // Activate the relay circuit with the calculated relayOnTime
            Serial.println ("    set relay Control Pin ");
            digitalWrite(relayControlPin, HIGH);
            delay(relayOnTime * 1000);
            digitalWrite(relayControlPin, LOW);

            if (digitalRead(starterPin) == HIGH) {
                // Starter signal detected during the initial operation
                // Override and cancel the operation
                controlSequenceComplete = true;
            }
            else {
                // Control sequence completed successfully
                controlSequenceComplete = true;
            }
            Serial.print   (" controlSequenceComplete = ");
            Serial.println (controlSequenceComplete);
        }
        else {
            Serial.println (" !controlSequenceComplete - no voltage");

            // Failsafe warning signal, if we lose temp sensor R2 then voltage
            // should go to 3.3, anything above 3.2v(-30C(-22F) will trigger.
            for (int i = 0; i < 5; i++) {
                digitalWrite(relayControlPin, HIGH);
                delay(1000);
                digitalWrite(relayControlPin, LOW);
                delay(2000);
            }
            digitalWrite(relayControlPin, HIGH);
            delay(10000);
            digitalWrite(relayControlPin, LOW);
            controlSequenceComplete = true;
        }

    } else {
        Serial.println ("controlSequenceComplete:");
        if (digitalRead(starterPin) == HIGH) {
            // Starter signal detected, activate the relay control pin
            digitalWrite(relayControlPin, HIGH);
        } else {
            digitalWrite(relayControlPin, LOW); // Deactivate the relay circuit
            // Starter signal went low, stop the control sequence
            controlSequenceComplete = true; // Set control sequence complete flag
        }
    }

    if ((!afterglowMode) && (digitalRead(alternatorPin)) == HIGH) {
        Serial.println ("!afterglowMode:");
        // Alternator signal detected, engine has started
        // Enter afterglow mode
        afterglowMode = true;
        digitalWrite(relayControlPin, HIGH); // Activate the relay control pin
        unsigned long afterglowCurrentTime = millis(); // Start afterglow mode timer
        float voltage = calculateVoltage(analogRead(temperaturePin));

        // Check if afterglow mode should continue
        if ((voltage <= 1.23) || (afterglowCurrentTime - afterglowStartTime >= afterglowDuration)) {
            // Coolant temperature reached 60┬░C or three minutes have elapsed
            digitalWrite(relayControlPin, LOW); // De-activate the relay circuit
            afterglowMode = false; // Exit afterglow mode
            controlSequenceComplete = true; // Set control sequence complete flag
        } else {
            controlSequenceComplete = false; // Set control sequence not complete flag
        }
        Serial.println (" !afterglowMode:");
        Serial.print   (" controlSequenceComplete = ");
        Serial.println (controlSequenceComplete);
    }
}

float calculateVoltage(int sensorValue) {
    float voltage = sensorValue * (3.3 / 1024.0); // Convert the analog value to voltage
    return voltage;
}

as @StefanL38 suggested, i would expect to see processing for the 3 mode: Pre Glow, Start Glow and After Glow, but code for each mode executed only while in that mode and tests to determine when to advance to the next mode and finally to a 4th End mode.

it looks like the controlSequenceComplete case is turning on/off the relay depending on the voltage/temperature reading. it seems this code always disables itself, but the !afterglowMode case reenables but !afterglowMode case also disables itself

please help me understand what you're trying to do? you can start with one mode, what it needs to do and what are the conditions for advancing to the next mode.

Maybe so, but it would be helpful to us (and you) to specify what the controller is to do. Not in terms of the code at all, but a description of the problem in terms of the glow plug, the temperature and the driver's selection.

Please don't ask me to read your wall o' text at the beginning or explain in detail how a diesel works, just the abstract of what you need from the controller.

This is one of the few times I would suggest using delay.
Let's look at your setup:
pinMode(relayControlPin, OUTPUT);
pinMode(starterPin, INPUT_PULLUP);
pinMode(alternatorPin, INPUT_PULLUP);
Each of these is a step that will not be repeated. You can break out each of your if statements and do them in a predefined order. If it is not true you skip or wait for it. If it is true we turn on a relay for ten seconds.
relay on
Delay 10 seconds
relay off
then continue. During the delay no other code will be executed. Hopefully this gives you an idea of what I am saying. This is not good coding but it will get you there faster.

when i read these descriptions, i'm still not sure what the code needs do.

it sounds like, when the ignition switch is turned on, the preheating control at start mode is entered and in this mode, the glow plug relay is simply turned on until the temperature exceeds some threshold and the preheating control mode is entered.

the documents simply say the relay current is set according to engine coolant temperature. i'll guess this is where the code determines the relayOnTime based on temperature. (still haven't seen where the relayOffTime delay occurs).

i guess that the afterGlow control is entered when the temperature exceeds some other threshold and from the code, that the alternator is active, indicating the engine is running

i believe the notes say the system remains in afterGlow mode for a fixed amount of time and then some idle mode is entered.

i can comment on the code if this understanding is reasonable, doubtful otherwise

i takes about 4 times as many man years to figure out "what" needs to be done than "how" to do it

Almost. In some languages yes. But in C, you can declare a function before its body of code is defined, using a "function prototype". It's really a kind of forward declaration.

The Arduino "language" eliminates the need for them. Except once in a while when an update or something like that breaks things... :slight_smile:

Really? In the time that it took you to post this could have pasted the link(s).