TheThingsNetwork / arduino-device-lib

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

2.7.0 release does not configure channels past 15 (problem for US915 FP) #279

Closed danalvarez closed 2 years ago

danalvarez commented 2 years ago

I just saw version 2.7.0 was just released following the changes in #277 , so decided to update my nodes with that code. Unfortunately, without any other changes than the library update, my nodes, which are configured to work in FSB 2 of the US915 FreqPlan, no longer transmit in the desired frequencies (channels 8-15). Diving into the code, I noticed that the setChannelStatus function was added, which is:

bool TheThingsNetwork::setChannelStatus (uint8_t channel, bool status){
  if (channel > 15)
    return false;

  if (status)
    return sendChSet(MAC_CHANNEL_STATUS, channel, "on");
  else
    return sendChSet(MAC_CHANNEL_STATUS, channel, "off");

}

I notice that this function does nothing when channel > 15, why? This seems to be the problem, because my nodes (which should only transmit in channels 8-15), now transmit in all channels greater than 8. If i comment out these two lines:

  // if (channel > 15)
  //    return false;

The nodes are then configured correctly and transmit only in the desired FSB.

I was eager to try out the new release, and I actually thank @flhofer who submitted the PR and @jpmeijers and @johanstokking who checked it, for mantaining and updating this already AMAZING repo. I was actually excited that RAM usage went down 5% for this new version my Arduino UNO. But, I feel a little worried to update my nodes now, for fear of encountering another problem such as this that might have gone unnoticed.

Anyways, hope this observation helps, please let me know the reasoning behind returning false when channel > 15.

johanstokking commented 2 years ago

Thanks for your report @danalvarez.

@flhofer @jpmeijers any comments on this? Is this a regression we should address?

danalvarez commented 2 years ago

If it is helpful, this is the output of the personalize() function with version 2.7.0 of the library, when configuring for US915 and FSB 2:

-- PERSONALIZE:
Model: RN2903
Version: 1.0.3
Sending: mac set deveui <redacted>
Sending: mac set adr off
Sending: mac set devaddr <redacted>
Sending: mac set nwkskey <redacted>
Sending: mac set appskey <redacted>
Sending: mac set ch status 0 off
Sending: mac set ch status 1 off
Sending: mac set ch status 2 off
Sending: mac set ch status 3 off
Sending: mac set ch status 4 off
Sending: mac set ch status 5 off
Sending: mac set ch status 6 off
Sending: mac set ch status 7 off
Sending: mac set ch status 8 on
Sending: mac set ch drrange 8 0 3
Sending: mac set ch status 9 on
Sending: mac set ch drrange 9 0 3
Sending: mac set ch status 10 on
Sending: mac set ch drrange 10 0 3
Sending: mac set ch status 11 on
Sending: mac set ch drrange 11 0 3
Sending: mac set ch status 12 on
Sending: mac set ch drrange 12 0 3
Sending: mac set ch status 13 on
Sending: mac set ch drrange 13 0 3
Sending: mac set ch status 14 on
Sending: mac set ch drrange 14 0 3
Sending: mac set ch status 15 on
Sending: mac set ch drrange 15 0 3
Sending: mac set pwridx 5
Sending: mac set retx 7
Sending: mac set dr 0
Sending: mac join abp
Personalize accepted. Status: 00000001

On the other hand, this is the output with version 2.6.0 of the library:

