sudomesh / LoRaLayer2

Layer 2 routing protocol for LoRa connected devices
86 stars 29 forks source link

Add setPins() and setSPIFrequency() + misc other begin() changes #1

Closed tlrobinson closed 4 years ago

tlrobinson commented 4 years ago
paidforby commented 4 years ago

Cool commit, thanks for the input!

I considered this addition when I was fist designing the Layer1 code, as you can see by my comment next to Layer1.init(). I think I decided against it because I wanted to keep things as simple as possible. Ultimately, the idea was to add something like the ability to define a constant that would then set the correct pin constants in the header, a la this code from LoRa.h, https://github.com/sandeepmistry/arduino-LoRa/blob/master/src/LoRa.h#L10. However, given the structure of this library, it is not that simple to accomplish this sort of thing because Layer1.h is called in both LoRaLayer2.cpp, and main.ino, which would both need to define that constant.

Anyway, I think this merge is a better, though perhaps less elegant, way of dealing with setting the pins.

I will merge this commit once I have a chance to test it. Thanks!

tlrobinson commented 4 years ago

@paidforby Yeah, I also tried to figure out a way to do it with defines. I think the best option is to just try to detect defaults for the pins, but still provide this API in case it doesn't support a board you're using. I couldn't figure out any other way to get disaster-radio working with the TTGO LoRa32 V1 board without forking this.

FYI TTGO's board definitions have LORA_CS/LORA_RST/LORA_IRQ defined which you could use as defaults:

https://github.com/espressif/arduino-esp32/blob/master/variants/ttgo-lora32-v1/pins_arduino.h#L20-L25 https://github.com/espressif/arduino-esp32/blob/master/variants/t-beam/pins_arduino.h#L15-L23

Something like:

#ifdef LORA_CS

#define LORA_DEFAULT_CS_PIN        LORA_CS
#define LORA_DEFAULT_RESET_PIN     LORA_RST
#define LORA_DEFAULT_DIO0_PIN      LORA_IRQ

#else

#define LORA_DEFAULT_CS_PIN        2
#define LORA_DEFAULT_RESET_PIN     5
#define LORA_DEFAULT_DIO0_PIN      16

#endif

The problem with that is the ttgo-lora32-v1 board definition matches v1 (of course) rather than v2 (which has the SD card). Maybe we should contribute a v2 board definition

In the meantime I think you could pass compiler flags like -D LORA_CS=18 -D LORA_RST=14 -D LORA_IRQ=26

paidforby commented 4 years ago

Tested. Merge appears to work, though I have only tested with the default TGGO v2 board. I will update disaster-radio repo to point at this commit for both makeEspArduino and platformIO.