Facing problem in Arduino code.

Hey everyone
I am kind of new in this forum,please bear with me…lol
So here’s the thing
I got one potentiometer connected to A2 of arduino uno and I have mapped down the values of pot from0-1023 to 0-100 and for every set of readings, I have designed a set pattern of led’s to turn on and off.
The problem is when the pot reading falls into one set of pattern and exceutes that pattern first
But the application of my project is that even if user changes the pot position the arduino should leave the first execution and start executing the corresponding current reading pattern.
I tried using software interrupts but failed terribly…lol
I am posting my code below
Please help me figure out this one
Thank you!

[code]
#include<TimerOne.h>



int potPin = A2;
int potValue; // variable used to store the value coming from the sensor
int percent; // variable used to store the percentage value
int led1 = 6; // red LED connected to digital PIN 13
int led2 = 7; // red LED connected to digital PIN 12
int led3 = 8;
int led4 = 11;
int flag;
long tmr;



void setup() {
  pinMode(led1, OUTPUT); // LED is declared as an output
  pinMode(led2, OUTPUT);
  pinMode(led3, OUTPUT);
  pinMode(led4, OUTPUT);
  Timer1.initialize(20000);
  Timer1.attachInterrupt(readPot);

  Serial.begin(9600);

}

void readPot()
{

  potValue = analogRead(potPin); // read the value from the potentiometer and assign the name potValue
  percent = map(potValue, 0, 1023, 0, 100); // convert potentiometer reading to a percentage
  Serial.print(", Percentage: "); // print out the percentage
  Serial.print(percent);
  Serial.println("%");
  flag = 1;
}


void loop() {

  if (flag == 1);

  if (percent <= 1) {
    digitalWrite(led1, HIGH);
    digitalWrite(led2, LOW);
    digitalWrite(led3, LOW);
    delay(40000);
    digitalWrite(led1, LOW);
    digitalWrite(led2, HIGH);
    digitalWrite(led3, LOW);
    delay(40000);
    digitalWrite(led1, LOW);
    digitalWrite(led2, LOW);
    digitalWrite(led3, HIGH);
    delay(40000);
    digitalWrite(led2, LOW);
    digitalWrite(led3, LOW);
  }

  else if (percent > 5 && percent <= 35) {
    digitalWrite(led1, HIGH);
    digitalWrite(led2, LOW);
    digitalWrite(led3, LOW);
    delay(35000);
    digitalWrite(led1, LOW);
    digitalWrite(led2, HIGH);
    digitalWrite(led3, LOW);
    delay(35000);
    digitalWrite(led1, LOW);
    digitalWrite(led2, LOW);
    digitalWrite(led3, HIGH);
    delay(35000);
    digitalWrite(led2, LOW);
  }

  else if (percent > 36 && percent <= 55) {
    digitalWrite(led1, HIGH);
    digitalWrite(led2, LOW);
    digitalWrite(led3, LOW);
    delay(15000);
    digitalWrite(led1, LOW);
    digitalWrite(led2, HIGH);
    digitalWrite(led3, LOW);
    delay(15000);
    digitalWrite(led1, LOW);
    digitalWrite(led2, LOW);
    digitalWrite(led3, HIGH);
    delay(15000);
    digitalWrite(led2, LOW);
  }

  else if (percent > 56 && percent <= 85) {
    digitalWrite(led1, HIGH);
    digitalWrite(led2, LOW);
    digitalWrite(led3, LOW);
    delay(1000);
    digitalWrite(led1, LOW);
    digitalWrite(led2, HIGH);
    digitalWrite(led3, LOW);
    delay(1000);
    digitalWrite(led1, LOW);
    digitalWrite(led2, LOW);
    digitalWrite(led3, HIGH);
    delay(1000);
    digitalWrite(led2, LOW);
    digitalWrite(led3, LOW);
  }

  else if (percent > 86 && percent <= 100) {
    digitalWrite(led1, HIGH);
    digitalWrite(led2, LOW);
    digitalWrite(led3, LOW);
    delay(100);
    digitalWrite(led1, LOW);
    digitalWrite(led2, HIGH);
    digitalWrite(led3, LOW);
    delay(100);
    digitalWrite(led1, LOW);
    digitalWrite(led2, LOW);
    digitalWrite(led3, HIGH);
    delay(100);
    digitalWrite(led2, LOW);
    digitalWrite(led3, LOW);
  }
}

