problems with converting delay to millis

I’ve up graded my program to use millis rather than delays. Basically it just sends a pulses with a set duty cycle from 1hz to 6hz the number of pulses at each frequency can also be set .
THis program ran fine when i used delays ( I have commented out the dealy functions.
l have added some Serial.println commands so I can see the values of current Millis and OnTime in the serial monitor. When I run the program the seral monitor starts but the the nstops with a value of 1& " and a back to front question mark"
Anyone any idea were I am going wrong
Also the program stops

uint8_t freq_Hz[6] = {1, 2, 3, 4, 5, 6}; // Set up an array for the 6 frequencies
uint8_t pulses = 25;             // Number of pulses per frequecncy
uint8_t  duty = 10;               // Sets Duty cycle
unsigned long period_ms;
unsigned long OnTime = 0;
unsigned long offTime = 0;
unsigned long currentMillis = 0;
unsigned long startMillis = 0 ;

int out_pin = 5;
int out_pin1 = 2;
int Led1 = 8;
int Led2 = 9;

void setup() {
  // put your setup code here, to run once:
  {
    pinMode(out_pin, OUTPUT);
    pinMode(out_pin1, OUTPUT);
    pinMode(Led1, OUTPUT);
    pinMode(Led2, OUTPUT);

  }

}
void loop()

// put your main code here, to run repeatedly:
{
  Serial.begin(9600);

  for (uint8_t i = 0; i < 6; ++i) {    //Cycle through the 6 elements of freq_hz array


    period_ms = 1000UL / freq_Hz[i];                  // Calculate the period in ms of freq_hz
    OnTime = (period_ms * duty) / 100UL;          // Calculate from % duty cycle ON Time in ms


    for (uint8_t j = 0; j < pulses; ++j) { // loops frequency x pulses at each frequency

      Serial.println(OnTime);// Show OnTime value on Serial Monitor

      unsigned long currentMillis = millis();// get current time

      Serial.println(currentMillis);// show CurrentMillis value on serial monitor
      if (currentMillis - startMillis <= OnTime)//test whether the ONTIME has elapsed
      {
        //Serial.println(currentMillis);
        digitalWrite (out_pin, HIGH);
        digitalWrite (out_pin1, HIGH);
        digitalWrite (Led1, HIGH);
        digitalWrite (Led2, HIGH);
        //delay(OnTime);
      }
      else if (currentMillis - startMillis >= OnTime && currentMillis - startMillis < period_ms  ) // test whether the off time has elapsed

      {

        digitalWrite(out_pin, LOW);
        digitalWrite(out_pin1, LOW);
        digitalWrite(Led1, LOW);
        digitalWrite(Led2, LOW);
        // delay(period_ms - OnTime);
      }
      else if (currentMillis - startMillis >= period_ms - OnTime )
        startMillis = currentMillis;

    }

  }
}

don't put   Serial.begin(9600); in the loop... Once in the setup() is plenty...

don't you need a startMillis per element ?

Start Millis per element? would it be possible you claify that?

The main problem is the for-statements.
Because of them, your code is executed only a finite times, and is finished to fast (before the output has changed).

The loop needs to be "free-running".

Start simple with just one frequency running "forever".
Then add puls counting.
When that works, add varying frequencies.

how can I change my for loops ? If my delays work surley the mills I’ve replaced them with should do the same think, shouldn’t they?

If you want to have different timing for each element of the array then each element needs its own startTime and maybe also its own onTime

It's difficult to be sure what your program is trying to do but I think it would be OK to use the outer FOR loop that iterates over the array.

However the inner FOR loop probably does not work because the times when things will happen cannot be predicted. Rather than use a FOR loop just have variables (again there may need to be an array so you have one for each element) to count the number of pulses whenever a pulse happens and then set the count back to 0 when it reaches 25.

...R

This is the orgininal code. I get 25pulses at each frequency out putted on to a number of pins.
The two delays that I have are delay (OnTime) and delay (period_ms - OnTIme).
All I want to do is remove the delays and use millis(), I thought it would be pretty simple but it’s turning out to be a complete night mare.
I understand what the millis() function does and all I have done it trying to mimic the delay functions, don’t understand why it’s so difficult to do :frowning:

uint8_t freq_Hz[6] = {1, 2, 3, 4, 5, 6}; // Set up an array for the 6 frequencies
uint8_t pulses = 25;             // Number of pulses per frequecncy
uint8_t  duty = 30;               // Sets Duty cycle
unsigned long period_ms;
unsigned long OnTime;
int out_pin = 5;
int out_pin1 = 2;
int Led1 = 8;
int Led2 = 9;
void setup() {
  // put your setup code here, to run once:
  {
    pinMode(out_pin, OUTPUT);
    pinMode(out_pin1, OUTPUT);
    pinMode(Led1, OUTPUT);
    pinMode(Led2, OUTPUT);
  }
}
void loop()

// put your main code here, to run repeatedly:
{
  for (uint8_t i = 0; i < 6; ++i) {               //Cycle through the 6 elements of freq_hz array
    period_ms = 1000 / freq_Hz[i];                  // Calculate the period in ms of freq_hz
    OnTime = (period_ms * duty) / 100;             // Calculate from % duty cycle ON Time in ms

    for (uint8_t j = 0; j < pulses; ++j) {       // loops frequency x pulses at each frequency
      digitalWrite (out_pin, HIGH);
      digitalWrite (out_pin1, HIGH);
      digitalWrite (Led1, HIGH);
      digitalWrite (Led2, HIGH);
      delay(OnTime);
      digitalWrite(out_pin, LOW);
      digitalWrite(out_pin1, LOW);
      digitalWrite(Led1, LOW);
      digitalWrite(Led2, LOW);
      delay(period_ms - OnTime);
    }
  }
}

alanj100:
how can I change my for loops ? If my delays work surley the mills I've replaced them with should do the same think, shouldn't they?

The problem with the for loops running so fast that nothing much appears to happen was pointed out in one of your other threads on the same project.

The reason that the problem does not manifest itself when using delay()s is that the program stops during the delay() period and nothing happens hence a delay() in a for loop will cause each step in the for loop to stop for a fixed period. However, when you use millis() for timing you check whether the timing period has ended and if not keep going until it has. If the check is in a for loop that means that the next step of the for loop executes whether or not the timing period has ended and the for loop ends very quickly with the timing period never having elapsed.

So how do you get round the problem ?
Answer, don't use for loops. Let the loop() function do what its name suggests, ie loop. Keep looping until the millis() timing period has elapsed, checking each time through loop(). When the period ends do whatever needs to happen which would typically be to change the state of an output, move to the next element in an array etc. You save the start time at that point and off you go again round and round loop() until the period ends

To make this easier you should look at your requirements and the states the program could be in such as the output being HIGH for a period, the output being LOW for a period, counting the number of pulses for the current frequency, the end of the array of frequencies etc

The concept is known as a state machine. It sounds scary but it isn't

Thanks for that, I can get the program running using a single frequency. When I look at examples using an array they all seem to use the for statement . Tring to work out how to use the array without for statement

Tring to work out how to use the array without for statement

Keep count of where you are in your own variable and increment it when required

Something like this, but be aware that it is missing details because it is pseudo code

array index = 0
pin state = HIGH
start time = millis()
declare frequency array(s)

start of loop()
  read frequency array using array index
  calculate time for pin to stay HIGH at current frequency
  calculate the required state change count at current frequency
  if the pin has been in its current state long enough (use millis())
    change the pin state
    save the current time from millis()
    add one to state change count
      if the state is LOW and required state change count has been reached
        increment frequency array index for next frequency
        if end of array has been reached
          set frequency array index to zero
        end if  //test for array end
        set state change count to zero
      end if  //test for number of state changes reached
    end if  //
end of loop()

thanks I’ll give it a go :slight_smile: