While loop does not function when volatile var supposed to trigger it

While loop does not function when volatile var is supposed to trigger it.
I am triggering an interrupt which works fine and immediately set the var to a value of 1 ( I have tried bool, uint8_t, and int) as types the same problem occurs. I intend to get rid of the interrupt in the future, but I know it works when the hall effect triggers it. I tried to use an if statement in the void loop() but that did not work so I put in the while (triggerState = 1); still no love. I have a lot of Serial.print lines I used for testing (sorry) If I trigger the hall effect with this code I get the Serial.print of the value of triggerState in the ISR

code follows:

/*
   Logic program to determine rotation direction and direction pointing
   Rotation CW will be represented as  value of 16 of the uint8_t
   which will include the least significant bits 0-3 for values between
   1 and 8 only.  To indicate rotation direction if value is greater than
   8 then CW else CCW.  Values between 9 and 15 are invalid as is 0.
*/

#include <VirtualWire.h>
//#include <SPI.h>
const uint8_t trigger = 2;   // the number of the interrupt trigger pin
const uint8_t   reed1 = 3;   // Reed group pins 4-7 are BCD inputs
const uint8_t   reed2 = 4;   //
const uint8_t   reed3 = 5;   //
const uint8_t   reed4 = 6;   //
const uint8_t  rotate = 11;  // detects call to rotate antenna from base
const int ledPin = 13;
uint8_t value = 0;
uint8_t prev_value = 0;      // Sets previous direction pointing to 0
uint8_t x = 0;
uint8_t triggered = 0;
uint8_t triggerState = 0;
uint8_t sum = 0;             // calculated value of current plus previous values
uint8_t cw = 16;             // sets CW indicator to false
uint8_t TX_buffer[1] = {0};
const uint8_t NUMBER = 4;                   // Array of inputs for BCD reading
uint8_t input_pin[NUMBER] = {3, 4, 5, 6};
uint8_t position[NUMBER] = {0};
bool runonce = false;
bool mot_ena = HIGH;         // enables motor thru MOSFET

// variables will change:

//volatile uint8_t value = 0;
//volatile uint8_t triggerState = 0;

void setup()
{
  Serial.begin(115200);
  pinMode   (trigger, INPUT);
  pinMode   (rotate,  INPUT);
  pinMode   (trigger, INPUT);     // Interrupt input from Hall effect
  pinMode   (ledPin, OUTPUT);
  vw_set_tx_pin(12);              // Transmit pin base data for direction and rotation
  vw_setup(4000);                 // bps transmission speed of transmitter
  TX_buffer[0] = 0;
  
  attachInterrupt(digitalPinToInterrupt(trigger), conv, FALLING);
}

void loop()
{
/*
if ((sum == 15) || (sum == 9))      // determines if rotation went too far in either direction
  {
    digitalWrite (mot_ena, LOW);
    delay (5000);
  
    digitalWrite (mot_ena, HIGH);
  } 
*/
 Serial.print("triggerState0:   ");    // 0 printed at beginning of void
 Serial.println(triggerState);
 
 while (triggerState = 1);
 {
 Serial.print("triggerState1:   ");     // 0 printed at beginning of while loop
 Serial.println(triggerState);

 TX_buffer[0] = value;
  vw_send(TX_buffer, 1);
  vw_wait_tx();
  
  delay (2500);
  
  x=x+1;
  digitalWrite(ledPin, 1);
  Serial.println("Triggered Value:");
  Serial.print("\t");
  Serial.println(value);
  //triggerState = 0;
 }
} 
  

// ISR conv Converts inputs to integer reading reed switches 1,2,4,8. Triggered on hall effect input on interupt pin.

