LoRaWAN library format is a bit strange

[ZaneL]

I was looking at the LoRaWAN example: https://github.com/HelTecAutomation/ASR650x-Arduino/blob/master/libraries/LoRa/examples/LoRaWAN/LoRaWan/LoRaWan.ino

...and it's strange how the class variables are all outside of the class and extern'd: https://github.com/HelTecAutomation/ASR650x-Arduino/blob/master/libraries/LoRa/src/LoRaWan_APP.h

and then you have to declare them in your app before calling LoRaWAN.init(). This is super confusing because the user has no idea which variables are needed to actually initialize the library. Even setting basic stuff like OTAA or ABP makes no sense to me because in the example there is this line:

bool overTheAirActivation = LORAWAN_NETMODE;

What is the value of LORAWAN_NETMODE? What does that even mean? Is this line setting it to ABP or OTAA mode? Am I supposed to know that overTheAirActivation is extern'd somewhere, because at quick glance it just looks like an unused variable.

imo you need to move all of these variables into the private section of the class, get rid of the extern stuff, and then either in the constructor or in the init function you should pass in the various parameters to set these private variables.

Something like how it's done here: https://github.com/TheThingsNetwork/arduino-device-lib/blob/master/src/TheThingsNetwork.h

[bertrik]

To be honest, I'm a bit concerned about this too.

As I understand now, all of the LoRaWAN stuff is all in a CubeCellLib.a library and you interact with it by modifying global variables and implementing your own global variables. This is not how libraries usually work.

Also there is no source code for the LoRaWAN code, or am I wrong?

Recent commits to the repository all seem to be related to adding more sensor code/examples, but that's not what I bought the board for. I got it for the LoRaWAN connectivity (I can do the sensor stuff myself).

[ZaneL]

I think most of the underlying source code for the LoRaWAN stuff is available to look at.

The MAC layer which comes from LoraMac-node (https://github.com/Lora-net/LoRaMac-node) is found here: https://github.com/HelTecAutomation/ASR650x-Arduino/tree/master/cores/asr650x/loramac

Looks like they stripped lots of stuff out and kept was was necessary. Looks fine to me. Then the driver (also from LoraMac-node) for the radio (SX1262) they put over here: https://github.com/HelTecAutomation/ASR650x-Arduino/tree/master/cores/asr650x/device

You can see that the driver calls SX126xWriteRegisters() to send SPI data to the SX1262 and that function is implemented here: https://github.com/HelTecAutomation/ASR650x-Arduino/blob/master/cores/asr650x/board/src/asr_board.c

Their LoRaWAN code sits on top of that semtech loramac code - and it is found here: https://github.com/HelTecAutomation/ASR650x-Arduino/tree/master/libraries/LoRa/src

I agree though -- the sensors are easy enough for the user to implement. I think their effort would be much better spent on getting the deep sleep code and lorawan code all working perfectly.

[bertrik]

Ok, cool. Thanks for the clarification.

[lnlp]

and then you have to declare them in your app before calling LoRaWAN.init(). This is super confusing because the user has no idea which variables are needed to actually initialize the library.

Am I supposed to know that overTheAirActivation is extern'd somewhere, because at quick glance it just looks like an unused variable.

imo you need to move all of these variables into the private section of the class, get rid of the extern stuff, and then either in the constructor or in the init function you should pass in the various parameters to set these private variables.

Correct. Using global parameters for settings is poor (OO) software design. Related settings shall be put in a separate class (a struct at minimum), init() should be parameterized (encapsulate implementation details) and enum class type should be used for settings parameter values where possible.

Make settings and their values discoverable so the compiler and IntelliSense can assist the developer.

[Heltec-Aaron-Lee]

bool overTheAirActivation = LORAWAN_NETMODE;

What is the value of LORAWAN_NETMODE? What does that even mean? Is this line setting it to ABP or OTAA mode? Am I supposed to know that overTheAirActivation is extern'd somewhere, because at quick glance it just looks like an unused variable.

Just hope some important parameters can easily choose in the Tools menu. But in Arduino IDE, if the parameters in the tools menu want acting on your code, you must define the relevant variable in this way.

bool overTheAirActivation = LORAWAN_NETMODE;

[eiten]

Just hope some important parameters can easily choose in the Tools menu. But in Arduino IDE, if the parameters in the tools menu want acting on your code, you must define the relevant variable in this way.

Yes, I see your point, but I think that's not a good way of doing it. I think you should concentrate on the ASR650x-stuf. For the Semtec-Part, there are really great libs LMIC for LoraWAN or the LoRa-Lib by Sandeep Mistry. If you concentrate on Arduino-Compatibility of the ASR650x, developpers can use the libs they are used to.

[davidbrodrick]

I've just created this pull request which I think is a step in the right direction, although maybe not yet all the way there.

https://github.com/HelTecAutomation/ASR650x-Arduino/pull/158

This does away with most of the globals, driving the OLED from inside the LoRa library and the state machine being embedded in the main loop. The interface is basically reduced to join and send.