Skip to content

Commit fba614b

Browse files
committed
drivers/sx126x: improve hardware detection
- split out the detection logic into a static helper function - replace a forgotten debug `printf()` with `DEBUG()` - add a `LOG_ERROR()` to when no device is detect to ease diagnosis of a failing SPI bus / incorrect wiring / broken chip - use a bullet-proof test: - First set the modulation type to LoRa - Then get the modulation type and expect it to be LoRa Even if the reset of the devices fails (e.g. because the reset signal is not connected) and the modulation would be different from LoRa, this now should reliably detect the chip.
1 parent 42283c4 commit fba614b

File tree

1 file changed

+31
-16
lines changed

1 file changed

+31
-16
lines changed

drivers/sx126x/sx126x.c

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
#include "sx126x_netdev.h"
2424

25+
#include "log.h"
2526
#include "macros/units.h"
2627
#include "net/lora.h"
2728
#include "periph/spi.h"
@@ -321,6 +322,35 @@ static void _dio1_isr(void *arg)
321322
}
322323
#endif
323324

325+
/**
326+
* @brief Detect whether an SX126x device is actually connected
327+
* @param dev Initialized device descriptor to use
328+
* @pre @p dev is initialized and the /CS pin has been configured
329+
*
330+
* This sadly is a bit more involved than one would hope, as no vendor and
331+
* model codes are in the register map that can be searched for. Instead,
332+
* we write the packet type (or rather modulation type) to be 100% the device
333+
* must be configured to use LoRa modulation, then read this back and expect
334+
* it to be LoRa. In we also check the device status to be valid, which adds
335+
* one more known bit to compare against.
336+
*/
337+
static bool _sx126x_detect(sx126x_t *dev)
338+
{
339+
sx126x_pkt_type_t pkt_type = UINT8_MAX;
340+
sx126x_chip_status_t radio_status = { 0 };
341+
sx126x_set_pkt_type(dev, SX126X_PKT_TYPE_LORA);
342+
sx126x_get_status(dev, &radio_status);
343+
sx126x_get_pkt_type(dev, &pkt_type);
344+
345+
if ((pkt_type == SX126X_PKT_TYPE_LORA) && (radio_status.chip_mode)) {
346+
return true;
347+
}
348+
DEBUG("[sx126x] pkt_type = %x, radio_status.chip_mode = %u\n",
349+
(unsigned)pkt_type, (unsigned)radio_status.chip_mode);
350+
LOG_ERROR("[sx126x] No device detected\n");
351+
return false;
352+
}
353+
324354
int sx126x_init(sx126x_t *dev)
325355
{
326356
/* Setup SPI for SX126X */
@@ -355,22 +385,7 @@ int sx126x_init(sx126x_t *dev)
355385
/* Reset the device */
356386
sx126x_reset(dev);
357387

358-
/* Read status and packet type to verify device presence. Just reading the
359-
* status can lead incorrectly detecting the chip to be present, e.g. when
360-
* CIPO (a.k.a. MISO) is pulled high. The packet type after reset should be
361-
* valid. The chance of it not being valid is quite good on floating SPI.
362-
* If CIPO is pulled down, `radio_status.chip_mode` will be zero, if CIPO
363-
* is pulled high, `pkt_type` will be `UINT8_MAX`. So in those two cases,
364-
* an unconnected chip will be detected reliably.
365-
*/
366-
sx126x_pkt_type_t pkt_type = UINT8_MAX;
367-
sx126x_chip_status_t radio_status = { 0 };
368-
sx126x_get_pkt_type(dev, &pkt_type);
369-
sx126x_get_status(dev, &radio_status);
370-
if ((pkt_type > SX126X_PKT_TYPE_LR_FHSS) || !radio_status.chip_mode) {
371-
DEBUG("[sx126x] error: no device found\n");
372-
printf("pkt_type = %x, radio_status.chip_mode = %u\n",
373-
(unsigned)pkt_type, (unsigned)radio_status.chip_mode);
388+
if (!_sx126x_detect(dev)) {
374389
return -ENODEV;
375390
}
376391

0 commit comments

Comments
 (0)