swapping out the delays for millis

#define DATA 8    
#define LATCH A3      
#define CLOCK 12      



void shiftByteMSF(byte HB){
digitalWrite(LATCH, LOW);
shiftOut(DATA, CLOCK, LSBFIRST, HB);
shiftOut(DATA, CLOCK, MSBFIRST, HB);
digitalWrite(LATCH, HIGH);
}


void setup(){
    pinMode(LATCH, OUTPUT);
    pinMode(DATA,  OUTPUT);  
    pinMode(CLOCK, OUTPUT);
}


void loop(){
    byte HBON = B10000000;
    shiftByteMSF(HBON);
    delay(500);
    byte HBOFF = B00000000;
    shiftByteMSF(HBOFF);
    delay(500);    

    
        
    }

so i need to convert this code to baremetal so this is my baremetal code:

#define DATA 8   
#define LATCH A3     
#define CLOCK 12
#define LATCH_ON PORTC = PORTC | B00001000
#define LATCH_OFF PORTC = PORTC & B11110111
#define CLOCK_ON PORTB = PORTB | B00010000
#define CLOCK_OFF PORTB = PORTB & B11101111
#define DATA_ON PORTB = PORTB | B00000001
#define DATA_OFF PORTB = PORTB & B11111110     


void shiftByteMSF(byte HB){


LATCH_OFF; //sets latch low before data sending
  for(int shiftBit=0; shiftBit<8;  shiftBit++) { //repeat 8 times, incrementing .
    if(bitRead(HB, 7-shiftBit) == 1) { //read the bit of the variable for sReg, and start from Right-most bit
      DATA_ON;
      //Serial.println(bitRead(sRegState, 7-shiftBit));
    }
    else{
     DATA_OFF;
      //Serial.println(bitRead(sRegState, 7-shiftBit));
    }
    CLOCK_ON; //cycle clock once data value stable
    CLOCK_OFF;
    DATA_OFF; //0 on data, if it was already high, after clock pulse
    }
  LATCH_ON; //latch to output data to register
 

     
 
    } 
   
void setup(){
DDRC|=LATCH_ON;
DDRB|=DATA_ON; 
DDRB|=CLOCK_ON;
}


void loop(){
  
int period = 1000;
unsigned long time_now = 0;
unsigned long previoustime = 0;

if((millis()+time_now)<period);{  
time_now = millis();
byte HBON = B10000000;
shiftByteMSF(HBON);

}
if((millis() - previoustime) > period);{
previoustime = millis();
byte HBOFF = B00000000;
shiftByteMSF(HBOFF);


}
   
       
}

now the problem im getting is that once i swapped out the delays for millis the DP on my 7segment display is not flashing like how it was in the first code. can anyone see the problem?

Never add something to a millis value. This is not okay: millis()+time_now
The Blink Without Delay is how it works: https://www.arduino.cc/en/Tutorial/BuiltInExamples/BlinkWithoutDelay.

Suppose you have a millis-timer that creates a 'tick' every 500 ms.
With the first 'tick' you do something, with the second 'tick' something else, and so on.
Therefor I suggest to make a counter.

unsigned long previousMillis;
int counter = 0;

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

  if( currentMillis - previousMillis >= 500)    // a 'tick' every 500 ms
  {
    previousMillis = currentMillis;

    if( counter == 0)
    {
      // do something
      ...
    }
    else if( counter == 1)
    {
      // do something else
      ...
    }

    counter++;
    if( counter > 1)
    {
      counter = 0;
    }
  }
}

There are many other ways to solve this. I choose this one. I don't know why.

The clock pulse is very short. I hope the hardware can detect it.
If you want, it can be even shorter. A output pin can be toggled by writing a bit to the PIN register while the pin is set as output. This works on a Arduino Uno with the ATmega328P.

Update: When only a led needs to be turned on and off (even when that led is part of a 7-segment display), then this will work, but it can be simpler. Use the Blink Without Delay.

if((millis()+time_now)<period);{

The semicolon makes this do nothing because it terminates the if code block. Instead all the lines in the following code block are ALWAYS executed.

But also,

void loop(){
 
int period = 1000;
unsigned long time_now = 0;
unsigned long previoustime = 0;

You made your timing variables local to loop() and intitialize them to zero every time through loop. They don’t retain their value between successive iterations of loop(). So no timing can be accomplished.

  LATCH_ON; //latch to output data to register
 

     
 
    }

Get rid of the silly extra blank lines, they don’t help anyone that is reading, use ctrl-T to perform automatic formatting on your code (mainly to produce readable indentation).

so i took out the semi colon so when re uploaded the sketch and it flashed once and thats it why is that?

There is more than one.

yh took both of them out

What about the timing variables I mentioned?

yh

aarg:
What about the timing variables I mentioned?

yh so the timing variables iv moved outside of loop and setup still getting the same problem

#define DATA 8   
#define LATCH A3     
#define CLOCK 12
#define LATCH_ON PORTC = PORTC | B00001000
#define LATCH_OFF PORTC = PORTC & B11110111
#define CLOCK_ON PORTB = PORTB | B00010000
#define CLOCK_OFF PORTB = PORTB & B11101111
#define DATA_ON PORTB = PORTB | B00000001
#define DATA_OFF PORTB = PORTB & B11111110     

int period = 1000;
unsigned long time_now = 0;
unsigned long previoustime = 0;
void shiftByteMSF(byte HB){


LATCH_OFF; //sets latch low before data sending
  for(int shiftBit=0; shiftBit<8;  shiftBit++) { //repeat 8 times, incrementing .
    if(bitRead(HB, 7-shiftBit) == 1) { //read the bit of the variable for sReg, and start from Right-most bit
      DATA_ON;
      //Serial.println(bitRead(sRegState, 7-shiftBit));
    }
    else{
     DATA_OFF;
      //Serial.println(bitRead(sRegState, 7-shiftBit));
    }
    CLOCK_ON; //cycle clock once data value stable
    CLOCK_OFF;
    DATA_OFF; //0 on data, if it was already high, after clock pulse
    }
  LATCH_ON; //latch to output data to register
 
 } 
   
void setup(){
DDRC|=LATCH_ON;
DDRB|=DATA_ON; 
DDRB|=CLOCK_ON;
}


void loop(){
  
int period = 1000;
unsigned long time_now = 0;
unsigned long previoustime = 0;

if((millis()+time_now)<period){  
time_now = millis();
byte HBON = B10000000;
shiftByteMSF(HBON);

}
if((millis() - previoustime) > period){
previoustime = millis();
byte HBOFF = B00000000;
shiftByteMSF(HBOFF);


}
   
       
}

aarg:
Your formatting is still bad.

sorry i will sort everything once iv completed the codes

You didn't move the timing variables:

void loop(){
 
int period = 1000;
unsigned long time_now = 0;
unsigned long previoustime = 0;

Google, "C++ variable scope".

aarg:
You didn’t move the timing variables:

void loop(){

int period = 1000;
unsigned long time_now = 0;
unsigned long previoustime = 0;




Google, "C++ variable scope".

sorry i forgot to delete it out of void loop

#define DATA 8   
#define LATCH A3     
#define CLOCK 12
#define LATCH_ON PORTC = PORTC | B00001000
#define LATCH_OFF PORTC = PORTC & B11110111
#define CLOCK_ON PORTB = PORTB | B00010000
#define CLOCK_OFF PORTB = PORTB & B11101111
#define DATA_ON PORTB = PORTB | B00000001
#define DATA_OFF PORTB = PORTB & B11111110     

int period = 1000;
unsigned long time_now = 0;
unsigned long previoustime = 0;
void shiftByteMSF(byte HB){


LATCH_OFF; //sets latch low before data sending
  for(int BitShift=0; BitShift<8;  BitShift++) //repeat 8 times, incrementing .
    if(bitRead(HB, 7-BitShift) == 1)  //read the bit for HB, and begins from right most bit
      DATA_ON;
    
    
    else{
     DATA_OFF;
      
    }
    CLOCK_ON; //cycle clock once data value stable
    CLOCK_OFF;
    DATA_OFF; //0 on data, if it was already high, after clock pulse
  LATCH_ON; //turns latch on to transfer the data to the register
} 

   
void setup(){
DDRC|=LATCH_ON;
DDRB|=DATA_ON; 
DDRB|=CLOCK_ON;
}


void loop(){
  


if((millis()+time_now)<period){  
time_now = millis();
byte HBON = B10000000;
shiftByteMSF(HBON);

}
if((millis() - previoustime) > period){
previoustime = millis();
byte HBOFF = B00000000;
shiftByteMSF(HBOFF);


}
   
       
}

now that its out of void loop nothing happens whatsoever before it used to flash once now it does nothing. btw i looked at C++ variable scopes it talks about local and global variables but the way they are defined in the examples cannot be used in arduino IDE as it does not detect it as a variavle i.e. int global period = 1000.

if((millis()+time_now)<period){

If my memory serves, you’ve already been told not to attempt to use future time stamps.

aarg:

if((millis()+time_now)<period){

If my memory serves, you’ve already been told not to attempt to use future time stamps.

what future time stamps?? i dont understand

Oh, my bad. 'time_now' isn't a future stamp, but what is it? You are adding a millis() value to another millis() value. It's just insane. Especially because the other "if (millis" you have there is correct.

if((millis() - previoustime) > period){
previoustime = millis();

It seems like you're sleepwalking through the design, without attempting to understand how it works.

Your formatting is still bad.

sorry i will sort everything once iv completed the codes

Sorting out the formatting is as much for the writer's sake (that's you) as for those trying to help. I find it much easier to write code if I keep the structure tidy all the time: constant control-T's and stick to some or other "rules" for things like where to put the { and } for example.

Don’t add stuff to millis(). It will give you problems the moment millis() rolls over. The beauty of integer maths is that

millis() - previousMillis > interval

always works, even when it rolls over.

Also you really should do your code formatting now. It’s a single keypress, and it not only helps us reading your code, it helps you reading your code as least as much.

Finally, this lovely snippet:

if((millis()+time_now)<period) { 
  time_now = millis();
  byte HBON = B10000000;
  shiftByteMSF(HBON);
}

evaluates true every run of loop() for the first 500 ms you run your sketch, and then again some 49 days later for another 250 ms, and another 49 days later for 125 ms…

(hint: how many milliseconds are there in 49 days and what’s the largest value a 32-bit unsigned integer can hold?)

aarg:
Oh, my bad. 'time_now' isn't a future stamp, but what is it? You are adding a millis() value to another millis() value. It's just insane. Especially because the other "if (millis" you have there is correct.

if((millis() - previoustime) > period){

previoustime = millis();



It seems like you're sleepwalking through the design, without attempting to understand how it works.

so what i am trying to do is have like a 'chain ' of timers which kinda lead on from the previous time which is where time_now and previoustime comes in. im certain the approach is correct the way i have executed the approach is wrong. which is where i need the help is there a possibility where you could give an example of the same/similar approached being used and most importantly how it is executed in code? i would really appreciate it

m005a:
so what i am trying to do is have like a 'chain ' of timers which kinda lead on from the previous time...

What is the goal? What are you building?

m005a:
so what i am trying to do is have like a 'chain ' of timers which kinda lead on from the previous time which is where time_now and previoustime comes in.

Please post the latest version of your complete program.

...R

dougp:
What is the goal? What are you building?

so basically part of myt project is to make a so called ‘heartbeat’ of the circuit that operates throughout the whole sequence regardless of what buttons or leds areit it has to be flashing to indicate the code is working. so this 'heartbeat ’ is indicated by the DP on the 7 segment display wich is connected to the 74HC595 shift register. so i have basically made the code the only thing is the delay at which the flashing has to be is not functioning . as i am not allowed to use delay i chose to use the millis() function

#define DATA 8
#define LATCH A3
#define CLOCK 12
#define LATCH_ON PORTC = PORTC | B00001000
#define LATCH_OFF PORTC = PORTC & B11110111
#define CLOCK_ON PORTB = PORTB | B00010000
#define CLOCK_OFF PORTB = PORTB & B11101111
#define DATA_ON PORTB = PORTB | B00000001
#define DATA_OFF PORTB = PORTB & B11111110

int period = 1000;
unsigned long time_now = 0;
unsigned long previoustime = 0;
void shiftByteMSF(byte HB) {


  LATCH_OFF; //sets latch low before data sending
  for (int BitShift = 0; BitShift < 8;  BitShift++) //repeat 8 times, incrementing .
    if (bitRead(HB, 7 - BitShift) == 1) //read the bit for HB, and begins from right most bit
      DATA_ON;


    else {
      DATA_OFF;

    }
  CLOCK_ON; //cycle clock once data value stable
  CLOCK_OFF;
  DATA_OFF; //0 on data, if it was already high, after clock pulse
  LATCH_ON; //turns latch on to transfer the data to the register
}


void setup() {
  DDRC |= LATCH_ON;
  DDRB |= DATA_ON;
  DDRB |= CLOCK_ON;
}


void loop() {



  if ((millis() + time_now) < period) {
    time_now = millis();
    byte HBON = B10000000;
    shiftByteMSF(HBON);

  }
  if ((millis() - previoustime) > period) {
    previoustime = millis();
    byte HBOFF = B00000000;
    shiftByteMSF(HBOFF);


  }


}