TheThingsNetwork / arduino-device-lib

Arduino Library for TTN Devices
MIT License
207 stars 96 forks source link

Support for IN865_867 #233

Closed akshaim closed 6 years ago

CLAassistant commented 6 years ago

CLA assistant check
All committers have signed the CLA.

akshaim commented 6 years ago

865_867 addition was tested to work on two channels but 865985000 is showing a few issues. I shall debug and update soon.

johanstokking commented 6 years ago

@akshaim thanks! Should we wait for your debugging and test results?

akshaim commented 6 years ago

Sure. I am waiting for an SDR to arrive to see exactly what is going on.

akshaim commented 6 years ago

Works now. Uplink and Downlink tested.

jpmeijers commented 6 years ago

It is fine as it is, but I would have written this function differently as there is no need for the loop and switch - they just complicate the algorithm without saving any memory.

void TheThingsNetwork::configureIN865_867()
{
  sendMacSet(MAC_ADR, "off"); // TODO: remove when ADR is implemented for this plan
  sendMacSet(MAC_RX2, "2 866550000"); // SF10  

  // Disable the three default LoRaWAN channels
  sendChSet(MAC_CHANNEL_STATUS, 0, "off");
  sendChSet(MAC_CHANNEL_STATUS, 1, "off");
  sendChSet(MAC_CHANNEL_STATUS, 2, "off");

  // Channel 3
  sendChSet(MAC_CHANNEL_DCYCLE, 3, "799");
  sendChSet(MAC_CHANNEL_FREQ, 3, "865062500");
  sendChSet(MAC_CHANNEL_DRRANGE, 3, "0 5");
  sendChSet(MAC_CHANNEL_STATUS, 3, "on");

  // Channel 4
  sendChSet(MAC_CHANNEL_DCYCLE, 4, "799");
  sendChSet(MAC_CHANNEL_FREQ, 4, "865402500");
  sendChSet(MAC_CHANNEL_DRRANGE, 4, "0 5");
  sendChSet(MAC_CHANNEL_STATUS, 4, "on");

  // Channel 5
  sendChSet(MAC_CHANNEL_DCYCLE, 5, "799");
  sendChSet(MAC_CHANNEL_FREQ, 5, "865985000");
  sendChSet(MAC_CHANNEL_DRRANGE, 5, "0 5");
  sendChSet(MAC_CHANNEL_STATUS, 5, "on");

  sendMacSet(MAC_PWRIDX, TTN_PWRIDX_IN865_867);
}
akshaim commented 6 years ago

Thanks. I have updated the code as per your recommendations and it works! Can you help me implement ADR on the channel?

johanstokking commented 6 years ago

What is the reason to disable the three default LoRaWAN channels?

Regarding the loop; this is consistent with other frequency plans (and it may reduce flash memory).

johanstokking commented 6 years ago

Any news @akshaim @jpmeijers ?

jpmeijers commented 6 years ago
  1. On the RN module with european firmware one can not change the frequency of the three default channels. For India we therefore have to disable these three channels to not use them, and rather use another three to configure the three default Indian channels.

  2. Even though the original loop method is consistent with the other frequency plans, it complicates the code and in this case do not save any space. In this case there is no constant step between 8 channel, so using a loop to add the constant step doesn't make sense. Configuring the three channels one by one by hand is much clearer and does not really use much more program space than the loop. In fact, I believe it will use less instructions as we are not using the loop. For-loops are notorious to be compiled to a lot of instructions and are quite often unrolled by the compiler.

jpmeijers commented 6 years ago

Also note that we plan to rewrite the frequency plan logic to save program space. So let's not worry too much about it now as we will rethink and re-implement it later. See branch: https://github.com/TheThingsNetwork/arduino-device-lib/tree/feature/use-less-program-space

akshaim commented 6 years ago

@johanstokking Please see the following snap from the RN2483 command reference manual. Channel 0-2 can be used only on default channels. https://imgur.com/a/sSO9SMp