-- PERSONALIZE:
Model: RN2903
Version: 1.0.3
Sending: mac set deveui <redacted>
Sending: mac set adr off
Sending: mac set devaddr <redacted>
Sending: mac set nwkskey <redacted>
Sending: mac set appskey <redacted>
Sending: mac set ch status 0 off
Sending: mac set ch status 1 off
Sending: mac set ch status 2 off
Sending: mac set ch status 3 off
Sending: mac set ch status 4 off
Sending: mac set ch status 5 off
Sending: mac set ch status 6 off
Sending: mac set ch status 7 off
Sending: mac set ch status 8 on
Sending: mac set ch drrange 8 0 3
Sending: mac set ch status 9 on
Sending: mac set ch drrange 9 0 3
Sending: mac set ch status 10 on
Sending: mac set ch drrange 10 0 3
Sending: mac set ch status 11 on
Sending: mac set ch drrange 11 0 3
Sending: mac set ch status 12 on
Sending: mac set ch drrange 12 0 3
Sending: mac set ch status 13 on
Sending: mac set ch drrange 13 0 3
Sending: mac set ch status 14 on
Sending: mac set ch drrange 14 0 3
Sending: mac set ch status 15 on
Sending: mac set ch drrange 15 0 3
Sending: mac set ch status 16 off
Sending: mac set ch status 17 off
Sending: mac set ch status 18 off
Sending: mac set ch status 19 off
Sending: mac set ch status 20 off
Sending: mac set ch status 21 off
Sending: mac set ch status 22 off
Sending: mac set ch status 23 off
Sending: mac set ch status 24 off
Sending: mac set ch status 25 off
Sending: mac set ch status 26 off
Sending: mac set ch status 27 off
Sending: mac set ch status 28 off
Sending: mac set ch status 29 off
Sending: mac set ch status 30 off
Sending: mac set ch status 31 off
Sending: mac set ch status 32 off
Sending: mac set ch status 33 off
Sending: mac set ch status 34 off
Sending: mac set ch status 35 off
Sending: mac set ch status 36 off
Sending: mac set ch status 37 off
Sending: mac set ch status 38 off
Sending: mac set ch status 39 off
Sending: mac set ch status 40 off
Sending: mac set ch status 41 off
Sending: mac set ch status 42 off
Sending: mac set ch status 43 off
Sending: mac set ch status 44 off
Sending: mac set ch status 45 off
Sending: mac set ch status 46 off
Sending: mac set ch status 47 off
Sending: mac set ch status 48 off
Sending: mac set ch status 49 off
Sending: mac set ch status 50 off
Sending: mac set ch status 51 off
Sending: mac set ch status 52 off
Sending: mac set ch status 53 off
Sending: mac set ch status 54 off
Sending: mac set ch status 55 off
Sending: mac set ch status 56 off
Sending: mac set ch status 57 off
Sending: mac set ch status 58 off
Sending: mac set ch status 59 off
Sending: mac set ch status 60 off
Sending: mac set ch status 61 off
Sending: mac set ch status 62 off
Sending: mac set ch status 63 off
Sending: mac set ch status 64 off
Sending: mac set ch status 65 on
Sending: mac set ch status 66 off
Sending: mac set ch status 67 off
Sending: mac set ch status 68 off
Sending: mac set ch status 69 off
Sending: mac set ch status 70 off
Sending: mac set ch status 71 off
Sending: mac set pwridx 5
Sending: mac set retx 7
Sending: mac set dr 0
Sending: mac join abp 
Personalize accepted. Status: 00000001
jpmeijers commented 2 years ago

@danalvarez thanks for this report. Unfortunately this slipped by in my tests, because I only have devices with RN2483 modules on them to test on. Also this issue only seems to affect ABP and not OTAA.

I agree that the check for channel indexes greater than 15 is invalid, as on the RN2903 there are 71 channels. I propose we do a hotfix to remove this check.

It seems like the problem here is that the new version does not explicitly disable channels 16 through 64 and 66 through 71 for FSB 2. All channels are enabled by default according to the command reference manual. So your device would be transmitting on FSB 2 through 8, and not only on FSB 2. So you only have a 1 in 7 chance of getting a message through. In the case of OTAA this could be beneficial, as after 8 join tries - once per FSB - the join accept will configure the active channels.

In the greater scheme of things we need to rework the setting of channels for the different frequency plans, and do this differently for ABP and OTAA. We've decided to do that in a separate PR: https://github.com/TheThingsNetwork/arduino-device-lib/pull/277#discussion_r776226107

flhofer commented 2 years ago

@danalvarez Yes, sorry, I overlooked that. I developed the methods for 868MHz usages, where there is no channel >15. Just remove the check. I will verify in a second moment what happens when you want to write >ch15 on an RN24xx. thanks @jpmeijers for the quick fix

danalvarez commented 2 years ago

Thank you @jpmeijers for the quick fix! That closes this issue for RN2903. Have a good day, everyone.