[solved] ATTiny85 if statement failing mysteriously

I’ve got a perplexing problem and I hope someone here has an answer for me. I have an if statement at the start of the main loop that works one time and then seems to skip the condition thereafter. What complicates this is that it is on an ATTiny 85 and I had no way of debugging with console, that is until I did some digging and found I could use the Uno serial connection. So I adjusted my program to use TinyDebugSerial and got a weird result. As you can see from the code below, if I run the program with the first Serial statement commented out, it fails to function properly. If, however, I leave the statement in, the program runs as it should. The way the program should work is that the analog read and write statements should run constantly unless the fan rpm drops below the threshold and then the output should drop to zero and the led will flash indicating the error. Because the checkFan function needs 1 second to get a reading from the rpmPin, I didn’t want this constantly interfering with the analog read/write so I set it to sample every 5 seconds (i’m probably going to increase this time, if I can get the program working right). The fastPWM stuff is so that I can get a PWM frequency of about 4KHz.

I figured there was a timing issue somewhere so I tried adding a small delay at both the start and the end of the main loop, but I can only get the program to work if the initial serial statement is in there (the one that says “here”). With that statement in, I get the other serial data every 5 seconds just like I planed it. As an added bonus, the “flashLED” function does not work right either. The LED only flashes once and not twice as expected (more of an annoyance than a problem right meow).

I am using the internal 8MHz oscillator. I commented out the fastPWM stuff when I started trying to troubleshoot, thinking maybe that was causing an issue but to no avail. I have also tried using a fresh micro as well with the same results. I have commented out one of my outputs as I needed that pin for the serial debugging. Just ignore the other commented stuff as this is stuff I use in the actual application (to keep up with PWM timings and such…yet another headache I have had to battle through).

Please forgive the terrible coding that I’m sure some of you seasoned programmers are gonna find. I’m learning from multiple sources all using different practices and methods of doing things. I try my best to understand the code snippets I find on the internet before deploying them in my projects. I hope I was descriptive enough.

#include <TinyDebugSerial.h>
TinyDebugSerial Serial = TinyDebugSerial();

const int pwmPin = 1;
const int ledGrn = 0;
//const int ledRed = 3;
const int potPin = A2;
//const int rpmPin = 2;
int inVal = 0;
int NbTopsFan; int Calc;
//The pin location of the sensor
int hallsensor = 2; typedef struct{
     
//Defines the structure for multiple fans and their dividers 
char fantype;
unsigned int fandiv; }fanspec;
//Definitions of the fans
//This is the varible used to select the fan and it's divider,
//set 1 for unipole hall effect sensor and 2 for bipole hall 
//effect sensor
fanspec fanspace[3]={{0,1},{1,2},{2,8}}; char fan = 1;

// when using millis, 1 sec = 7812
//unsigned long interval = 31248;  //this is 4s (7812 x 4)
unsigned long interval = 5000;
unsigned long curTime;
volatile bool cooling = true;

void setup() {
  // set the micro to use fastPWM
//  TCCR0A = 2<<COM0A0 | 2<<COM0B0 | 3<<WGM00;
//  TCCR0B = 0<<WGM02 | 1<<CS01;
//  TCCR1 = 0<<PWM1A | 0<<COM1A0 | 1<<CS10;
//  GTCCR = 1<<PWM1B | 2<<COM1B0;
  pinMode(pwmPin, OUTPUT);
  analogWrite(pwmPin, 0);
  pinMode(ledGrn, OUTPUT);
//  pinMode(ledRed, OUTPUT);
//  pinMode(rpmPin, INPUT);
  pinMode(hallsensor, INPUT);
  attachInterrupt(0, rpm, RISING);
  digitalWrite(ledGrn, HIGH); // turn on green LED for control OK
//  digitalWrite(ledRed, LOW);
  //delay(3125); //500ms
  Serial.begin(9600);
  delay(20); //500ms
}

void loop() {
//  Serial.println("here");
  delay(20);
  if (cooling) {
    inVal = analogRead(potPin);
    analogWrite(pwmPin, inVal/4);
    if (millis() >= interval) {
//      interval = millis() + 31248;
      interval = (millis() + 5000UL);
      Serial.print(millis());
      Serial.print(" mills and ");
      Serial.print(interval);
      Serial.println(" interval");
//      if (interval >= 4294951665) { // check to see if interval is close to millis overflow
//        interval = 31248;  //reset interval if it is above millis overflow less 2 secs
//        interval = 4000UL;  //reset interval if it is above millis overflow less 4 secs
//      }
      checkFan();
    }   
  } else {
    analogWrite(pwmPin, 0);
    flashLED();
    checkFan();
  }
  //delay(20);
}

void rpm() {
  NbTopsFan++;
}

