refactor: update TAG constants and improve logging format in SPI implementation

2025-08-07 19:25:16 +08:00
parent edcdc779d9
commit 1237e5ea48
3 changed files with 46 additions and 33 deletions
--- a/inc/hal_spi.hpp
+++ b/inc/hal_spi.hpp
@ -17,7 +17,7 @@ namespace app::driver::hal::spi {
 template <typename T, typename E>
 using Result       = app::utils::Result<T, E>;
 using Unit         = app::utils::Unit;
-constexpr auto TAG = "spi";
+constexpr auto TAG = "app_spi";

 constexpr auto MOSI_PIN = llcc68::MOSI_PIN;
 constexpr auto MISO_PIN = llcc68::MISO_PIN;
--- a/inc/llcc68.hpp
+++ b/inc/llcc68.hpp
@ -11,6 +11,7 @@
 #include <cstring>
 #include <optional>
 #include <esp_log.h>
+#include "hal/gpio_types.h"
 #include "hal_gpio.hpp"
 #include "app_const_llcc68.hpp"
 #include "radiolib_definitions.hpp"
@ -157,7 +158,7 @@ reset() {
 		// don't spam the module
 		delay_ms(100);
 	}
-	if (!res) {
+	if (not res) {
 		return res;
 	} else {
 		return {};
@ -207,7 +208,13 @@ inline Result<irq_status_t, error_t>
 get_irq_status() {
 	uint8_t data[] = {0x00, 0x00};
 	const auto res = spi::read_stream(RADIOLIB_SX126X_CMD_GET_IRQ_STATUS, data);
-	APP_RADIO_RETURN_ERR(res);
+	// get_irq_status would sometimes return CMD_PROC_ERR
+	// however, the result is still valid
+	if (not res) {
+		if (not(res.error() == error::RADIO_TRANS_CMD_PROC_ERR)) {
+			return ue_t{res.error()};
+		}
+	}
 	return irq_status_t{.raw = static_cast<uint16_t>(static_cast<uint16_t>(data[0] << 8) | static_cast<uint16_t>(data[1]))};
 }

@ -580,7 +587,7 @@ set_packet_params(const uint16_t preamble_length = 8,
 /**
 * \brief frequency synthesizer calibration
 */
-inline Result<Unit, error_t> fs() {
+inline Result<Unit, error_t> set_fs() {
 	// default constructor of std::span is empty
 	return spi::write_stream(RADIOLIB_SX126X_CMD_SET_FS, std::span<uint8_t>());
 }
@ -588,7 +595,7 @@ inline Result<Unit, error_t> fs() {
 /**
 * \param timeout no idea why you need a timeout for TX
 */
-inline Result<Unit, error_t> tx(const uint32_t timeout = 0) {
+inline Result<Unit, error_t> set_tx(const uint32_t timeout = 0) {
 	const uint8_t data[] = {static_cast<uint8_t>((timeout >> 16) & 0xFF), static_cast<uint8_t>((timeout >> 8) & 0xFF), static_cast<uint8_t>(timeout & 0xFF)};
 	return spi::write_stream(RADIOLIB_SX126X_CMD_SET_TX, data);
 }
@ -604,7 +611,7 @@ inline Result<Unit, error_t> tx(const uint32_t timeout = 0) {
  For any other value, timeout will be applied and signal will be generated on DIO1 for conditions
  defined by irqFlags and irqMask.
 */
-inline Result<Unit, error_t> rx(const uint32_t timeout = RADIOLIB_SX126X_RX_TIMEOUT_INF) {
+inline Result<Unit, error_t> set_rx(const uint32_t timeout = RADIOLIB_SX126X_RX_TIMEOUT_INF) {
 	const uint8_t data[] = {static_cast<uint8_t>((timeout >> 16) & 0xFF), static_cast<uint8_t>((timeout >> 8) & 0xFF), static_cast<uint8_t>(timeout & 0xFF)};
 	return spi::write_stream(RADIOLIB_SX126X_CMD_SET_RX, data);
 }
@ -1053,11 +1060,12 @@ read_data_internal() {
 */
 inline Result<Unit, error_t>
 kick_inf_rx(const modulation_params_t &mod_params, const packet_params_t &packet_params) {
+	constexpr auto TAG = "llcc68::kick_inf_rx";
 	Result<Unit, error_t> res;
 	res = standby();
-	APP_RADIO_RETURN_ERR_CTX(res, "failed to standby");
+	APP_RADIO_RETURN_ERR_CTX(res, "standby");
 	res = set_buffer_base_address();
-	APP_RADIO_RETURN_ERR_CTX(res, "failed to set buffer base address");
+	APP_RADIO_RETURN_ERR_CTX(res, "set buffer base address");

 	constexpr auto irq_mask   = RADIOLIB_SX126X_IRQ_RX_DEFAULT;
 	constexpr auto irq_params = irq_params_t{
@ -1076,7 +1084,7 @@ kick_inf_rx(const modulation_params_t &mod_params, const packet_params_t &packet
 	APP_RADIO_RETURN_ERR_CTX(res, "dio irq params");
 	res = clear_irq_status();
 	APP_RADIO_RETURN_ERR_CTX(res, "clear irq status");
-	res = rx(RADIOLIB_SX126X_RX_TIMEOUT_INF);
+	res = set_rx(RADIOLIB_SX126X_RX_TIMEOUT_INF);
 	APP_RADIO_RETURN_ERR_CTX(res, "set rx");
 	return {};
 }
@ -1120,6 +1128,7 @@ inline Result<transmit_state_t, error_t>
 async_transmit(const std::span<const uint8_t> data,
 			   const lora_parameters_t &params,
 			   const transmit_state_t &tx_state) {
+	constexpr auto TAG = "llcc68::async_transmit";
 	if (tx_state.is_transmitting) {
 		return ue_t{error_t{error::RADIO_BUSY_TX}};
 	}
@ -1157,8 +1166,8 @@ async_transmit(const std::span<const uint8_t> data,
 	clear_irq_status(irq_mask);
 	APP_RADIO_RETURN_ERR_CTX(res, "clear irq status");

-	res = tx();
-	APP_RADIO_RETURN_ERR_CTX(res, "tx");
+	res = set_tx();
+	APP_RADIO_RETURN_ERR_CTX(res, "set tx");

 	constexpr auto TIMEOUT_FACTOR = 11u / 10u;
 	const auto timeout_us         = calc_time_on_air(data.size(),
@ -1205,27 +1214,30 @@ static constexpr auto init_pins = [](void (*isr)(void *), void *arg) {
 	if (RST_PIN != GPIO_NUM_NC) {
 		gpio::set_mode(RST_PIN, gpio::Mode::OUTPUT);
 	}
-	uint64_t pin_bit_mask = 0;
-	for (const auto pin : EXTI_PINS) {
-		pin_bit_mask |= (1ULL << pin);
-	}
-	gpio_config_t io_conf = {
-		.pin_bit_mask = pin_bit_mask,
-		.mode         = GPIO_MODE_INPUT,
-		.pull_up_en   = GPIO_PULLUP_DISABLE,
-		.pull_down_en = GPIO_PULLDOWN_DISABLE,
-		.intr_type    = GPIO_INTR_POSEDGE,
-	};
-	// https://github.com/espressif/esp-idf/blob/v5.3.2/examples/peripherals/gpio/generic_gpio/main/gpio_example_main.c
-	ESP_ERROR_CHECK(gpio_config(&io_conf));
-	gpio_install_isr_service(ESP_INTR_FLAG_IRAM);
-	for (const auto pin : EXTI_PINS) {
-		if (pin == NC_PIN) {
-			continue;
-		}
-		gpio_isr_handler_add(pin, isr, arg);
-	}

+	{
+		uint64_t pin_bit_mask = 0;
+		for (const auto pin : EXTI_PINS) {
+			pin_bit_mask |= (1ULL << pin);
+		}
+		gpio_config_t io_conf = {
+			.pin_bit_mask = pin_bit_mask,
+			.mode         = GPIO_MODE_INPUT,
+			.pull_up_en   = GPIO_PULLUP_DISABLE,
+			.pull_down_en = GPIO_PULLDOWN_DISABLE,
+			.intr_type    = GPIO_INTR_DISABLE,
+		};
+		// https://github.com/espressif/esp-idf/blob/v5.4.2/examples/peripherals/gpio/generic_gpio/main/gpio_example_main.c
+		ESP_ERROR_CHECK(gpio_config(&io_conf));
+		gpio_install_isr_service(0);
+		for (const auto pin : EXTI_PINS) {
+			if (pin == NC_PIN) {
+				continue;
+			}
+			ESP_ERROR_CHECK(gpio_set_intr_type(pin, GPIO_INTR_POSEDGE));
+			ESP_ERROR_CHECK(gpio_isr_handler_add(pin, isr, arg));
+		}
+	}
 	// initialize TX & RX enable pins
 	init_tx_rx_en();
 };
@ -1254,6 +1266,7 @@ static constexpr auto begin = [](const lora_parameters_t &params) -> Result<Unit
 	 * \note
 	 * If this order is not respected, the behavior of the device could be unexpected.
 	 **/
+	constexpr auto TAG = "llcc68::begin";
 	Result<Unit, error_t> res;

 	/* explicit not reset the chip by pin, due to unexpected side effect */