lancaster-university / codal-microbit-v2

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

Radio setFrequencyBand only takes effect when sending #245

Closed martinwork closed 1 year ago

martinwork commented 1 year ago

Arising from support ticket https://support.microbit.org/helpdesk/tickets/59660 (private)

On V2 only (V1 is OK), setFrequencyBand only takes effect when sending.

MakeCode project: https://makecode.microbit.org/_8uh0m6DEJLeX

Universal hex from code below: test-radio-band.zip

Flash to 2 micro:bits. They both start on the default frequency band 7.

Press A on one micro:bit to make it start sending R[band] Data is displayed on receiver OK Press B on sender to set band to 8 No data is displayed on receiver OK Press B on receiver to set band to 8 No data is received on receiver Shows band not changed on receiver Press B on sender to set band to 7 Data is displayed on receiver Confirms band was not changed on receiver Press B on sender to set band to 8 Press A on receiver to start it sending Data is displayed on both Previous band change activated by sending

With both sending, they can receive on either band


#include "MicroBit.h"

MicroBit uBit;

int send = 0;
int band = 7;

void onButtonA(MicroBitEvent e)
{
    send = !send;
    uBit.display.scroll( "S" + ManagedString( send));
}

void onButtonB(MicroBitEvent e)
{
    band = band == 7 ? 8 : 7;
    uBit.radio.setFrequencyBand( band);
    uBit.display.scroll( "B" + ManagedString( band));
}

void onRadioDatagram(MicroBitEvent e)
{
    PacketBuffer p = uBit.radio.datagram.recv();
    ManagedString s( (const char *) p.getBytes(), p.length());
    uBit.display.scroll(s);
}

int main(void)
{
    uBit.init();
    uBit.radio.enable();
    uBit.radio.setGroup(1);
    uBit.radio.setFrequencyBand( 7);

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

    uBit.messageBus.listen( MICROBIT_ID_BUTTON_A,  MICROBIT_BUTTON_EVT_CLICK, onButtonA);
    uBit.messageBus.listen( MICROBIT_ID_BUTTON_B,  MICROBIT_BUTTON_EVT_CLICK, onButtonB);

    while(1)
    {
        if ( send)
            uBit.radio.datagram.send("R" + ManagedString( band));

        uBit.sleep(3000);
    }       
}
martinwork commented 1 year ago

CODAL MicroBitRadio doesn't reset radio in setFrequencyBand like DAL does. https://github.com/lancaster-university/codal-microbit-v2/blob/master/source/MicroBitRadio.cpp#L146 https://github.com/lancaster-university/microbit-dal/blob/master/source/drivers/MicroBitRadio.cpp#L144

It's not relevant to micro:bit, but neither does NRF52Radio https://github.com/lancaster-university/codal-nrf52/blob/master/source/NRF52Radio.cpp#L146

DAL was fixed after CODAL's MicroBitRadio was created here: https://github.com/lancaster-university/microbit-dal/commit/651594254217bd53618327934424988de61b20c6 Note the other changes, e.g. not calling setFrequencyBand() from enable()

martinwork commented 1 year ago

I was looking to make a PR to fix setFrequencyBand to work like the DAL, which includes changing enable(), so it doesn't call setFrequencyBand, and I noticed that enable() resets power and frequency to the defaults. setSleep() does not take account of this.

Is it assumed that enable() resets these values? It doesn't reset the group. This surprised me, although the description of enable(), does say "Initialises". https://lancaster-university.github.io/microbit-docs/ubit/radio/#enable

Last March, MakeCode gained APIs on() and off() to call enable() and disable() https://github.com/microsoft/pxt-common-packages/commit/9a3c943896e3676f154cea022ec40902c2e83d9c https://github.com/microsoft/pxt-microbit/issues/4700

If resetting those values is right for enable(), then I think setSleep needs to be fixed to store and restore the values.

microbit-carlos commented 1 year ago

Thanks Martin! It might be worth opening a new issue for the enable() question/findings to be able to track these two things separately, as that might have an impact as well on the sleeping methods.

microbit-carlos commented 1 year ago

@martinwork could you create a PR for this fix and then @JohnVidler can review it.

martinwork commented 1 year ago

PR https://github.com/lancaster-university/codal-microbit-v2/pull/252

microbit-carlos commented 1 year ago

Fixed in PR #252 🎉