void checkFan() {
  NbTopsFan = 0;
  sei();
//  delay(7812);
  delay(1000);
  cli();
  Calc = ((NbTopsFan * 60)/fanspace[fan].fandiv);
  if (Calc >= 6500) {
    cooling = true;  // turn on green side of LED
    grnLED();
  } else {
    cooling = false;
    digitalWrite(ledGrn, LOW);
    //redLED();  
  }
}

void flashLED() {
  digitalWrite(ledGrn, LOW);
  delay(400);  
  digitalWrite(ledGrn, HIGH);
  delay(400);
  digitalWrite(ledGrn, LOW);
  delay(1000);
}

void grnLED() {
  digitalWrite(ledGrn, HIGH);
//  digitalWrite(ledRed, LOW);
}

//void redLED() {
//  digitalWrite(ledRed, HIGH);
//  digitalWrite(ledGrn, LOW);
//}

Nothing jumps out at me.

What core are you using? Some of them use timer1 for millis, some use timer0. My core uses timer0 - I leave the fancy timer free and use the 8-bit timer like every other arduino board does; for some reason some of the cores use timer1 for millis, depriving the user of the most interesting peripheral on the tiny85, for no improvement in code portability since timer0 is used for millis everywhere else, so there isnt a library of pre-written arduino code for timer0....

If you're using a core that uses timer0 , then the timer reconfiguration you do will trash the timekeeping, potentially throwing the delay period way off. If the core you're using uses timer1 for millis, this doesn't explain it.

You don't need to do anything special to deal with millis overflow if you use subtraction when you test it (think about it, after overflow, you'll be subtracting a much larger value from the current millis - causing it to wrap back around and give the right answer anyway)

