spi.write() (in C, in Makecode extension) crashes the Microbit?

[MTKilpatrick]

I have a situation in which a write to SPI seems to crash the Microbit. I’ve been looking at this for an hour or two now but cannot see what is wrong.

My Makecode extension to drive a Nokia 3310 LCD screen (84 x 48 pixels) is in this repository
https://github.com/MTKilpatrick/pxt-nokialcd

The TS file has shims for lots of functions that I have transferred to the .CPP file to speed up all the graphics. I’m transferring all the SPI routines to C as well.

The extension beings by callinginit()in JS which in turn calls SPIinit() (in the CPP) to initialise the SPI port. init() then sets up the LCD display with several commands - these are determine by setting Digital Pin16 to low (LCD_CMD). Data writes to the screen are performed when P16 is high (LCD_DAT). The chip select is performed by P12 and reset by P8.

The first set-up call is writeFunctionSet() then lcdExtenedFunctions() which sets the display into a command mode in order to set up a few parameters. writeFunctionSet() exists in both the JS and C files.

This extension makes the Microbit hang up if I use the shim=nokialcd::writeFunctionSet to make it use the C version of the function. If I use the JS version by removing the shim, the whole thing works.

If you look at the code in both the TS and CPP files, writeFunction() is identical. It merely sets P12 and P16 to the appropriate states, calls spi.write() with a value and then reverts P12 and P16.

Please note that spi.write() is also used in the function writeSPIBuf() in the CPP file - this transfers the 504-byte buffer to the screen. This function works and I have tested it.

Unless I comment out thespi.write(0x20 | (v << 1) | (h & 1));line in writeFunctionSet, the init() hangs up and does not exit from the call to writeFunctionSet, Line 72 of nokialcd.ts.

[MTKilpatrick]

Update: The spi.write() function in C only works if, beforehand, I have done at least one call to pins.spiWrite() in the JS code.

I tested this by removing all of the set-up calls in init() after initialising the SPI port and then making a call to writeSPIBuf() (in C) to push the 504-byte buffer to the screen. This operation did not complete. It only completed after I uncommented a minimum of one JS function call that writes a byte to the SPI.

The LCD screen does not use the MISO pin. Therefore there is no interaction with the screen that could be affected by omitting the set-up calls within init().

[MTKilpatrick]

I have finally discovered what the curious problem is, and it makes no sense to me. Surely this is a bug?

When I successfully no longer have SPI write operations in the Javascript part of my extension, having moved them all to C, I find that the thing crashes at the init() stage unless I put either or both a pins.spiFormat() or pins.spiFrequency() calls in the init() function in Javascript:

export function init(): void {
    pins.spiFormat(8, 0)
    pins.spiFrequency(1000000)
    SPIinit()
}

Even though in the CPP file I have this:

namespace nokialcd {
   SPI spi(mbit_p15, mbit_p14, mbit_p13);
    DigitalOut LCD_CE(mbit_p12);
    DigitalOut LCD_RST(mbit_p8);
    DigitalOut LCD_DC(mbit_p16);
   //%
       void SPIinit() {
        LCD_CE = 1;
        LCD_RST = 0;
        spi.format(8,0);
        spi.frequency(1000000);
....and so on....

...which clearly inititalises the SPI device in C. Why does it need to be either formatted or frequency-set in the JS too? That makes no sense to me.

[martinwork]

It makes no sense to me either, but I hope this helps...

I searched the MakeCode source for spiFrequency.

In pins.cpp, pins.spiFrequency() calls allocSPI(), which calls new SPI(MOSI, MISO, SCK);

The extension cpp file uses another SPI object defined by SPI spi(mbit_p15, mbit_p14, mbit_p13);

I would remove that and use the pins.spiXXX functions instead. I think MOSI, MISO and SCK are 15,14 and 13 by default.

[MTKilpatrick]

I can understand that the SPI device is not initialised by default - and therefore isn't visible as any uBit.somethingorother() functions in C - in order to keep pins 13-15 free for GPIO use by default. The need to have both and C++ SPI set-up calls is a bit puzzling, though. There are now no other SPI calls in my JS. It's all in C++ and it works.

Unfortuntely, executing SPI calls from the Javascript is very time-consuming - I can save 15% of the run-time by using C++ to deliver all 504 bytes of a buffer to the SPI device simply because even a simple FOR loop in JS is not efficiently coded. Therefore I have ported to C++ absoutely everything in my extension for driving the 84x48 pixel Nokia 3310 LCD screen.

Earlier I submitted this issue: https://github.com/lancaster-university/microbit-dal/issues/464 while I was trying to get to grips with C++. @mmoskal said he would be adding a pins.spiTransfer(buffer) function to the microbit, which would be good.

I really hope the equivalent function will be available to call from a cpp extension too?

[martinwork]

I may be missing some reason why this is the wrong thing to do, but if you put the declarations below at the top of your CPP file, you can delete the SPI object and use the pins functions. allocSPI() gives a pointer to call SPI::write directly. I can't test if it actually works, but I seem to be able to remove the pins call from the ts file without it crashing. I initially included a declaration for spiPins, but got complaints about DigitalPin. Don't know why.

namespace pins
{
    extern SPI* allocSPI();
    extern int  spiWrite(int value);
    extern void spiFrequency(int frequency);
    extern void spiFormat(int bits, int mode);
}
using namespace pins;

[MTKilpatrick]

Thank you. It's a bit beyond my understanding of C++ and so by trial and error I found it could just call spiWrite() and not pins.spiWrite() or something else. However, the purpose of extern is more or less obvious.

I then deleted the seemingly unnecessary spiFormat() from the TS file and it did not crash the Microbit as before.

However, I'm a bit surprised that using this method rather than original SPI object makes my buffer transfer (a FOR loop with spiWrite(bytearray->data[i]); in it) ten percent slower than it was. I'm assuming this must be because of an additional layer of function call nesting introduced as a result of this method?

For now I will leave your method in as it certainly "looks better".

Hopefully we'll see the spiTransfer() function at a later date.

[martinwork]

Where typescript says "declare namespace pins" and "pins.", C++ says "namespace pins" and "pins::". And "using namespace pins" removes the need for "pins::".

spiWrite is better than spi->write() for future compatibility. Even better, spiTransfer( bytebuffer, NULL) is now supported in the MakeCode beta.

The declaration would be
extern void spiTransfer(Buffer command, Buffer response);

An unexpectedly large jump in the loop time may be due to a small difference in the code causing loop iterations to contain extra regular interrupts. I wonder what happens to the transfer times if the micro:bit screen is disabled (better?), or if Bluetooth is connected (worse?).

[MTKilpatrick]

I've tested that on the beta. spiTransfer() it's 10% faster for a buffer transfer than a C++ FOR loop with spiWrite(), so that's good, thank you very much.

I think it's a shame that there isn't a more native SPI buffer transfer routine which could eliminate more of the overheads from repeatedly calling spiWrite() through however many layers of abstraction: for a receiving device (eg LCD screen) which is capable of accepting an unbroken stream of bits and which doesn't deliver data back again, it would be lovely to be able to get as close as possible to an unbroken stream of bits delivered by the Microbit.

I guess there would be a difference in circumstances when the Microbit needs to read the receive-buffer.