Help figuring out why the timing is off in this script

I am currently trying to use hardware interrupts to generate a continuous loop of the following tasks:

  1. Turn an LED on to HIGH
  2. Turn off the LED to LOW after 7000 microseconds
  3. Turn on the LED to HIGH again after 15000 microseconds have passed since its HIGH state in number 1
  4. Turn it off to LOW after 7000 microseconds have elapsed.
  5. Wait 197.111+54.555 seconds before repeating steps from the beginning

Meanwhile an external pulse is used to trigger other devices which is being sent every 266667 microseconds in the background.

Here is the code:

#define microsInOneSecond 1000000UL


unsigned long waitBlink;


byte exState,  exPin = 7;
unsigned long startEx, waitEx;




unsigned long fbstart;
unsigned long sbstart;
unsigned long sbstop;
byte ledState, ledPin = 13;


const unsigned long blinkdelay = 54555;
const unsigned long waitOffTime = 197111+ blinkdelay;
unsigned long pulsestart;



void setup() {
 // put your setup code here, to run once:
 Serial.begin( 115200 );
 pinMode(exPin,OUTPUT);
 digitalWrite(exPin,HIGH);
 pinMode( ledPin, OUTPUT );
 waitEx = 266666;
 waitBlink = 15000;
 pulsestart = micros();
}


void external() 
{
if ( waitEx > 0 ) 
{
 
  if (  micros() - startEx >= waitEx) 
  {
    exState = !exState;  
    digitalWrite( exPin, exState ); 
    startEx += waitEx; 
  }
}
else if ( exState > 0 ) 
{
  digitalWrite( exPin, exState = LOW ); 
} 
}






void firstBlinkStart()
{
 
  if (micros()-pulsestart >= waitOffTime)
  {
    digitalWrite(ledPin,HIGH);
    fbstart = micros();
    Serial.println(micros());
  }
 
}


void firstBlinkStop()
{
 
  if(micros()-fbstart >= 7000)
  {
    digitalWrite(ledPin,LOW);
    waitBlink = 15000;
  }
 }
}


void secondBlinkStart()
{
 if(micros()-fbstart >= waitBlink)
 {
   digitalWrite(ledPin,HIGH);
   sbstart = micros();
   Serial.println(micros());
 }
}


void secondBlinkStop()
{
 if(micros()-sbstart >= 7000)
 {
   digitalWrite(ledPin,LOW);
   pulsestart = micros();
 }
}


void loop() {
 // put your main code here, to run repeatedly:
 external();


 firstBlinkStart();
 firstBlinkStop();
 secondBlinkStart();
 secondBlinkStop();


}

The problem right now is the timing is completely off. I am trying to make it so that in the serial monitor, I would be seeing 15,000 microseconds of difference between two numbers, followed by a difference of 251000 (197111+54555) then a difference of 15,000 again and so forth. But it is not doing that. (Note that the 7000 microseconds is just how long it is on for but does not have any bearing on the timing between each individual blink)

Thank you in advance.

I'm sorry, but that is not how to write code.

Please right-click with the mouse on the code and use "Format document". Then make everything look good. Make good use of indents, empty lines, comma's, spaces, and so on. You do that for yourself, not for me.

The digitalWrite() takes a HIGH or a LOW.

digitalWrite(ledPin, LOW);

A boolean "not" operator is best used on a "bool" variable.

bool ledActivated = false;
if(!ledActivated)
{
  ...
}

A sketch is easier to read if you use the same names as the Arduino examples.

unsigned long interval;
unsigned long previousMillis;

I do not understand the relation between these functions: firstBlinkStart(), firstBlinkStop(), secondBlinkStart(), secondBlinkStop(). Those are not millis-timers, you will get into a lot of trouble that way.

There is just one LED, therefor it is better to use one millis-timer. You can adjust the interval in the millis-timer.

Using millis() is better than using micros(). It might even be more accurate in your sketch.

You are sending too many messages to the Serial Monitor with Serial.println(). There is a TX buffer of 64 bytes in the Serial library. If that is completely filled, then the sketch waits for a free spot in the buffer, and then one byte is put in the buffer. A sketch can run 100 times slower that way. Your baudrate is 115200, that is better than 9600, but you will still get a full TX buffer.

This is dangerous: const unsigned long waitOffTime = 197111+ blinkdelay;
The compiler defaults to a integer (int), and 197111 does not fit in a integer on a Arduino Uno.

Which Arduino board do you use ?

Thanks for the reply.

I am using an UNO board. As for the relation between the different functions the firstBlinkStart() only aims to turn on LED during the first blink while the firstBlinkStop() only aims to turn it off. The secondBlinkStart() turns on the led again based on a timing difference where the recorded value is from firstblinkStart(). secondBlinkStop() then turns it off again.

This is the source I have been trying to use to help me implement this:

I know that is what you want, but those functions do not run sequentially. You can avoid such trouble by using a single millis-timer and changing the interval from inside the millis-timer.