Thanks DrAzzy for the info on handling the roll-over. I'll delete that from my final code. The core I'm using is the one that is bundled with the Arduino IDE which is the version by David Mellis (or maybe I added it and just forgot about that - I'm not opposed to changing cores if there is a better one out there). Yes, the reconfiguration of the timer does affect millis() and delay(), but I have already calculated the correct values I need for my time keeping. This is also why I commented it out of code, but I still have the issue.

I didn't think my code was broken, but it is a little frustrating not being able to understand why having the serial command at the start of the loop causes everything to work fine, but a delay does not.

Some more background on the setup of this circuit. The fan is a 12v CPU fan with the tach output pulled high to the 5v rail via a 1k resistor. This keeps the pulses at 5V so that if the fan is disconnected, the rpmPin sees a steady 5v and therefore no movement. If I start the program in this condition, then about 4 secs after start the "error" condition is run and the LED flashes. If I start the program with the fan plugged in and wait about 5 secs then unplug the fan, nothing happens. The program continues to run without the "error" condition. When I uncomment the Serial command and start the program with the fan on and then unplug the fan after about 5 secs, the "error" condition is run and the led flashes (as it should - and I can plug the fan back in and the program then goes back to normal function).

My core is imo better than David's:

  1. It has a builtin software serial implementation on the AIN0/1 pins (uses the analog comparator interrupt instead of a pcint, so it interferes with fewer other things than most software serial implementations).
  2. It has configuration options for almost every conceivable clock source and fuse setting, and supports almost every attiny device.
  3. It has my "unified wire" and "unified spi" libraries, which allow you to treat any of the attiny devices my core supports as if they had real hardware I2C or SPI, and it will pick the appropriate implementation to use for that hardware and present a compatible interface, so code and libraries can usually be used without modification.
  4. It is actively and aggressively maintained.

Anyway... why not leave timer0 alone and use timer1 to generate the PWM?

It's perfectly capable of generating PWM, in fact, better at it than timer0 (timer1 has every power of 2 as an option in the prescaler, and you can clock it off the internal PLL giving it a base frequency of ~64mhz if you for some reason need very high frequency PWM)

DrAzzy, I'm using timer 0 because that's the one I found the most information on as well as examples (again, this is for the ATTiny85). Most other help I found simply said "read the datasheet" or there were links to the datasheet; which is all good and well, but if you don't have a history with micros and their verbiage, then the information there might as well be in Mandarin. I would like to learn how it all works, but I have been unable to find a good source that explains the register bits and the various methods of setting them (because what I have found is there is more than one way).

Having said all of that, PWM is not my problem. The main problem still persists. I have stripped down my code and still have the issue (see below). Whats really bothering me is the fact that the error condition, when it works on the initial pass, still only flashes the LED once instead of twice and I know the delay I have set in there is plenty of time for me to be able to see the double flash. I even added a digital write to the start of the loop just to see if that had an effect. It did not. I will try your core and see if that makes a difference.

const int ledGrn = 0;
//const int outPin = 1;
int NbTopsFan; int Calc;
//The pin location of the sensor
int hallsensor = 2; typedef struct{
     
//Defines the structure for multiple fans and their dividers 
char fantype;
unsigned int fandiv; }fanspec;
//Definitions of the fans
//This is the varible used to select the fan and it's divider,
//set 1 for unipole hall effect sensor and 2 for bipole hall 
//effect sensor
fanspec fanspace[3]={{0,1},{1,2},{2,8}}; char fan = 1;

// when using millis, 1 sec = 7812
//unsigned long interval = 31248;  //this is 4s (7812 x 4)
unsigned long interval = 5000;  //this is 5s (7812 x 4)
unsigned long curTime;
volatile bool cooling = true;

void setup() {
  pinMode(ledGrn, OUTPUT);
//  pinMode(outPin, OUTPUT);
  pinMode(hallsensor, INPUT);
  attachInterrupt(0, rpm, RISING);
  digitalWrite(ledGrn, HIGH); // turn on green LED for control OK
  delay(20); //500ms
}

void loop() {
//  digitalWrite(outPin, HIGH);
  if (cooling) {
    if (millis() >= interval) {
      interval = (millis() + 5000UL);
      checkFan();
    }   
  } else {
    digitalWrite(outPin, LOW);
    digitalWrite(ledGrn, LOW);
    delay(800);  // this equals 200ms (7812 x .2s)
    digitalWrite(ledGrn, HIGH);
    delay(400);
    digitalWrite(ledGrn, LOW);
    delay(400);  // this equals 200ms (7812 x .2s)
    digitalWrite(ledGrn, HIGH);
    delay(400);
    digitalWrite(ledGrn, LOW);
    delay(1000);
    checkFan();
  }
}

void rpm() {
  NbTopsFan++;
}

void checkFan() {
  NbTopsFan = 0;
  sei();
  delay(1000);
  cli();
  Calc = ((NbTopsFan * 60)/fanspace[fan].fandiv);
  if (Calc >= 6500) {
    cooling = true;  // turn on green side of LED
    digitalWrite(ledGrn, HIGH);
  } else {
    cooling = false;
    digitalWrite(ledGrn, LOW);
  }
}

NbTopsFan needs to be declared volatile, since it's modified in an ISR but accessed outside of one, otherwise accesses to it could be optimized away.

Edit: AAH! You're using delay with interrupts globally disabled! Delay relies on the timekeeping interrupt firing in the background to count the time; without it, delay() will hang...

Your code would be more readable if you did ctl+T in the IDE. Putting multiple statements on the same line is bad practice, as it makes it harder to spot mistakes.

DrAzzy:
NbTopsFan needs to be declared volatile, since it's modified in an ISR but accessed outside of one, otherwise accesses to it could be optimized away.

I agree, not sure how I missed that.

DrAzzy:
Edit: AAH! You're using delay with interrupts globally disabled! Delay relies on the timekeeping interrupt firing in the background to count the time; without it, delay() will hang...

BINGO! We have a winner. I was not aware of that but it shall be duly noted for the future. Let me rework this and I will get back to you. Thank you for your help so far.

Ok, Thanks to DrAzzy for your help on this. Everything is working fine now and here is the final code that made it all possible. I removed the interrupt enable/disable calls all together and just went with a while loop to give me a 1 sec sample. Of course this means that the interrupt is going to be firing about 8000 times a sec (approx speed of the fan) but it shouldn’t be a problem (unless someone does see a problem with that).

const int ledGrn = 0;
const int pwmPin = 1;
const int potPin = A2;
int inVal = 0;
volatile unsigned long NbTopsFan;
int Calc;
int hallsensor = 2;

unsigned long interval = 5000;
unsigned long curTime;
volatile bool cooling = true;

void setup() {
  pinMode(ledGrn, OUTPUT);
  pinMode(pwmPin, OUTPUT);
  analogWrite(pwmPin, 0);
  pinMode(potPin, INPUT);
  pinMode(hallsensor, INPUT);
  attachInterrupt(0, rpm, RISING);
  digitalWrite(ledGrn, HIGH);
  delay(10);
}

void loop() {
  if (cooling) {
    inVal = analogRead(potPin);
    analogWrite(pwmPin, inVal / 4);
    if (millis() >= interval) {
      interval = (millis() + 6000UL);
      checkFan();
    }
  } else {
    analogWrite(pwmPin, 0);
    digitalWrite(ledGrn, LOW);
    delay(500);
    digitalWrite(ledGrn, HIGH);
    delay(150);
    digitalWrite(ledGrn, LOW);
    delay(150);
    digitalWrite(ledGrn, HIGH);
    delay(150);
    digitalWrite(ledGrn, LOW);
    checkFan();
  }
}

void rpm() {
  NbTopsFan++;
}

void checkFan() {
  NbTopsFan = 0;
  unsigned long rpmNum = 0;
  unsigned long pause = millis() + 1000UL;
  while (millis() <= pause) {
    rpmNum = NbTopsFan;
  }
  Calc = ((rpmNum * 60) / 2);
  if (Calc >= 6500) {
    cooling = true;
    digitalWrite(ledGrn, HIGH);
  } else {
    cooling = false;
    digitalWrite(ledGrn, LOW);
  }
}