[/code]

Plesse use the Autoformat function in the IDE. Then copy the code, use cide tags, and paste the code.

Skip the delays. Look at millis(), the topic "do several things at the same time".

Tell us what this line of code does:

delay(40000);

OK, first things first.

You need to go and read the forum instructions so that you can go back and modify your original post (not re-post it) - using the “More → Modify” option below the right hand corner of your post - to mark up your code as such using the “</>” icon in the posting window. Just highlight each section of code (or output if you later need to post that) from the IDE and click the icon.

In fact, the IDE itself has a “copy for forum” link to put these markings on a highlighted block for you so you then just paste it here in a posting window. But even before doing that, don’t forget to use the “Auto-Format” (Ctrl-T) option first to make it easy to read. While you were lucky so far, If you do not post it as “code” it can easily be quite garbled and is generally more difficult to read due to the font.

It is inappropriate to attach it as a “.ino” file unless it is clearly too long to include in the post proper. People can usually see the mistakes directly and do not want to have to actually load it in their own IDE. And even that would also assume they are using a PC and have the IDE running on that PC.

Also tidy up your blank space. Do use blank lines, but only single blanks between complete functional blocks.

Why do we think this is more important than just having your question answered? Because it is really difficult to write code properly - as with any other task requiring care - if everything is not well organised!

Also, please edit your first post to a meaningful topic heading.

I think you have to mark 'flag' and 'percent' as "volatile" so the compiler knows that they may be changed with an interrupt.

volatile int flag = 0;
volatile int percent;

It would be a good idea to move:

  Serial.print(", Percentage: "); // print out the percentage
  Serial.print(percent);
  Serial.println("%");

out of 'readPot()' and put that code inside the "if (flag == 1)" statement in loop(). That way you will get the message when the input is about to be used, rather than each time the pot is sampled.

Whoa!

Didn't notice that! Using interrupts? That's seriously warped! :roll_eyes:

larryd:
Tell us what this line of code does:

delay(40000);

delay for 40 seconds

johnwasser:
I think you have to mark 'flag' and 'percent' as "volatile" so the compiler knows that they may be changed with an interrupt.

volatile int flag = 0;

volatile int percent;



It would be a good idea to move:


Serial.print(", Percentage: "); // print out the percentage
 Serial.print(percent);
 Serial.println("%");



out of 'readPot()' and put that code inside the "if (flag == 1)" statement in loop(). That way you will get the message when the input is about to be used, rather than each time the pot is sampled.

moved the serial print to the loop
still the interrupt monitors the pot readings for 20ms and then enters the loop but the arduino keeps on executing the corresponding pattern completely and then moves to collect the next reading.
Oh God! this is so frustrating and irritating at times!

delay for 40 seconds

Exactly. That means do nothing for 40 seconds and that includes not stopping the delay() and moving on to something else

Take a look at Using millis() for timing. A beginners guide, Several things at the same time and the BlinkWithoutDelay example in the IDE

UKHeliBob:
Exactly. That means do nothing for 40 seconds and that includes not stopping the delay() and moving on to something else

Take a look at Using millis() for timing. A beginners guide, Several things at the same time and the BlinkWithoutDelay example in the IDE

so using millis function
i toggle led for 40 seconds each and at the same time if I change my pot readings it will read this new pot value and jump to ececute the correspong pattern of that reading???

so using millis function
i toggle led for 40 seconds each and at the same time if I change my pot readings it will read this new pot value and jump to ececute the correspong pattern of that reading???

Yes, as long as you use millis() properly. It is not a drop in replacement for delay()

UKHeliBob:
Yes, as long as you use millis() properly. It is not a drop in replacement for delay()

I tried using millis to avoid of 40 seconds
i am lil bit of lost over here..
mind helping me with the code
I

So you read all of the references given?

Paul__B:
So you read all of the references given?

Yes and Thank You!

To start with, the current sketch has a fatal flaw

 if (flag == 1);

The only code dependant on flag being equal to 1 is the semicolon on the end of the line so basically the value of flag is ignored

In order to use millis() for timing to meet your requirements you need to restructure your sketch

