2
\$\begingroup\$

This code is something that I have used for about 3-4 simple embedded systems projects.

The idea is something like this:

  1. This is non-OS based implementation, everything is done in an outer infinite loop. No dynamic memory involved or advised.
  2. It's an interrupt-driven system, polling for peripherals is not used anywhere.
  3. Peripherals like Timer, UART, I2C, etc loads a particular value based on the event, in a Queue-like variable.
  4. Non-trivial tasks like blinking an LED is done in a common Idle task.
  5. Dispatcher loads the function handlers based on the event added in the queue.
  6. (UPDATE) All read/write activities on CPU registers are managed in the respective Interrupt Service Routine. Adding the event in queue is possibly the last thing the code does in the ISR.

Enum for possible events:

typedef enum {

    TASK_EMPTY = 0,
    TASK_INITIALIZE = 1,
    TASK_IDLE,
    TASK_SOMETHING_1,
    TASK_SOMETHING_2,
    TASK_SOMETHING_3,
    TASK_SOMETHING_4,
    TASK_SOMETHING_N,

    MAX_TASKS,
    TASK_INVALID = -1,

} eTasks_t ;

A constant array which contains the event mapped to the handler:

const stTask_t GlobalTasksList[MAX_TASKS] =
{
    { TASK_EMPTY,              Placeholder             },
    { TASK_INITIALIZE,         SystemInitialization    },
    { TASK_IDLE,               Idle                    },
    { TASK_SOMETHING_1,        Handler_1               },
    { TASK_SOMETHING_2,        Handler_2               },
    { TASK_SOMETHING_3,        Handler_3               },
    { TASK_SOMETHING_4,        Handler_4               },
    { TASK_SOMETHING_N,        Handler_N               },
};

I talked about a Queue:

eTasks_t Queue[MAX_TASKS];
eTasks_t qTasksToAdd[MAX_TASKS * 2];

There is a structure which binds an event and it's handler together:

typedef struct stTask {

    eTasks_t TaskID;
    bool (*handler) (void);   // Would return true for success, and false for failure

} stTask_t; 

stTask_t Task;

A Dispatcher function will take care of queued events:

void Dispatcher(void)
{
    while(1)
    {
        // Assign the task handler
        Task.handler = GlobalTasksList[Task.TaskID].handler;

        //Invoke handler
        if(FAILURE == Task.handler())
        {
            //Something went wrong with handler, Error!
            // Error handling

        }

        //Does the task needs a retry, or is a recurring task?
        // Reload if yes, else discard and load the next
        if(bTaskRetry == FALSE)
        {
            Queue_DiscardTask0();
        }

        //Assign the 0th task from Queue
        Task.TaskID = Queue[0];
    }
}

Let's look at an example of adding the event in the Queue from an ISR:

#pragma vector=PORT2_VECTOR
__interrupt void PORT2_Interrupt(void)
{
    // Clear interrupt
    CLR_BIT(P2IFG, 2);  //P2IFG is a CPU register which indicates the Port pin that has generated an interrupt. 
    // Check if the queue is overflowing
    if(qIndexTasksToAdd < MAX_TASKS-1)
    {
        // Add event
        qTasksToAdd[qIndexTasksToAdd++] = TASK_SOMETHING_1;
    }
}

As of now, this strategy works fine without any questionable latency. I have used the same skeleton for systems that could have as many as 20 events lined-up, and it works as per requirement to say the least.

Another advantage of this implementation that I felt is in regards with the efficiency this has when I am debugging an issue. Everything is routed through the handler/dispatcher, so I precisely know where to have a debug point/print setup.

The concern that I have with this implementation is the fixed memory footprint is has. For example, this won't be memory efficient for a LED blinking project.

Any thoughts where I could improve on this?

\$\endgroup\$
  • \$\begingroup\$ Blinking a LED seems like a trivial task to me. \$\endgroup\$ – Reinderien May 11 '18 at 20:07
  • \$\begingroup\$ What's your microcontroller? This much pointer math and function indirection is usually a bad idea in the inside of an ISR. Have you measured the latency incurred? \$\endgroup\$ – Reinderien May 11 '18 at 20:20
  • \$\begingroup\$ This ISR that I have shared is for a TI based MSP430 microcontroller. Note that no indirection is involved in tbe ISR. ISR only sets an event in queue. The indirection and invokation happens in Dispatcher function. \$\endgroup\$ – WedaPashi May 12 '18 at 13:10
3
\$\begingroup\$

.... have as many as 20 events lined-up, and it works as per requirement
Any thoughts where I could improve on this?

Stress test

The queue approach is non-deterministic and would benefit in knowing how much extra CPU is left.

Presently the processor is ample and the cumulative task list is low. There is spare CPU power. Yet let us stress test this design to find out how close and how code handles falling behind.

Add code to detect when Dispatcher() is starving. Did every task get serviced at least once every 0.1s say?

Append a do-nothing task that consumes a x% of CPU time, call it Administratium. Did the queue ever fail to add because it was full? What was the maximum queue usage? Was task latency too great?

Armed with this adjustable idle task, preliminary testing can gauge the degree of margin in the design.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your inputs. "Add code to detect when Dispatcher() is starving. Did every task get serviced at least once every 0.1s say?", I'd say no, I have never made the Dispatcher() to starve. So far the most frequent event that it deals with is an ADC measurement, which is at every 50 msec, LED blinks at every 1 sec, so I'll test it for events lined up with a faster rate w.r.t to 100 msec as you discussed above. About latency, I check various events by toggling a bit, used an oscilliscope, the lag is inconsistent and maximum of range 5-8 microseconds :-) Thank you. \$\endgroup\$ – WedaPashi May 14 '18 at 7:35
2
\$\begingroup\$

Why do you need the stTask_t translation? Can't you directly store function pointers?

Why do you use typedef enum? AFAIK in C enum names live in the same namespace as other types.

How do you pass parameters to event handlers? If you expect the handler to read some register in order to learn what is happening, I can see the following problem. In a long queue of events, later event handlers can read hardware registers after those have changed. I have seen this handled with something like struct params{ int id; union{...} };.

You have not shown the code for adding and removing events from the queue. There could be problems with reentrancy there.

This is minor nitpicking. Function names are supposed to be verbs.

Adding _Noreturn specifier would make the dispatcher easier to read.

\$\endgroup\$
  • \$\begingroup\$ Hey, thanks for attempting it. I have update a section on question, which may make your this concern inapplicable "In a long queue of events, later event handlers can read hardware registers after those have changed.", and I am sorry about that. \$\endgroup\$ – WedaPashi May 11 '18 at 10:49
  • \$\begingroup\$ Thank you for pointing out one hurdle that event handlers can not take arguments. \$\endgroup\$ – WedaPashi May 11 '18 at 10:51
  • \$\begingroup\$ @WedaPashi suppose the interrupt event was more complex than a pin interrupt. For example ADC conversion complete. You want to immediately read the value (as you are doing) and pass it somehow to the handler. \$\endgroup\$ – Vorac May 11 '18 at 10:55
  • \$\begingroup\$ Indeed, very valid. Right now the way this is handled is by a global structure that contains ADC measurements, and ADC ISR simply read the counts and load it into the global variables. The event is then added in the queue and the handler converts counts into an engineering value. \$\endgroup\$ – WedaPashi May 11 '18 at 11:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.