Lora-net / LoRaMac-node

Reference implementation and documentation of a LoRa network node.
Other
1.85k stars 1.08k forks source link

Radio.Sleep() may leave antenna switch powered #1611

Open lff5 opened 2 weeks ago

lff5 commented 2 weeks ago

I was trying to work around this issue https://github.com/Lora-net/LoRaMac-node/issues/1594 by calling Radio.Sleep() after LoRaMAC init with NVM restore. My initial issue is this one https://github.com/zephyrproject-rtos/zephyr/issues/73417#issuecomment-2165139700.

The problem I'm bringing up here following: calling Radio.Sleep() puts radio to sleep mode, but in some states turns on the Antenna Switch pin which means higher current consumption.

Its easy to see in the code. Here is the stack call walkthrough: RadioSleep() -> SX126xSetSleep() -> SX126xWriteCommand() -> sx126x_spi_transceive() -> SX126xCheckDeviceReady() -> SX126xAntSwOn()

Here are the functions in calling order:

void RadioSleep( void )
{
    SleepParams_t params = { 0 };

    params.Fields.WarmStart = 1;
    SX126xSetSleep( params );

    DelayMs( 2 );
}

https://github.com/Lora-net/LoRaMac-node/blob/dcbcfb329b4a343ab007bc19ac43a8dc952b3354/src/radio/sx126x/sx126x.c#L254-L269

void SX126xWriteCommand(RadioCommands_t opcode, uint8_t *buffer, uint16_t size)
{
    uint8_t req[] = {
        opcode,
    };

    LOG_DBG("Issuing opcode 0x%x w. %" PRIu16 " bytes of data",
        opcode, size);
    sx126x_spi_transceive(req, NULL, sizeof(req), buffer, NULL, size);
}
static int sx126x_spi_transceive(uint8_t *req_tx, uint8_t *req_rx,
                 size_t req_len, void *data_tx, void *data_rx,
                 size_t data_len)
{
    int ret;

    const struct spi_buf tx_buf[] = {
        {
            .buf = req_tx,
            .len = req_len,
        },
        {
            .buf = data_tx,
            .len = data_len
        }
    };

    const struct spi_buf rx_buf[] = {
        {
            .buf = req_rx,
            .len = req_len,
        },
        {
            .buf = data_rx,
            .len = data_len
        }
    };

    const struct spi_buf_set tx = {
        .buffers = tx_buf,
        .count = ARRAY_SIZE(tx_buf),
    };

    const struct spi_buf_set rx = {
        .buffers = rx_buf,
        .count = ARRAY_SIZE(rx_buf)
    };

    /* Wake the device if necessary */
    SX126xCheckDeviceReady();

    if (!req_rx && !data_rx) {
        ret = spi_write_dt(&dev_config.bus, &tx);
    } else {
        ret = spi_transceive_dt(&dev_config.bus, &tx, &rx);
    }

    if (ret < 0) {
        LOG_ERR("SPI transaction failed: %i", ret);
    }

    if (req_len >= 1 && req_tx[0] != RADIO_SET_SLEEP) {
        SX126xWaitOnBusy();
    }
    return ret;
}

https://github.com/Lora-net/LoRaMac-node/blob/dcbcfb329b4a343ab007bc19ac43a8dc952b3354/src/radio/sx126x/sx126x.c#L134-L143

So RadioSleep() -> SX126xSetSleep() -> SX126xWriteCommand() -> sx126x_spi_transceive() -> SX126xCheckDeviceReady() -> SX126xAntSwOn()

SX126xSetSleep() first calls SX126xAntSwOff() and then calls SX126xWriteCommand() which internally may call SX126xAntSwOn() and leave RF switch on. I think it would be safer to call SX126xAntSwOff() in the end.

I discovered this because of 'working around' or 'misusing' the internal api, but please do consider this tiny, it seems much safer! I haven't checked if the other radio implementations suffer from the same potential issue.

lff5 commented 2 weeks ago

My current workaround is to call Radio.Sleep() and then SX126xAntSwOff() after lorawan init (with NVM) in application layer.

Here is what the lorawan init looks like: https://github.com/zephyrproject-rtos/zephyr/blob/d45605e6a3dfe68e43207d15e3bfc003cbfaa72f/subsys/lorawan/lorawan.c#L682-L723