blink without delay problem

Using Arduino Programming Questions
#1

Hi! I have a question regarding blink without delay code:

void loop() {
  // here is where you'd put code that needs to be running all the time.

  // check to see if it's time to blink the LED; that is, if the
  // difference between the current time and last time you blinked
  // the LED is bigger than the interval at which you want to
  // blink the LED.
  unsigned long currentMillis = millis();

  if (currentMillis - previousMillis >= interval) {
    // save the last time you blinked the LED
    previousMillis = currentMillis;

    // if the LED is off turn it on and vice-versa:
    if (ledState == LOW) {
      ledState = HIGH;
    } else {
      ledState = LOW;
    }

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);
  }
}

Why is millis() function saved in currentMillis variable before and not used directly? when I do this, I have erratic led toggles.

#2

maybe you have blocking code outside of this block?

What does your entire loop() function look like?

#3

So that previousMillis can be set to the exact time that the comparison was made and without the need for a second call to millis()

when I do this, I have erratic led toggles.

Post your code.

#4

alexmath:
I have erratic led toggles.

What does that mean?- you mean it doesn't always toggle as expected? In that case BulldogLowell is likely yo be right: delays elsewhere are active when you expect a toggle.

But this...

Why is millis() function saved in currentMillis variable before and not used directly? when I do this, I have erratic led toggles.

....makes it sound as if you don't get the problem when you don't set currentMillis to millis()?

#5 
void main()
{
	OSCCONbits.IRCF0 = 1;
	ANSEL = 0;
	ANSELH = 0;
	TRISA = 0xF0;
	TRISD = 0x00;
	InitTimer1();	
	while(1)
	{		
		currentMillis = millis();
		if(currentMillis - previousMillis >6000)
		{
			PORTAbits.RA5bits ^= 1;				
		}
	}
}

Sometimes it lights up and then it lights down back… ???

The hole code:

#include <xc.h>

// CONFIG1
#pragma config FOSC = INTRC_NOCLKOUT// Oscillator Selection bits (INTOSCIO oscillator: I/O function on RA6/OSC2/CLKOUT pin, I/O function on RA7/OSC1/CLKIN)
#pragma config WDTE = OFF // Watchdog Timer Enable bit (WDT disabled and can be enabled by SWDTEN bit of the WDTCON register)
#pragma config PWRTE = OFF // Power-up Timer Enable bit (PWRT enabled)
#pragma config MCLRE = OFF // RE3/MCLR pin function select bit (RE3/MCLR pin function is digital input, MCLR internally tied to VDD)
#pragma config CP = OFF // Code Protection bit (Program memory code protection is disabled)
#pragma config CPD = OFF // Data Code Protection bit (Data memory code protection is disabled)
#pragma config BOREN = OFF // Brown Out Reset Selection bits (BOR enabled)
#pragma config IESO = OFF// Internal External Switchover bit (Internal/External Switchover mode is enabled)
#pragma config FCMEN = OFF // Fail-Safe Clock Monitor Enabled bit (Fail-Safe Clock Monitor is enabled)
#pragma config LVP = OFF // Low Voltage Programming Enable bit (RB3 pin has digital I/O, HV on MCLR must be used for programming)

// CONFIG2
#pragma config BOR4V = BOR40V // Brown-out Reset Selection bit (Brown-out Reset set to 4.0V)
#pragma config WRT = OFF // Flash Program Memory Self Write Enable bits (Write protection off)

#define _XTAL_FREQ 8000000


#define dataInReg PORTAbits.RA0
#define latchReg  PORTAbits.RA1
#define clockReg  PORTAbits.RA2
#define clearReg  PORTAbits.RA3
#define leftPin PORTAbits.RA4
#define upPin PORTAbits.RA5
#define downPin PORTAbits.RA6
#define rightPin PORTAbits.RA7

#define line0 0x00
#define line1 0x01
#define line2 0x02
#define line3 0x03
#define line4 0x04
#define line5 0x05
#define line6 0x06
#define line7 0x07

typedef unsigned char uint8;
typedef signed char sint8;
typedef unsigned int uint16;
typedef signed int sint16;
typedef unsigned long int uint32;
typedef signed long int sint32;


//VOLATILE GLOBAL VARIABLES
volatile uint32 millisecondCounter = 0;



//GLOBAL VARIABLES
uint8 matrix[8];
uint8 currentLine = 0;
uint32 previousMillis = 0;
uint32 currentMillis = 0;

//FUNCTION PROTOTYPES
void InitTimer1();
void shiftOutReg(uint32 value, uint8 nrOfbits);
uint32 millis();
void dropDown();



void main()
{
	OSCCONbits.IRCF0 = 1;
	ANSEL = 0;
	ANSELH = 0;
	TRISA = 0xE0;
	TRISD = 0x00;
	InitTimer1();	
	while(1)
	{		
		currentMillis = millis();
		if(currentMillis - previousMillis >6000)
		{
			PORTAbits.RA5bits ^= 1;				
		}
	}
}





void shiftOutReg(uint32 value, uint8 nrOfbits)
{
			latchReg = 0;
			clearReg = 0;
			clearReg = 1;
			for(int i = 0 ; i < nrOfbits; ++i)
			{
				clockReg = 0;
				dataInReg = (value >> i) & 0x01 ;
				clockReg = 1;
			}
}



void interrupt ISR(){
  if (PIR1bits.TMR1IF){
	++millisecondCounter; 
    PIR1bits.TMR1IF = 0;
    TMR1H	 = 0xF8;
    TMR1L	 = 0x30;	
	shiftOutReg( ~(1<<currentLine), 8);
	PORTD = matrix[currentLine];
	latchReg = 1;
	++currentLine;
	if(currentLine > 7)
	{
		currentLine = 0;
	}
  }
} 



void InitTimer1(){
  T1CON	 = 0x01;
  PIR1bits.TMR1IF	 = 0;
  TMR1H	 = 0xF8;
  TMR1L	 = 0x30;
  PIE1bits.TMR1IE	 = 1;
  INTCON	 = 0xC0;
}



uint32 millis()
{
	return millisecondCounter;
}

This IS HAPPENING only when I do not save millis() in currentMillis!!!

#6

you are missing this line.

previousMillis = currentMillis;

This is what starts/resets the timing sequence, it needs to be within a conditional statement
otherwise this: if(currentMillis - previousMillis >6000)
will always be true after the first 6 seconds

#7