HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Simple and fair scheduler for function calls on Arduino

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
callsschedulerarduinosimplefairfunctionforand

Problem

because Arduino platforms are fairly limited in its capacities, I wrote a small process scheduler. I append a function to an array and define a tickrate. After this tickrate elapses, the function should be called. Additionally I set a small delay. If several function share the same tickrate, this delays hinders them to get called very shortly together. In case, that the serial bus is used, this could avoid write/read blocks.
I never wrote something similiar and wouldn't say that my approach with the delay is elegant. Maybe someone has ideas to enhance this story.

Ads:
  • CAPITALS are #defines



Sample:

Emitter emitMain(&main_loop, MAIN_LOOP_T_MS);
// Prepare scheduler for the main loop ..
_SCHED.addEmitter(&emitMain,  0);
_SCHED.run();


Code *.h:

class Emitter {
public:
  Emitter(void (*pf_foo)(), uint16_t delay = 0);

  bool emit();
  void reset();
  uint16_t getDelay(uint16_t iNum);

private:
  bool bSend;
  uint16_t iDelay;
  void (*pfEmitter)();
};
///////////////////////////////////////////////////////////
// Container for emitter objects
///////////////////////////////////////////////////////////
class Emitters {
private:
  const AP_HAL::HAL *m_pHAL;

  uint8_t   m_iItems; // Current number of items in the arrays below
  uint32_t  m_timerList   [MAX_NO_PROC_IN_SCHED];
  Emitter  *m_functionList[MAX_NO_PROC_IN_SCHED];
  uint16_t  m_tickrateList[MAX_NO_PROC_IN_SCHED];

protected:

  ///////////////////////////////////////////////////////////
  // pEmitters: Array of iSize_N elements
  // iTickRates: The times in ms until the emitter in the array will emit again
  // iTimerList: Array holding the timers for each element
  // iSize_N: The number of emitters in the array
  ///////////////////////////////////////////////////////////
  void scheduler(Emitter **pEmitters, uint16_t *iTickRates, uint32_t *iTimerList, const uint8_t iSize_N);

public:
  Emitters(const AP_HAL::HAL *);

  void addEmitter(Emitter *, uint16_t iTickRate);
  void run();
};


C

Solution

Just a few minor notes: guard clauses (inverting a few conditions and using continue) would make the code flatten and easier to read:

for(uint8_t i = 0; i scheduler->millis() - iTimerList[i];
    if(time getDelay(i) ) {
        continue;
    }
    if(!pEmitters[i]->emit()) {
        continue;
    }
    if(i != (iSize_N - 1) ) { 
        continue;
    }

    // Reset everything if last emitter successfully emitted
    for(uint16_t i = 0; i reset();
    }
    iTimerList[i] = m_pHAL->scheduler->millis();
}


Accessing pEmitters[i] happens three two times. You could create a named local variable for that. The third one is easy to misread (as I did) since both for loops use the same loop variable. It would be more readable if they used different variables or the inner loop was extracted out to a named function.

The function could be called resetEverything (something more conrete would be better). This eliminates the first part of the comment. The second part of the comment could be also a function, isLastEmitterSuccessfullyEmitted which contains the conditions.

for(uint8_t i = 0; i scheduler->millis() - iTimerList[i];
    if(time getDelay(i) ) {
        return false;
    }
    if(!pEmitters[i]->emit()) {
        return false;
    }
    if(i != (iSize_N - 1) ) { 
        return false;
    }
}

void resetEverything(...) {
    // Reset everything if last emitter successfully emitted
    for(uint16_t i = 0; i reset();
    }
    iTimerList[i] = m_pHAL->scheduler->millis();
}


It increases the abstraction level of the code and gives a high level overview for readers/maintainers while the details are hidden in the functions.

Code Snippets

for(uint8_t i = 0; i < iSize_N; i++) {
    uint32_t time = m_pHAL->scheduler->millis() - iTimerList[i];
    if(time <= iTickRates[i] + pEmitters[i]->getDelay(i) ) {
        continue;
    }
    if(!pEmitters[i]->emit()) {
        continue;
    }
    if(i != (iSize_N - 1) ) { 
        continue;
    }

    // Reset everything if last emitter successfully emitted
    for(uint16_t i = 0; i < iSize_N; i++) {
        pEmitters[i]->reset();
    }
    iTimerList[i] = m_pHAL->scheduler->millis();
}
for(uint8_t i = 0; i < iSize_N; i++) {
    if (!isLastEmitterSuccessfullyEmitted(...)) {
        continue;
    }
    resetEverything(...);
}

boolean isLastEmitterSuccessfullyEmitted(...) {
    uint32_t time = m_pHAL->scheduler->millis() - iTimerList[i];
    if(time <= iTickRates[i] + pEmitters[i]->getDelay(i) ) {
        return false;
    }
    if(!pEmitters[i]->emit()) {
        return false;
    }
    if(i != (iSize_N - 1) ) { 
        return false;
    }
}

void resetEverything(...) {
    // Reset everything if last emitter successfully emitted
    for(uint16_t i = 0; i < iSize_N; i++) {
        pEmitters[i]->reset();
    }
    iTimerList[i] = m_pHAL->scheduler->millis();
}

Context

StackExchange Code Review Q#41954, answer score: 5

Revisions (0)

No revisions yet.