void conv()
{
volatile uint8_t value = 0;
volatile uint8_t triggerState = 0;

        // Current state should be equal to the 4 input BCD values.
  triggerState = 1;
  Serial.print("triggerState2:   ");  // Testing only (2 is only an indicator of 
  Serial.println(triggerState);       // Testing only where it was printed from)
    value = 0;   //Resets value to zero, stops accumulation of value growing
    position[1] = digitalRead(reed1);
    value = value * 2 + position [1];
    //digitalWrite (reed_1, position [1]);
    position[2] = digitalRead(reed2);
    value = value + position[2] * 2;
    //digitalWrite (reed_2, position [2]);
    position[3] = digitalRead(reed3);
    value = value + position[3] * 4;
    // digitalWrite (reed_3, position [4]);
    position[4] = digitalRead(reed4);
    value = value + position[4] * 8;
    //digitalWrite (reed_4, position [8]);

    if (value > prev_value)
    {
      cw = 16;
    }
    else
    {
      cw = 0;
      value = value + cw;
    }
cw = 16;
    /*
    Serial.print ("previous Value: ");
    Serial.println (prev_value);
    Serial.print ("current value: ");
    Serial.println(value);
    Serial.println("++++++++++++++++++");
    */
 
  sum = prev_value + value;
  prev_value = value;
  
  digitalWrite (mot_ena, HIGH);
  delay(25);
}

Forget the local

volatile uint8_t value = 0;
volatile uint8_t triggerState = 0;

and make the global variables volatile that you are hiding with the local declaration.

  while (triggerState = 1);

What does that line of code do ?

How many different variables named triggerState, each with different scopes, do you have in your sketch ?

Those are four different variables, two global and two local. You want both functions to use the globals.

volatile uint8_t value = 0;
volatile uint8_t triggerState = 0;

void conv()
{
}

@woodcrafter: pay close attention to @UKHeliBob's question! :wink:

I wanted the ISR to make the while loop run one time then the end of the while loop would reset triggerState to 0

simply stated: triggerState is set in the ISR to 1 and read to initiate the while loop.
triggerState is set in the ISR and is used in the while loop to transmit the data that the ISR calculated. All the other places you see triggerState are where I have printed it out to see what its status is in that particular location of the sketch. Right now when I trigger the ISR triggerState is set to 1 I know that to be true and it is printed as 1 from inside the ISR. I should also see it in the loop but it does not showup either before or after the while function. Once working I expect to get rid of all the Serial.print statements as they only bugger up the code.

Ok, I have tried both earlier and this is just where after two days of working on it I decided to talk with the experts.

@UKHeliBob was dropping a hint: look carefully at that line. Twice.

So should the volatiles in the ISR be removed?

yes it should be while (triggerState == 1); // DUH

I said "twice" above. There could be another issue....

while (triggerState == 1);
it is still continuing down that sketch even when triggerState is 0 and the interrupt is not triggered and not running. I have about two weeks of sketch programming and I know from old basic programming (TRS80) days that a missing character can play havoc

That's what you programmed.

While triggerState equals 1 do nothing.

while (something == true)
  doThisTask();
while (something == true)  {
  doThisTask();
  doThatTask();
}

Spotted it? Don't worry, everybody on this forum has made both of those mistakes! :laughing:

Ok, I do not understand. I thought that while it was 1 then {do everything below it until the closing } just before the closing I had a triggerState = 0 and thought that the while loop would not be acted on because it was no longer equal to 1. Am I backwards on this?

Loose the semicolon behind the while condition.

OK, this has gone on rather a long time.

Why did you put that ';' after the while?

Got it now? The only time you would write

while (something == true);

...is if you want your code to hang at that point until something gets changed to false by an ISR. It wouldn't be an ideal way to write your code.

So true. Here, it was an extra one that bit you :wink:

I so often miss the semicolon that I now habitually put it after every line, I should know better because I know not to do it while programming an if or for loop.
I thank you for your patience with me and I have now learned to stop and think thru my code more deeply and go back to examples to match up if it is not working.

Again thank you and the rest who took their time to help this novice.

1 Like