The tutorial by Nick Gammon does not show how to do a sequence of timing.
I have two simple examples, but there are more good ways to implement it. They are "millis_different_on_and_off_times.ino" and "millis_rhythm.ino" here: Fun_with_millis/README.md at master · Koepel/Fun_with_millis · GitHub
Both have just one millis-timer with just one: if (currentMillis - previousMillis >= interval)

Ok i will try to implement that. I was under the impression that it would all run sequentially because of the if conditions I set which force them to only contribute when the timing criteria is met.

I took a few million samples of your code, but only managed to copy/paste groups of 400 (the IDE does not allow Serial Monitor copy CTRL-A/CTRL-C). Here is what I have... the chart shows samples on bottom and time between samples in microseconds on the left. You have 430, then 9, then 430, then 9. The 400 step from 300us difference to 500us to 600us and one 700us... Hope this helps.

First 400 samples...
image

Then these nine samples on their own evenly spaced at 580us...

267900
268580
269260
269940
270620
271300
271980
272660
273340

Then the next 400 samples...

then nine samples exactly at 780us...

540832
541512
542192
542872
543552
544232
544912
545592
546272

... and it repeats

So this tells me that the timing isn't behaving as it should because the interval is increasing with time instead of staying consistent?

I would be scolded to say "yes, I agree" after only 1200 samples so I will post a few more from the same run and take the beatings. : ) ... and notice the graph "2-steps" then "3-steps" then 9 samples, back to "2-step"... et c.

Next 9

813764
814444
815128
815804
816484
817168
817844
818524
819208

Next 400

Next 9

1086776
1087540
1088308
1089072
1089836
1090608
1091368
1092132

So I looked at both of your scripts. The different on off times script is helpful when I have a different time I want it to be on and off for. In this case I want it to be on for 7 but off for either 251000 microseconds or 15,000 depending on which of the 2 I am at. It changes state by setting the state to opposite of whatever it is currently.

As for the rhythm I can see it working, but I would have to create the index to have specialized values of on and off times. As I am trying to make it user friendly so that they can change the main interval they want, the blink width, and such and have over 100 iterations. I prefer to not have to create an index of over 100 on and off values.

Create an array of intervals...

Lol at this point ive been given so many different ways to do this that im just confused. I tried to make it as simple as I can and fix what I can. But Ill take a look.

Hello JLam

Arrays are your friend.

Check and test this proposal:

//https://forum.arduino.cc/t/help-figuring-out-why-the-timing-is-off-in-this-script/1244771
//https://europe1.discourse-cdn.com/arduino/original/4X/7/e/0/7e0ee1e51f1df32e30893550c85f0dd33244fb0e.jpeg
#define ProjectName "Help figuring out why the timing is off in this script"
#define NotesOnRelease "Arduino MEGA tested"
// make variables
uint32_t currentMicros = micros();
constexpr uint32_t timing[] {7000, 15000, 7000, 197111 + 54555 };
constexpr uint8_t led {9};
// make support
void heartBeat(const uint8_t LedPin, uint32_t currentMillis)
{
  static bool setUp = false;
  if (setUp == false) pinMode (LedPin, OUTPUT), setUp = true;
  digitalWrite(LedPin, ((currentMillis) / 500) % 2);
}
// make application
void setup()
{
  Serial.begin(115200);
  Serial.print("Source: "), Serial.println(__FILE__);
  Serial.print(ProjectName), Serial.print(" - "), Serial.println(NotesOnRelease);
  pinMode(led, OUTPUT);
  digitalWrite(led, HIGH);
  Serial.println(" =-> and off we go\n");
}
void loop()
{
  currentMicros = micros();
  static uint32_t previousTime = 0;
  static uint16_t timingCounter = 0;
  heartBeat(LED_BUILTIN, millis());
  if (currentMicros - previousTime >= timing[timingCounter])
  {
    previousTime = currentMicros;
    timingCounter = (timingCounter + 1) % (sizeof(timing) / sizeof(timing[0]));
    digitalWrite(led, digitalRead(led) ? LOW : HIGH);
  }
}

Have a nice day and enjoy coding in C++.

Hi thank you for taking the time to reply. I wanted to ask about timingCounter in the loop. I get that we are incrementing so we can move through the array, but does the timingCounter = 0; at the top make it so that every time we go through the loop the index is 0? Or is that the purpose of static? To have them equal 0 only at the first iteration prior to any changes?

Yes, this the declaration of a local used variable.

Hey Paul, a follow up question if you do not mind.

Do you think if the LED is not blinking at that speed (and Im not claiming to be able to view things at microsecond intervals) but the blinking seems rather slow, could it be an issue with the power rating of the led emitter? Thank you. I checked the timing on the serial monitor and it seems very good.

edit: changing heartBeat(LED_BUILTIN,millis()) to heartBeat(LED_BUILTIN,micros()) seems to fix this. Albiet making the flash way dimmer

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.