Offsetting a timer based trigger pulse

That subject line doesn't help much does it ?

This is part of a synthesizer arpeggiator project I've build for a friend. The code below is a separate sketch to figure out this problem.

I'm using the Timer1 library. Timer1 is being used to create a trigger pulse on an output. So at the moment I have Timer1's interrupt service routine store millis() into a variable. Within loop() I have an two if() that checks for when 20 milliseconds has elapsed it sets the output to LOW. Easy. I'm also using a potentiometer to change the time interval of Timer1. This is ARP_SPEED.

In addition to this I have another potentiometer, TRIGGER_OFFSET_POTENTIOMETER, which offsets or delays the time that the trigger pulses. Using the simple millis() check it works fine until the offset time gets close to the retrigger time of the next Timer1 service. At this point the Trigger output just stays high.

I actually will be using both the Timer1 based trigger and the offset trigger.

Here's the code:-

#include <TimerOne.h> // for the internal clock

#define ARP_SPEED A0
#define TRIGGER_OFFSET_POTENTIOMETER A1
#define TRIGGER_OUT 2
#define INDICATOR_OUT 3
#define TRIGGER_LENGTH 20

uint32_t arpSpeedAverage = 200;
uint16_t arpSpeedConstrain = 0;
float arpSpeed = 200;
float previousArpSpeed = 0;
uint32_t triggerStartTimer = 0;
uint16_t triggerOffset = 0;
uint8_t whichTrigger = LOW;


void setup()
{
    // initialise the internal clock as a TImer1 interrupt
    Timer1.initialize(100000); // set a timer of length 100000 microseconds (or 0.1 sec - or 10Hz => the led will blink 5 times, 5 cycles of on-and-off, per second)
    Timer1.attachInterrupt( internalClockInterrupt ); // attach the service routine here

    pinMode(TRIGGER_OUT, OUTPUT);
    pinMode(INDICATOR_OUT, OUTPUT);
    
}

void loop()
{
    setTimerInterval();
}

void setTimerInterval( void )
{
    arpSpeedAverage = (analogRead(ARP_SPEED) + 50 * arpSpeedAverage)/51;  // averaging - new value only counts for 16%
    arpSpeedConstrain = constrain(arpSpeedAverage,5,972); // the actual read value of the potentiometer is limited here so the full pot range can be mapped later
    arpSpeed = map(arpSpeedConstrain,5,972,200,10) / 10.0;

    triggerOffset = analogRead(TRIGGER_OFFSET_POTENTIOMETER) / 6;
 

    if (arpSpeed != previousArpSpeed)
    {
        Timer1.setPeriod((float)(1/arpSpeed)*1000000);
        previousArpSpeed = arpSpeed;
    }


    // set trigger low if the trigger has finished
    if (millis() > ( triggerStartTimer + TRIGGER_LENGTH + triggerOffset))
        digitalWrite(TRIGGER_OUT,LOW);
    else
    {
        // set trigger high if the trigger is called and the offset time is reached
        if (millis() > (triggerStartTimer + triggerOffset))
            digitalWrite(TRIGGER_OUT,HIGH);
    }
}

void DelayedTrigger( void )
{
    triggerStartTimer = millis();
}

void internalClockInterrupt()
{
    Indicator();
    DelayedTrigger();
}

void Indicator( void )
{
    digitalWrite(INDICATOR_OUT,!digitalRead(INDICATOR_OUT));
}

Any suggestions of how I might solve this one would be greatly appreciated ?

if you insist on using interrupts, you should declare triggerStartTimer as a volatile and check again (this is needed to avoid code optimisation you want to ensure you always read from memory)

you might still have some challenges as some bytes of triggerStartTimer can start while you are doing your compares so ideally you should stop interrupts, copy the current value of triggerStartTimer, re-enable interrupts and proceed with your code using the copy.

but really there is no real need to use interrupts for this... just a simple state machine using millis() in the loop would do given you are not in need of microsecond timing...

Thanks for the pointer on using Interrupts.