LoRaMac-node icon indicating copy to clipboard operation
LoRaMac-node copied to clipboard

Radio.Sleep() may leave antenna switch powered

Open lff5 opened this issue 1 month ago • 1 comments

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 avatar Jun 13 '24 10:06 lff5