lancaster-university / microbit-dal

http://lancaster-university.github.io/microbit-docs
Other
254 stars 130 forks source link

Messagebus patch #380

Closed jamesadevine closed 6 years ago

jamesadevine commented 6 years ago

In the runtime, we currently observe a "one handler per event and function" rule when adding and ignoring listeners; the message bus does not observe cb_arg (callback argument) as a differentiator between listeners. This has lead to MessageBus functionality duplication at the MakeCode layer (the MakeCode layer receives all events and invokes the correct handler). Effectively, in the MakeCode layer the source and value of an event is the key, and the cb_arg is the value. What this means is that only one function can be added per event in TypeScript, limiting the ability to write additional drivers for things like the radio entirely in TypeScript.

The same effect can be seen in C++. To observe, simply compile the code snippet bellow:

#include "MicroBit.h"

MicroBit uBit;

MicroBitEvent events[3] = {MicroBitEvent(500,400,CREATE_ONLY),MicroBitEvent(500,400,CREATE_ONLY), MicroBitEvent(900,700,CREATE_ONLY)};

void some_cb(MicroBitEvent e, void* payload)
{
    uBit.display.scroll((char *)payload);
}

void one_more_cb(MicroBitEvent e, void* payload)
{
    uBit.display.scroll((char *)payload);
}

void a_click(MicroBitEvent)
{
    events[0].fire();
}

void b_click(MicroBitEvent)
{
    events[1].fire();
}

int main()
{
    // Initialise the micro:bit runtime.
    uBit.init();

    uBit.messageBus.listen(events[0].source, events[0].value, some_cb, (void*)"ABC");
    uBit.messageBus.listen(events[1].source, events[1].value, some_cb, (void*)"123");

    uBit.messageBus.listen(MICROBIT_ID_BUTTON_A, MICROBIT_BUTTON_EVT_CLICK, a_click);
    uBit.messageBus.listen(MICROBIT_ID_BUTTON_B, MICROBIT_BUTTON_EVT_CLICK, b_click);

    release_fiber();
}

The snippet above scrolls "ABC" on either button press, however, given that the callback argument is different, the above snippet should scroll "ABC" "123" on button a or b .

This PR introduces the ability to differentiate listeners on their cb_arg in addition to the source and id they are assigned. Implicitly, this means that multiple listeners can now have the same source and id. Therefore, in addition I also introduce a callback function that is invoked whenever a listener is deleted so that in the case multiple listeners are deleted, the application can take note.

Here's a simple test program after checking out this PR:

#include "MicroBit.h"

MicroBit uBit;

int shakeCount = 0;

const char* abc = "ABC";

MicroBitEvent events[3] = {MicroBitEvent(500,400,CREATE_ONLY),MicroBitEvent(500,400,CREATE_ONLY), MicroBitEvent(900,700,CREATE_ONLY)};

void listener_dereg(MicroBitListener* l)
{
    uBit.serial.printf("REM src: %d, val: %d, cb_arg: %p", l->id, l->value, l->cb_arg);
}

void some_cb(MicroBitEvent e, void* payload)
{
    uBit.display.scroll((char *)payload);
}

void one_more_cb(MicroBitEvent e, void* payload)
{
    uBit.display.scroll((char *)payload);
}

void a_click(MicroBitEvent)
{
    events[0].fire();
}

void b_click(MicroBitEvent)
{
    events[1].fire();
}

void a_b_click(MicroBitEvent)
{
    if (shakeCount == 0)
        uBit.messageBus.ignore(events[0].source, events[0].value, some_cb, (void*)abc);

    shakeCount++;
}

int main()
{
    // Initialise the micro:bit runtime.
    uBit.init();

    uBit.messageBus.setListenerDeletionCallback(listener_dereg);
    uBit.messageBus.listen(events[0].source, events[0].value, some_cb, (void*)abc);
    uBit.messageBus.listen(events[1].source, events[1].value, some_cb, (void*)"123");

    uBit.messageBus.listen(MICROBIT_ID_BUTTON_A, MICROBIT_BUTTON_EVT_CLICK, a_click);
    uBit.messageBus.listen(MICROBIT_ID_BUTTON_B, MICROBIT_BUTTON_EVT_CLICK, b_click);
    uBit.messageBus.listen(MICROBIT_ID_BUTTON_AB, MICROBIT_BUTTON_EVT_CLICK, a_b_click);

    release_fiber();
}

This test program will:

1) Scroll "abc" "123" when either button a or b is pressed. 2) After pressing button AB, only "123" will scroll.

You can also remove all listeners by replacing:

uBit.messageBus.ignore(events[0].source, events[0].value, some_cb, (void*)abc);

with a less specific variant:

uBit.messageBus.ignore(events[0].source, events[0].value, some_cb);

@tballmsft @jaustin @mmoskal @finneyj

jamesadevine commented 6 years ago

P.S. this is built off of the dal-integration branch.

jamesadevine commented 6 years ago

The messagebus-patch branch can be used for testing from https://github.com/lancaster-university/microbit/compare/dal-integration...messagebus-patch

finneyj commented 6 years ago

Thanks @jamesadevine

Looks good. Nice rationale. All those white space changes put me off doing a quick review. ;)

jamesadevine commented 6 years ago

@finneyj My editor has an aversion for tabs... I wonder how they got there in the first place? :wink:

finneyj commented 6 years ago

cheeky. ;)