crownstone / bluenet

Bluenet is the in-house firmware on Crownstone hardware. Functions: switching, dimming, energy monitoring, presence detection, indoor localization, switchcraft.
https://crownstone.rocks
91 stars 62 forks source link

Bug: enabling a microapp requires bluetooth #146

Closed mrquincle closed 2 years ago

mrquincle commented 2 years ago

The microapp could be uploaded and made to run using a jlink towards a development kit without the need for any bluetooth commands. The information is all in the binary header that comes with the microapp.

Dependency on state

However, what seems to happen now is that a dependency has been introduced on a state object. See https://github.com/crownstone/bluenet/blob/master/source/src/microapp/cs_Microapp.cpp#L56.

void Microapp::loadApps() {
    // Loop over every app index, as an app might've been uploaded via jlink.
    cs_ret_code_t retCode;
    for (uint8_t index = 0; index < MAX_MICROAPPS; ++index) {
        loadState(index);
        retCode = validateApp(index);
        if (g_AUTO_ENABLE_MICROAPP_ON_BOOT && retCode == ERR_SUCCESS) {
            LOGMicroappInfo("Enable microapp %u", index);
            enableApp(index);
        }
        storeState(index);
        startApp(index);
    }
}

Get info from flash

When a microapp is loaded through jlink, there's no state information available. In contrast, this has to be obtained from flash.

This can be found in handleGetInfo, https://github.com/crownstone/bluenet/blob/master/source/src/microapp/cs_Microapp.cpp#L207 but this function is purely event-driven (only used in handling the bluetooth commands).

    MicroappStorage & storage = MicroappStorage::getInstance();
    microapp_binary_header_t appHeader;
    for (uint8_t index = 0; index < MAX_MICROAPPS; ++index) {
        storage.getAppHeader(index, appHeader);
        info->appsStatus[index].buildVersion = appHeader.appBuildVersion;
        info->appsStatus[index].sdkVersion.major = appHeader.sdkVersionMajor;
        info->appsStatus[index].sdkVersion.minor = appHeader.sdkVersionMinor;
        memcpy(&(info->appsStatus[index].state), &(_states[index]), sizeof(_states[0]));
    }

This requires some rewriting to be able to load microapps automatically.

Naming

The blame is on me on having the previous functionality called AUTO_ENABLE_MICROAPP_ON_BOOT which seems to only enable an app on boot (without requiring extra user interaction). The reason for this functionality was however not limited to just enabling apps, but to be able to automatically do everything regarding a microapp to make it run (be it validation, enabling, setting state, setting it to run, etc.).

We can add AUTO_VALIDATE_MICROAPP_ON_BOOT or we can just have one auto command for a microapp. Feel free to suggest a different name.

Rationale

Some rationale to be able to upload microapps directly:

Fix

The fix is to get the info from flash when AUTO_XXX is set (see get info from flash paragraph above).

mrquincle commented 2 years ago

Mmm, misread. https://github.com/crownstone/bluenet/blob/master/source/src/microapp/cs_Microapp.cpp#L71 does actually continue with

    // TODO: we only need this because you may have flashed the microapp on the chip.
    MicroappStorage & storage = MicroappStorage::getInstance();
    _states[index].hasData = !storage.isErased(index);

And it's because I was flashing the microapp to the wrong location that it didn't pick it up. The reason why I did the latter is because this is an nRF52840 and it expects the microapp at location 0x000DD000 rather than 0x00069000. This has to be adjusted in the upload files for the microapp in config.mk and ./include/nrf_symbols.ld.