RAKWireless / RAK-STM32-RUI

RUI3 BSP for RAK3172 modules
Other
24 stars 15 forks source link

Various issues with LoRa / LoRaWAN support #13

Closed Kongduino closed 3 months ago

Kongduino commented 8 months ago
  1. The API has been split into two parts, RAKLorawan / RAKLoraP2P, which is great. However, a large chunk of RAKLoraP2P functions is still inside RAKLorawan files. Why?!?

  2. As a consequence of this, compiling with "Support LoRa P2P" does not work, as some of the required functions are in RAKLorawan. Oops.

  3. Also, there were many changes in the API, which is also kind of great, as they made the API more readable. However, it seems like the devs never bothered to recompile the examples published with the BSP: if they did they would have realized that the examples, especially the example for P2P (which is originally by me), would not compile, since they use the old API. Oops...

  4. In boards.txt we have these definitions, for what to do when the user wants support for LoRaWAN and/or LoRa P2P:

WisCoreRAK4631Board.menu.supportlora.1=Support LoRaWAN and LoRa P2P
WisCoreRAK4631Board.menu.supportlora.1.build.supportlora=-DSUPPORT_LORA
WisCoreRAK4631Board.menu.supportlora.1.build.supportlora_p2p=-DSUPPORT_LORA_P2P
WisCoreRAK4631Board.menu.supportlora.2=Support LoRaWAN
WisCoreRAK4631Board.menu.supportlora.2.build.supportlora=-DSUPPORT_LORA
WisCoreRAK4631Board.menu.supportlora.2.build.supportlora_p2p=
WisCoreRAK4631Board.menu.supportlora.3=Support LoRa P2P
WisCoreRAK4631Board.menu.supportlora.3.build.supportlora=
WisCoreRAK4631Board.menu.supportlora.3.build.supportlora_p2p=-DSUPPORT_LORA_P2P -DREGION_EU868
WisCoreRAK4631Board.menu.supportlora.4=Not Support
WisCoreRAK4631Board.menu.supportlora.4.build.supportlora=
WisCoreRAK4631Board.menu.supportlora.4.build.supportlora_p2p=

There are 2 issues here. SUPPORT_LORA should really be SUPPORT_LORAWAN. LoRa != LoRaWAN. And SUPPORT_LORA_P2P should be SUPPORT_LORA, really. This has an unfortunate consequence:

In sleep.cpp / sleep.h, sleep.lora() is restricted to SUPPORT_LORA (meaning LoRaWAN). Which doesn't make sense, since this is for LoRa P2P.

#ifdef SUPPORT_LORA
void sleep::lora(uint32_t ms_time) {
  if (ms_time == 0) {
    return;
  }
  udrv_radio_sleep_ms(ms_time);
}

void sleep::lora(int ms_time) {
  if (ms_time == 0) {
    return;
  }
  udrv_radio_sleep_ms((uint32_t)ms_time);
}
#endif

So either the SUPPORT_LORA is to be replaced with SUPPORT_LORA_P2P, or, as I suggest above, make SUPPORT_LORA for P2P, and use SUPPORT_LORAWAN for LoRaWAN...

All these points are valid for all platforms, but I happen to be using RAK3172 right now. I have made all these changes myself (except SUPPORT_LORAWAN, as it's quite more extensive), and it is working well. It makes code smaller too, since if you are using only P2P or LoRaWAN, the other API doesn't get included. I'd be happy to contribute – or put up an alternate version on my GitHub.

IoTThinks commented 3 months ago

I believe LoRaWAN still needs to use some basic function of LoRa for sending, receiving packets and setting SF, BW, DR... So "Support LoRaWAN" may include "Support LoRa".

beegee-tokyo commented 3 months ago

When in LoRaWAN mode there is no possibility to set SF/BW/DR, it is all handled by the LoRaWAN protocol and the regional LoRaWAN settings.