Example code manages appPort variable extremely poorly

[jcwren]

All the examples use the same subroutine to prepare data to send. Using OTAA.ino as an example.

Where most of the globals are defined in any of the example code, we have

uint8_t appPort = 2;

to define and set the global variable that specifies which port the application data should be sent to.

In the send portion of the state machine, we have the following code:

    case DEVICE_STATE_SEND :
      {
        prepareTxFrame (appPort);
        LoRaWAN.send (loraWanClass);
        deviceState = DEVICE_STATE_CYCLE;
      }
      break;

Looking at prepareTxFrame(), we have this code:

static void prepareTxFrame (uint8_t port)
{
  appDataSize = 4;
  appData [0] = 0x00;
  appData [1] = 0x01;
  appData [2] = 0x02;
  appData [3] = 0x03;
}

The Arduino compiler flags suppress just about every useful warning a user might actually ever want to see, such as unused variables. The port variable is never used, and furthermore, it's passed a global variable as the parameter. The same global variable that's used in ESP32_LoRaWAN.cpp to set the port...

The more correct way to do this would be to #define APP_DATA_PORT 2, pass that (or other values) to the prepareTxFrame() function, and set appPort inside prepareTxFrame() so that user code can actually use multiple ports. That way we don't waste 20 minutes wondering how the port gets set, since NOWHERE does the "documentation" indicate appPort is required to be defined by the user, unlike appData and appDataSize, which are defined in ESP32_LoRaWAN.cpp.

This is also a shining example of why global variables are generally bad, and why they should be inside a structure instead of polluting the entire namespace.