Your sketch has a number of states, primarily based on the pot position. I count 5 of them. Then within each main state there are sub states based on time periods. I count 16 of them. This would lend itself to a structure like this

if in state 1
  in state 1.1
    light LED pattern for 40 seconds
  in state 1.2
    light LED pattern for 40 seconds
  in state 1.3
    light LED pattern for 40 seconds
  set state to 1.1
end if

if in state 2
  in state 2.1
    light LED pattern for 35 seconds
  in state 2.2
    light LED pattern for 35 seconds
  in state 2.3
    light LED pattern for 35 seconds
  set state to 2.1
end if

etc, etc

The timing of such a state machine can be done using millis(). For instance, stay in state 1.1 until 40 seconds has elapsed by letting loop() run freely and determining when the period ends by subtracting the start time from the current value of millis() then save the new start time and move to state 1.2

Because loop() is running freely you can read the pot frequently in loop() and change state based on its value. In practice using a float for the current state number is not a good idea and you would be better off using state numbers like 11, 12, 13, 21 etc

UKHeliBob:
To start with, the current sketch has a fatal flaw

 if (flag == 1);

The only code dependant on flag being equal to 1 is the semicolon on the end of the line so basically the value of flag is ignored

In order to use millis() for timing to meet your requirements you need to restructure your sketch

Your sketch has a number of states, primarily based on the pot position. I count 5 of them. Then within each main state there are sub states based on time periods. I count 16 of them. This would lend itself to a structure like this

if in state 1

in state 1.1
   light LED pattern for 40 seconds
 in state 1.2
   light LED pattern for 40 seconds
 in state 1.3
   light LED pattern for 40 seconds
 set state to 1.1
end if

if in state 2
 in state 2.1
   light LED pattern for 35 seconds
 in state 2.2
   light LED pattern for 35 seconds
 in state 2.3
   light LED pattern for 35 seconds
 set state to 2.1
end if

etc, etc



The timing of such a state machine can be done using millis(). For instance, stay in state 1.1 until 40 seconds has elapsed by letting loop() run freely and determining when the period ends by subtracting the start time from the current value of millis() then save the new start time and move to state 1.2

Because loop() is running freely you can read the pot frequently in loop() and change state based on its value. In practice using a float for the current state number is not a good idea and you would be better off using state numbers like 11, 12, 13, 21 etc

Thank you tho!
So I understood the flag statement mistake...and hence removed that error
Basically you are saying to use millis function for each substate???

It appears to me like the chase pattern is the same for each ‘percent’ range and all you are changing is the speed of the chase. At 0% or 1% it’s 40 seconds per step and at 87% to 100% its 0.1 second per step.

Do you really need to break the range into chunks? Perhaps a linear mapping would work:

unsigned long interval = map(analogRead(PotPin), 0, 1023, 40000ul, 100ul);

Then you just need to use the current interval to control how often the chase pattern steps:

const byte PotPin = A2;


const byte LED1Pin = 6; // red LED
const byte LED2Pin = 7; // red LED
const byte LED3Pin = 8;
const byte LED4Pin = 11;


unsigned long ChaseInterval = 100;
unsigned long LastStepTime = 0;
int ChaseStep = 0;
const int ChaseSteps = 4;


void setup()
{
  Serial.begin(115200);
  while (!Serial && millis() < 5000) {}


  pinMode(LED1Pin, OUTPUT);
  pinMode(LED2Pin, OUTPUT);
  pinMode(LED3Pin, OUTPUT);
  pinMode(LED4Pin, OUTPUT);
}


void loop()
{
  unsigned long currentMillis = millis();


  ChaseInterval = map(analogRead(PotPin), 0, 1023, 40000ul, 100ul);


  // Is it time to take the next step?
  if (currentMillis - LastStepTime >= ChaseInterval)
  {
    LastStepTime = currentMillis;
    ChaseStep++;


    // Wrap Around
    if (ChaseStep >= ChaseSteps)
      ChaseStep = 0;


    switch (ChaseStep)
    {
      case 0:
        digitalWrite(LED1Pin, HIGH);
        digitalWrite(LED2Pin, LOW);
        digitalWrite(LED3Pin, LOW);
        break;


      case 1:
        digitalWrite(LED1Pin, LOW);
        digitalWrite(LED2Pin, HIGH);
        digitalWrite(LED3Pin, LOW);
        break;


      case 2:
        digitalWrite(LED1Pin, LOW);
        digitalWrite(LED2Pin, LOW);
        digitalWrite(LED3Pin, HIGH);
        break;


      case 3:
        digitalWrite(LED1Pin, LOW);
        digitalWrite(LED2Pin, LOW);
        digitalWrite(LED3Pin, LOW);
        break;
    }
  }
}

It appears to me like the chase pattern is the same for each ‘percent’ range

I thought that too, and it would make the code simpler, but looking closely I see

  else if (percent > 36 && percent <= 55)
  {
    digitalWrite(led1, HIGH);
    digitalWrite(led2, LOW);
    digitalWrite(led3, LOW);
    delay(15000);
    digitalWrite(led1, LOW);
    digitalWrite(led2, HIGH);
    digitalWrite(led3, LOW);
    delay(15000);
    digitalWrite(led1, LOW);
    digitalWrite(led2, LOW);
    digitalWrite(led3, HIGH);
    delay(15000);
    digitalWrite(led2, LOW);
  }
  else if (percent > 56 && percent <= 85)
  {
    digitalWrite(led1, HIGH);
    digitalWrite(led2, LOW);
    digitalWrite(led3, LOW);
    delay(1000);
    digitalWrite(led1, LOW);
    digitalWrite(led2, HIGH);
    digitalWrite(led3, LOW);
    delay(1000);
    digitalWrite(led1, LOW);
    digitalWrite(led2, LOW);
    digitalWrite(led3, HIGH);
    delay(1000);
    digitalWrite(led2, LOW);
    digitalWrite(led3, LOW);
  }

The difference may, of course, be a mistake in the code

Basically you are saying to use millis function for each substate???

Yes

johnwasser:
It appears to me like the chase pattern is the same for each ‘percent’ range and all you are changing is the speed of the chase. At 0% or 1% it’s 40 seconds per step and at 87% to 100% its 0.1 second per step.

Do you really need to break the range into chunks? Perhaps a linear mapping would work:

unsigned long interval = map(analogRead(PotPin), 0, 1023, 40000ul, 100ul);

Then you just need to use the current interval to control how often the chase pattern steps:

const byte PotPin = A2;

const byte LED1Pin = 6; // red LED
const byte LED2Pin = 7; // red LED
const byte LED3Pin = 8;
const byte LED4Pin = 11;

unsigned long ChaseInterval = 100;
unsigned long LastStepTime = 0;
int ChaseStep = 0;
const int ChaseSteps = 4;

void setup()
{
 Serial.begin(115200);
 while (!Serial && millis() < 5000) {}

pinMode(LED1Pin, OUTPUT);
 pinMode(LED2Pin, OUTPUT);
 pinMode(LED3Pin, OUTPUT);
 pinMode(LED4Pin, OUTPUT);
}

void loop()
{
 unsigned long currentMillis = millis();

ChaseInterval = map(analogRead(PotPin), 0, 1023, 40000ul, 100ul);

// Is it time to take the next step?
 if (currentMillis - LastStepTime >= ChaseInterval)
 {
   LastStepTime = currentMillis;
   ChaseStep++;

// Wrap Around
   if (ChaseStep >= ChaseSteps)
     ChaseStep = 0;

switch (ChaseStep)
   {
     case 0:
       digitalWrite(LED1Pin, HIGH);
       digitalWrite(LED2Pin, LOW);
       digitalWrite(LED3Pin, LOW);
       break;

case 1:
       digitalWrite(LED1Pin, LOW);
       digitalWrite(LED2Pin, HIGH);
       digitalWrite(LED3Pin, LOW);
       break;

case 2:
       digitalWrite(LED1Pin, LOW);
       digitalWrite(LED2Pin, LOW);
       digitalWrite(LED3Pin, HIGH);
       break;

case 3:
       digitalWrite(LED1Pin, LOW);
       digitalWrite(LED2Pin, LOW);
       digitalWrite(LED3Pin, LOW);
       break;
   }
 }
}

Thank you so much…!
It was difficult to understand this code at first but it worked out very well for me and it works properly for me…
just need explanation for this instruction

while (!Serial && millis() < 5000) {}