Some issues LLM found that might be worth taking a look at.

[markg85]

Hi,

I was just playing a bit with LLMs (gemini and mistral). Both have a lot to say about your code :)

There are a couple things that might be worth taking a look at.

1. (gemini and mistral) Integer Overflow: The variable counts is an unsigned 32-bit integer, which means it can hold values from 0 to 4,294,967,295. If your ticker is set to repeat indefinitely (repeat is set to 0xFFFFFFFF) and the ticker's interval is small enough, counts could eventually overflow, leading to undefined behavior.
2. (mistral, code below) Callback Function Execution Time: The ticker does not take into account the time it takes to execute the callback function. If the callback function takes a long time to execute, it could delay the next callback and affect the overall system's performance.
3. (gemini, code below) Efficiency: In the tick() function, there are two conditional checks for resolution within a short timeframe. This could be optimized by storing the appropriate function pointer (e.g., millis() or micros()) based on the resolution during initialization and using that pointer directly.

There's more but these are imho important issues to have a further look at.

Callback execution time fix Note, i'm not entirely sure if this fix is proper. If the callback takes longer then the time between repeats then you'd in theory have a callback being executed while a previous callback hasn't finished yet. Now this isn't multithreaded so that hypothetical case can't occur. Still, you might want to handle what to do if a callback takes longer then the interval.

void Ticker::update() {
    if (tick()) {
        callback();
        lastTime = (resolution == MILLIS) ? millis() : micros();
    }
}

Efficiency: In the tick() function This would be a nice small micro optimization. Gemini spits out this code as better version:

bool Ticker::tick() {
    if (!enabled) return false;

    // Use appropriate function pointer based on resolution
    uint32_t (*getTime)() = (resolution == MILLIS) ? millis : micros;
    uint32_t currentTime = getTime();

    if ((currentTime - lastTime) >= timer) {
        lastTime = currentTime;
        if (repeat == 0 || (repeat - counts == 1 && counts != MAX_COUNTS)) {
            enabled = false;
            status = STOPPED;
        }
        counts++;
        return true;
    }
    return false;
}

I haven't tried these changes myself but the reasoning seems sound to me.