ClusterDuck-Protocol / ClusterDuck-Protocol

Firmware for an ad-hoc mesh network of Internet-of-Things devices based on LoRa (Long Range radio) that can be deployed quickly and at low cost.
https://clusterduckprotocol.org
Apache License 2.0
355 stars 149 forks source link

DuckRadio.cpp compile error when using SX126X #344

Closed amirna2 closed 5 months ago

amirna2 commented 6 months ago

Describe the bug When building for a board with a Semtech SX1262 the build fails, unless SPI pins are defined in the board definition. SPI should always be optional and it is not needed in most cases, because the boards we support all have a SoC that integrates the LoRa chip with the CPU and in which case the SPI communication is handled by RadioLib internally.

The failing code is

#if defined(CDPCFG_PIN_LORA_SCLK) & defined(CDPCFG_RADIO_SX126X)
#include <SPI.h>
SPIClass spi;
SPISettings spiSettings;
CDPCFG_LORA_CLASS lora =
        new Module(CDPCFG_PIN_LORA_CS, CDPCFG_PIN_LORA_DIO1, CDPCFG_PIN_LORA_RST,
                   CDPCFG_PIN_LORA_BUSY, spi);
#elif defined(CDPCFG_RADIO_SX126X)
CDPCFG_LORA_CLASS lora =
        new Module(CDPCFG_PIN_LORA_CS, CDPCFG_PIN_LORA_DIO1, CDPCFG_PIN_LORA_RST,
                   CDPCFG_PIN_LORA_BUSY, spi);
#else
CDPCFG_LORA_CLASS lora = new Module(CDPCFG_PIN_LORA_CS, CDPCFG_PIN_LORA_DIO0,
                  CDPCFG_PIN_LORA_RST, CDPCFG_PIN_LORA_DIO1);
#endif

Notice that both #if and #elif blocks are identical, however the #elif block fails to compile if CDPCFG_RADIO_SX126X is defined. Which is the most common case.

To reproduce

  1. Choose the Heltec CubeCell for example (it doesn't define the SPI pins)
  2. The compilation will fail at this line beccause spi is undefined (it's defined in the #if block not in the #elif
    new Module(CDPCFG_PIN_LORA_CS, CDPCFG_PIN_LORA_DIO1, CDPCFG_PIN_LORA_RST,
                   CDPCFG_PIN_LORA_BUSY, spi);

Expected behavior When building for an SX1262 board and we don't defined SPI pins, DuckRadio should build.

Additional context The fix for this is to not use SPI in #elif defined(CDPCFG_RADIO_SX126X)

new Module(CDPCFG_PIN_LORA_CS, CDPCFG_PIN_LORA_DIO1, CDPCFG_PIN_LORA_RST,
                   CDPCFG_PIN_LORA_BUSY);

Additionally, instead of checking for the SCLK define here, it's better to use a more explicit definition e.g USE_SPI which should be defined in the board definition header file.

#if defined(CDPCFG_PIN_LORA_SCLK) & defined(CDPCFG_RADIO_SX126X)
amirna2 commented 5 months ago

Fixed in CDP 4.0.0