Microapp refactor

[mrquincle]

Microapp refactor so that it uses a single coroutine which yields and is resumed multiple times. The microapp does not return anymore. This works in tandem with https://github.com/crownstone/crownstone-microapp/tree/ble.

[mrquincle]

The code is now - sort of - blocking on the other pull request: https://github.com/crownstone/bluenet/pull/147

The code itself is quite restructured compared to when this was branched of master. The code size of https://github.com/crownstone/bluenet/runs/4549164658?check_suite_focus=true compared to the code size now https://github.com/crownstone/bluenet/runs/5009783399?check_suite_focus=true is pretty much the same.

-- The app is of size 252628 (0x3DAD4)

while it was in Dec. 16 2021:

-- The app is of size 252580 (0x3DAA4)

Darn, forget what I'm saying here. This is probably with microapp support disabled.

The block is not just due to nRF52840 support, but also the ability to send messages through Bluetooth mesh on the nRF52840. If Bluetooth mesh 5.0.0 is working for SDK 17 (and this hardware) it's easier to check if that part of the microapp is now functional as well. (This apparently didn't work, but there's no detailed bug report.)

[mrquincle]

At https://github.com/crownstone/crownstone-microapp/tree/ble/docs an attempt of the current interaction between bluenet and the microapp has been visualized using mermaid.

[vliedel]

I'm very excited for all the improvements!

Some improvements I'd like to see:

• Code style should be fixed, I see a mixed style right now, especially in naming. Also make sure the defines / structs / etc in headers are prefixed with "microapp".
• Make sure to check if calls from the microapp that expect a return value (like CS_MICROAPP_COMMAND_POWER_USAGE) still work.
• Maybe rename callback to microapp interrupt or so to differentiate from the coroutine callbacks.

Improvements for later:

• A lot of "coroutine code" now ended up in the cs_MicroappProtocol files. Move these to cs_DoubleStackCoroutine, as that file is microapp specific anyway, or into a new file.
• Instead of counting the number of callbacks in progress, simply let the microapp return the number of callbacks in progress when acknowledging a callback, and when finishing a callback.

Documentation of the way it works should be improved, I wrote the text below for my own understanding. But that does not even explain interrupts of the main loop of the microapp, and the use of the localQueue.

================= INIT =================

Microapp::init()
    MicroappProtocol::setIpcRam()
        IpcRamData::setRamData(IPC_INDEX_CROWNSTONE_APP)
            .microapp_callback = microapp_callback

Microapp::startApp()
    MicroappProtocol::callApp()
        sharedState.entry = MicroappStorage::getStartInstructionAddress()
            DoubleStackCoroutine::startCoroutine()
                stack_params.coroutineFunction = goIntoMicroapp
                stack_params.args = sharedState
                set_sp(microapp)
                setjmp(microapp)
                set_sp(bluenet)

================= RUN =================

Microapp::tick()
    MicroappProtocol::tickMicroapp()
        MicroappProtocol::callMicroapp()
            DoubleStackCoroutine::nextCoroutine()
                setjmp(bluenet)
                longjmp(microapp)
                -> DoubleStackCoroutine::startCoroutine
                    MicroappProtocol::goIntoMicroapp(sharedState)
                        jumpToAddress(sharedState.entry)
                            microapp::main ...
MicroappProtocol::microapp_callback(buffers)
    sharedState.io_buffer = buffers
    DoubleStackCoroutine::yieldCoroutine()
        setjmp(microapp)
        longjmp(bluenet)
        -> DoubleStackCoroutine::nextCoroutine
            return YIELDING_MICROAPP0
            -> MicroappProtocol::tickMicroapp
                MicroappProtocol::retrieveCommand()
                    MicroappProtocol::handleMicroappCommand()
                        ...

================= EVENTS =================

MicroappProtocol::handleEvent(EVT_DEVICE_SCANNED)
    MicroappProtocol::onDeviceScanned()
        MicroappProtocol::performCallbackIncomingBLEAdvertisement()
            MicroappProtocol::writeCallback()
                MicroappProtocol::callMicroapp()
                    Same-ish as in RUN

[mrquincle]

Arguments to yield and next

We've also talked about the design decision of not passing coargs->coroutine as an argument to yieldCoroutine() and nextCoroutine(). These are namely stored in stack_params->coroutine and can be easily retrieved https://github.com/crownstone/bluenet/blob/microapp-refactor/source/src/util/cs_DoubleStackCoroutine.c#L63

Setting buffer all the time

That means that in https://github.com/crownstone/bluenet/blob/microapp-refactor/source/src/microapp/cs_MicroappProtocol.cpp#L88 we can assign the incoming pointer to the io_buffer from the microapp to stack_params->arg->io_buffer. Actually, this only is required to be done once (afterwards this is very unlikely to change). Maybe it would be more elegant to introduce there one additional opcode argument.

Now

cs_ret_code_t microapp_callback(bluenet_io_buffer_t* io_buffer) {}

To

cs_ret_code_t microapp_callback(uint8_t opcode, void *additionalArgs) {}

Here an opcode, say MICROAPP_UPDATE_BLUENET_IO_BUFFER can be used to cast the additional arguments to a pointer to bluenet_io_buffer_t and write it to stack_params to be available to bluenet. An opcode MICROAPP_SIGNAL_YIELD can be used as a signaling that it wants to yield.

Location of coargs_t

Then, I've decided to store coargs_t sharedState as a variable in https://github.com/crownstone/bluenet/blob/microapp-refactor/source/src/microapp/cs_MicroappProtocol.cpp#L67 but this can be moved into the MicroappProtocol class. This struct should be stored somewhere where it's accessible by all the functions that write to the buffer.

Functions getting buffers as arguments

The buffers are now read from and written to in e.g. https://github.com/crownstone/bluenet/blob/microapp-refactor/source/src/microapp/cs_MicroappProtocol.cpp#L325 and other functions, rather than being given as arguments. It is possible to move it "up the chain".

For example, handleMicroappCommand does actually have a single buffer as argument, but retrieveCommand obtains it directly, The function retrieveCommand is used in writeCallback where it's easy to pass this also as an argument. However, it's also used in tickMicroapp and there we didn't need it (yet). The decision was now to only start to touch this struct in the function that actually needed it, but not in the parent functions. It can be moved up the function chain though. Then the tickMicroapp() would get this as argument and each callback, perhaps even already in handleEvent().

I'm not sure if I'm 100% on board with this. This restructures perhaps a bit easier if we introduce a few more classes to divide the code into more readable sections. I myself see this struct as a class member that should be accessible by the member functions without them having it to pass those buffers around.

[mrquincle]

Mesh message buffer

When looking through https://github.com/crownstone/bluenet/blob/microapp-refactor/source/src/microapp/cs_MicroappCommandHandler.cpp most is fine except for how https://github.com/crownstone/bluenet/blob/microapp-refactor/source/src/microapp/cs_MicroappCommandHandler.cpp#L628 is implemented.

This implements a synchronous read construction from the microapp rather than soft interrupts. The buffer for the mesh messages is stored at the bluenet side. Even if the microapp doesn't use those mesh messages this is created in https://github.com/crownstone/bluenet/blob/microapp-refactor/source/include/microapp/cs_MicroappCommandHandler.h

CircularBuffer<microapp_buffered_mesh_message_t> _meshMessageBuffer;

The burden of this storage should be at the microapp side. If there are multiple microapps this is important as well, because when one microapp has handled a mesh message, that doesn't mean that another microapp has also handled it.

State about buffers on the microapp side

At first it might be attractive to have something like _meshMessageBuffer.full() available, but this runs into similar issues. For example, if bluenet has to maintain if mesh messages buffers are full on the microapp side, it has to do this for all the microapps.

Hence, retrieving slot information at the bluenet side with CS_MICROAPP_COMMAND_SOFT_INTERRUPT_RECEIVED so that bluenet can bail out fast, means that this state information has to be stored per microapp.

It might be just as fast (and less code) to jump into the microapp, discover that it's full, and jump back.

Multiple microapps

In general multiple microapps have not yet been high on the radar. It will necessarily further simplify stuff. The bluenet binary should not maintain state of X microapps. This means that e.g. _emptyInterruptSlots will need to go again. And then there will be no throttling through softInterruptInProgress(). If there is throttling it will be only a maximum number of interrupts sent towards the complete set of microapps (and probably equally distributed).

[mrquincle]

Make sure to check if calls from the microapp that expect a return value (like CS_MICROAPP_COMMAND_POWER_USAGE) still work.

The read mesh messages functionality has been checked. See https://github.com/crownstone/crownstone-microapp/blob/master/examples/mesh_poll.ino

The above comments on that the buffer must be moved into the microapp itself is a point for a future PR.