lancaster-university / codal-microbit-v2

CODAL target for the micro:bit v2.x series of devices
MIT License
44 stars 52 forks source link

Different V1/V2 behaviour of printChar in two fibers #95

Closed martinwork closed 1 year ago

martinwork commented 3 years ago

With two fibers displaying 0,1,2 and 3,4,5, V1 shows 0,1,2,3,4,5 V2 shows 0,1,2,5

Related: https://github.com/microsoft/pxt-microbit/issues/3870

Hex: test-forever.zip

#include "MicroBit.h"

MicroBit uBit;

int interval = 750;

void loopA()
{
  while (true)
  {
    uBit.display.printChar('0', interval);
    uBit.display.printChar('1', interval);
    uBit.display.printChar('2', interval);
    uBit.sleep(20);
  }
}

void loopB()
{
  while (true)
  {
    uBit.display.printChar('3', interval);
    uBit.display.printChar('4', interval);
    uBit.display.printChar('5', interval);
    uBit.sleep(20);
  }
}

int  main()
{
    uBit.init();

    create_fiber(loopA);
    create_fiber(loopB);

    release_fiber();
}
martinwork commented 3 years ago

Here's the difference...

https://github.com/lancaster-university/microbit-dal/blob/77d679c87f3550ab01f7ee3f5de3c295d8161ab1/source/drivers/MicroBitDisplay.cpp#L447

void MicroBitDisplay::waitForFreeDisplay()
{
    // If there's an ongoing animation, wait for our turn to display.
    while (animationMode != ANIMATION_MODE_NONE && animationMode != ANIMATION_MODE_STOPPED)
        fiber_wait_for_event(MICROBIT_ID_NOTIFY, MICROBIT_DISPLAY_EVT_FREE);
}

https://github.com/lancaster-university/codal-core/blob/master/source/drivers/AnimatedDisplay.cpp#L274

void AnimatedDisplay::waitForFreeDisplay()
{
    // If there's an ongoing animation, wait for our turn to display.
    if (animationMode != ANIMATION_MODE_NONE && animationMode != ANIMATION_MODE_STOPPED)
        fiber_wait_for_event(DEVICE_ID_NOTIFY, DISPLAY_EVT_FREE);
}

This fix was made in DAL: https://github.com/lancaster-university/microbit-dal/commit/77d679c87f3550ab01f7ee3f5de3c295d8161ab1

martinwork commented 3 years ago

Created PR https://github.com/lancaster-university/codal-core/pull/140

martinwork commented 1 year ago

@JohnVidler As noted in the PR, this behaves differently on V1, V2 and the MakeCode simulator.

V2 is definitely wrong, and the PR changes V2 to match V1.

The simulator allows a fiber switch after each display call, but V1 only switches in the implicit sleep at the end of the forever loop. V1 is right. because a program can insert sleeps to get the extra fiber switches, but could not stop them if the extra switches were the default. Consider this example https://makecode.microbit.org/_87wWRrYLM7bg.

microbit-carlos commented 1 year ago

@martinwork once the latest CODAL tag is out let's retest and list here the behaviours of the sim vs v1 vs v2.

microbit-carlos commented 1 year ago

The https://github.com/lancaster-university/codal-core/pull/140 patch didn't fix the issue as expected. @finneyj will look into this before the next CODAL tag is release.

microbit-carlos commented 1 year ago

This issue, to match V1 and V2 behaviour, has been resolved with the https://github.com/lancaster-university/codal-core/pull/140 patch.

However, we have an open question about changing this behaviour to match the MakeCode simulator.

@finneyj will create a new issue to describe the two behaviours.

microbit-carlos commented 1 year ago

Issue https://github.com/lancaster-university/codal-microbit-v2/issues/253 opened summarising the current behaviour and possible options, so we can close this issue.