Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Erasing data logging storage in a fiber while plotting a bar graph in the display crashes programme #362

Open
microbit-carlos opened this issue Aug 7, 2023 · 5 comments
Assignees
Labels
Milestone

Comments

@microbit-carlos
Copy link
Collaborator

microbit-carlos commented Aug 7, 2023

This programme erases the DAPLink storage in a separate fiber, while the main one updates the display with a bar graph using random numbers.

  • If clearDataLoggingStorage is not run in a fiber, it works
  • If uBit.display.print is used instead of plotBarGraph, it works
  • The example as shown here crashes after a couple of bar graph iterations
#include "MicroBit.h"

MicroBit uBit;

static void clearDataLoggingStorage() {
    uBit.log.clear(true);
    uBit.audio.soundExpressions.playAsync("hello");
}

static void plotBarGraph(uint32_t value, int high) {
    float v = (float)value / (float)high;
    float dv = 1.0 / 16.0;
    float k = 0;
    for (int y = 4; y >= 0; --y) {
        for (int x = 0; x < 3; ++x) {
            if (k > v) {
                uBit.display.image.setPixelValue(2 - x, y, 0);
                uBit.display.image.setPixelValue(2 + x, y, 0);
            } else {
                uBit.display.image.setPixelValue(2 - x, y, 255);
                uBit.display.image.setPixelValue(2 + x, y, 255);
            }
            k += dv;
        }
    }
}

int main() {
    uBit.init();

    create_fiber(clearDataLoggingStorage);

    for (int i = 0; i < 500;i++) {
        int value = uBit.random(10);
        uBit.serial.printf("value: %d\n", value);
        plotBarGraph(value, 10);
        // Oddly enough, uBit.display.print instead of plotBarGraph does not crash
        //uBit.display.print(value);
        uBit.sleep(10);
    }

    uBit.display.print("!");
    for (;;)
        uBit.sleep(1000);
}

MICROBIT.hex.zip


Update: Replicable only when when adding "DMESG_ENABLE": 1 to codal.json, and with gcc v10.3 (gcc v12 works).

#362 (comment)

@microbit-carlos
Copy link
Collaborator Author

microbit-carlos commented Aug 7, 2023

Having a quick look with the debugger call stack, the hard fault handler is reached from:

image image image image

@microbit-carlos
Copy link
Collaborator Author

Looks like this is replicable with gcc v10.3, but not with gcc v12.
And only when adding "DMESG_ENABLE": 1 to codal.json.

@JohnVidler
Copy link
Collaborator

With the merge of PR 165 in codal-core we can now build without spurious compiler warnings on gcc 12 so the recommendation here is to move to this compiler, as the bug looks to be caused by incorect generation of the copy-on-return constructor when returning a ManagedBuffer under very specific circumstances (these, in fact!), and is rather beyond the remit of CODAL to fix.

I'm going to close this for now, but feel free to re-open if this needs to be discussed further.

@microbit-carlos
Copy link
Collaborator Author

microbit-carlos commented Sep 7, 2023

If that's the case, can't we code the move constructor and workaround the gcc issue?
MakeCode and CODAL users will still be using older GCC versions than v12, so if possible I think it'd be good to try to resolve this issue.

@microbit-carlos microbit-carlos modified the milestones: v0.2.60, v0.2.61, v0.2.62 Sep 15, 2023
@microbit-carlos microbit-carlos added p2 and removed p1 labels Oct 12, 2023
@microbit-carlos microbit-carlos modified the milestones: v0.2.64, v0.2.65 Nov 3, 2023
@microbit-carlos microbit-carlos modified the milestones: v0.2.65, v0.2.66 Nov 14, 2023
@microbit-carlos
Copy link
Collaborator Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants