RFC: Use correct command processor function type

[wegendt-bosch]

Introduction

Our event execution model right now is as follows: when an event occurs a handler function is enqueued. The command processor calls this handler when it deems it right.

The command processor expects functions to be of the following type:

void (*CmdProcessor_Func_T)(void *param1, uint32_t param2)

or for people more familiar with arrow syntax:

type CmdProcessor_Func_T = (Ptr, Uint32) → ()

Current Situation

In Mita enqueued functions are always of type

Retcode_T (*Mita_EventHandler_T) (void* userParameter1, uint32_t userParameter2)

respectively

type Mita_EventHandler_T = (Ptr, Uint32) → Retcode_T

This obviously is not the same. It doesn't corrupt the stack since the compiler always allocates 4 byte for the return type even if the called function returns void, but in principle this is undefined behaviour and could break in the future.

Possible solutions

There are two ways to remedy this:

• change enqueueable functions to not return anything
• provide a wrapper function for enqueueable functions

Change enqueueable functions

This solution is certainly the cleaner solution. Instead of generating for example

Retcode_T HandleEvery100Millisecond1(void* userParameter1, uint32_t userParameter2)
{ /* ... */ }

we would generate

void HandleEvery100Millisecond1(void* userParameter1, uint32_t userParameter2)
{ /* ... */ }

However this would require control flow in those functions to be implemented depending on their context like they are for try ... catch: if an exception is not caught we need to immediatly do whatever we do when an event handler would throw (see #34).

Wrapper for enqueueable functions

I see two ways to provide a wrapper to such functions. One is to provide one wrapper per function, the other is to not enqueue event handlers but to always enqueue the same function and use the void* argument to pass the real handler. The first way is more verbose, the second is way cooler. These would look like the following code fragments:

// wrapper per handler
void _HandleEvery100Millisecond1(void* p1, uint32_t p2) {
    Retcode_T exception = HandleEvery100Millisecond1(void* p1, uint32_t p2);
    if(exception != NO_EXCEPTION) {
        // restart the platform, print stuff, etc.
    }
}
Retcode_T HandleEvery100Millisecond1(void* p1, uint32_t p2)
{ /* ... */ }

{   // enqueue:
    CmdProcessor_Enqueue(&Mita_EventQueue, (CmdProcessor_Func_T) Wrapper, Mita_initialize, UINT32_C(0));
}

void Wrapper(Mita_EventHandler_T func, uint32_t unused) {
    Retcode_T exception = func(null, 0);
    if(exception != NO_EXCEPTION) {
        // restart the platform, print stuff, etc.
    }
}

Retcode_T HandleEvery100Millisecond1(void* p1, uint32_t p2)
{ /* ... */ }

This is easier to implement than the first solution but it is most certainly slower since the compiler can't possibly comprehend what we are trying to do, especially if we decide to go for the second way.

[csweichel]

There is a third option (which in my mind is the preferable one): to have an event loop which supports event handlers which can return retcodes. It does not necessarily have to be the CommandProcessor, but could be something tailor-made for Mita (modelled after the the CmdProcessor). Personally I feel like we have to jump through hoops here just to support the idiosyncrasy of the platform.

We had a discussion with Lars a while ago to support an API like this in the CmdProcessor but decided against it as we were afraid it would break API. However, it would seem that for the platform this would be a great feature, if the CmdProcessor would support handlers with (Ptr, Uint32) → Retcode_T and allow the registration of a common error handler which is called if any cmd handler returns a retcode other than Retcode_OK.

[wegendt-bosch]

OTOH jumping through hoops so that we generate correct and safe code is exactly what Mita is all about instead of trying to better the C API (which is impossible), no?