Radio: Last 3 bytes (header size?) not sent/received

[microbit-carlos]

The documentation and the source code indicate that the maximum "packet" size is 32 bytes: https://lancaster-university.github.io/microbit-docs/ubit/radiodatagram/

However, if we try to send 32 bytes of data, the last 3 bytes are not sent or received (not 100% sure which one it is).

The following programme uses uBit.radio.datagram to:

• Send 32 bytes via radio
• Receive PacketBuffers and print their contents to serial

MICROBIT.hex.zip

#include "MicroBit.h"

#define PACKET_SIZE 32

MicroBit uBit;

void on_radio_data(MicroBitEvent e) {
    PacketBuffer radio_packet = uBit.radio.datagram.recv();
    uBit.serial.send("RX: ");
    for (int i = 0; i < radio_packet.length(); i++) {
        uBit.serial.printf("%d ", radio_packet[i]);
    }
    uBit.serial.send("\r\n");
}

int main() {
    uBit.init();
    uBit.radio.enable();

    uBit.messageBus.listen(MICROBIT_ID_RADIO, MICROBIT_RADIO_EVT_DATAGRAM, on_radio_data);

    uint8_t radio_tx[PACKET_SIZE];
    for (int i = 0; i < PACKET_SIZE; i++) {
        radio_tx[i] = i;
    }

    while (true) {
        uBit.sleep(500 + uBit.random(500));
        int result = uBit.radio.datagram.send(radio_tx, PACKET_SIZE);
        if (result != MICROBIT_OK) {
            target_panic(123);
        }
    }
}

Output:

RX: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 0 0 0 

Not to sure how the low level radio works (register wise), but looks like the data size check for the MicroBitRadioDatagram::send(uint8_t *buffer, int len) function might not be quite right? https://github.com/lancaster-university/codal-microbit-v2/blob/348afc4c2de5c602b40b79ae63c377909404c96f/source/MicroBitRadioDatagram.cpp#L120-L123

MICROBIT_RADIO_MAX_PACKET_SIZE = 32 MICROBIT_RADIO_HEADER_SIZE = 4

So len > MICROBIT_RADIO_MAX_PACKET_SIZE + MICROBIT_RADIO_HEADER_SIZE - 1) would be the same as if (32 > 32 + 4 -1). Should that be len > (MICROBIT_RADIO_MAX_PACKET_SIZE - (MICROBIT_RADIO_HEADER_SIZE - 1)) , so that the maximum data the user can send is 29 bytes?

Or if the user is meant to be able to send 32 bytes, should that check be len > MICROBIT_RADIO_MAX_PACKET_SIZE? As the header data is added on top of that? In this case, we still have the issue that the last 3 bytes are not being sent / received.

[microbit-carlos]

Sending MICROBIT_RADIO_MAX_PACKET_SIZE to 64 and sending 64 bytes also results in the last 3 bytes missing:

RX: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 0 0 0

And setting MICROBIT_RADIO_MAX_PACKET_SIZE to 64, but sending only 32 bytes, we get all the data correctly:

RX: 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 

[martinwork]

The configuration allows for MICROBIT_RADIO_MAX_PACKET_SIZE bytes to be sent and received. https://github.com/lancaster-university/codal-microbit-v2/blob/master/source/MicroBitRadio.cpp#L335

But that doesn't allow for the 3 bytes before the payload in the FrameBuffer struct. https://github.com/lancaster-university/codal-microbit-v2/blob/master/inc/MicroBitRadio.h#L88

I think the DAL has the same problem.

[microbit-carlos]

Thanks Martin!

I had tried to increase the max packet size in NRF_RADIO->PCNF1 in that line before, but it didn't seem to work on that occasion. However, as I write this I realise that for some reason some of my other changes I tried in the radio code didn't seem to be built in my project unless I did a clean build. So I think it's definitely worth trying again, I'll give it a go once I find some time.

Additionally, it looks like MakeCode is only sending 29 bytes as well, since their header size is 9 bytes and the max user payload 20 bytes: https://github.com/microsoft/pxt-common-packages/blob/dbecae95b46eca9014a9b7d4a4c19ffdd02bb0f6/libs/radio/radio.ts#L23-L24

So I guess that might points at DAL/CODAL never been able to send the full 32 bytes? I haven't tried it in V1 yet though, so I cannot confirm if it behaves the same. It'd be good to run the same test code on V1 to check that.

[martinwork]

I think the sizes haven't changed, and problem has always been there in DAL and CODAL. The configuration and the FrameBuffer payload size are incompatible. As you noticed, some of the size checks are wrong, as are comments like this. https://github.com/lancaster-university/codal-microbit-v2/blob/master/inc/MicroBitRadioDatagram.h#L98

Will packets just be truncated if 2 micro:bits use different configured sizes?

[microbit-carlos]

I've run this a test again, changing NRF_RADIO->PCNF1 = 0x02040000 | MICROBIT_RADIO_MAX_PACKET_SIZE to NRF_RADIO->PCNF1 = 0x02040000 | (MICROBIT_RADIO_MAX_PACKET_SIZE + 3) and that worked 🎉

So I guess this confirms we are basically packing data in a FrameBuffer with the idea to send a max of 35 bytes (3 header bytes + 32 user payload bytes), but then configuring the peripheral to cut off at byte 32.

The first byte in the data received is still 35, as that's what the sender sends in the FrameBuffer. Then the PacketBuffer provided by MicroBitRadioDatagram calculates the "user payload" length to be 35 - (4-1) = 32: https://github.com/lancaster-university/codal-microbit-v2/blob/1370026f4bfbf96b204fa901f98d1f166f91b078/source/MicroBitRadioDatagram.cpp#L101

So even if the radio didn't actually receive the last 3 bytes, the PacketBuffer the user gets back says it has received 32 bytes, so the last three bytes we see are likely the default value in the buffer.

---

For additional info, the packet format is:

Where CODAL has configured S0 and S1 to be zero bytes, so basically it's length + payload. The FrameBuffer then still prepends 3 bytes at the front of the payload as DAL/CODAL header with the version, group, and protocol values https://github.com/lancaster-university/codal-microbit-v2/blob/1370026f4bfbf96b204fa901f98d1f166f91b078/inc/MicroBitRadio.h#L88-L98

[microbit-carlos]

Will packets just be truncated if 2 micro:bits use different configured sizes?

The rx FrameBuffer is big enough to store all the data, and MicroBitRadioDatagram calculates the length of the user payload received based on that first byte value, which has always been "user-payload-size + 3", so I don't think it'll cause any issues to fix this.

@JohnVidler did point out that we need to double check what MakeCode might be doing on top of this, in case they were accidentally, or by design, counting with the last bytes of the data "received" always being set to zero. Specially in cases where sending a string, as the zero at the end could be treated as a null termination. If that was the case we should consider that a MakeCode bug to fix, but we do need to check nothing is broken if this is fixed.