search

mcci-catena / arduino-lmic

LoraWAN-MAC-in-C library, adapted to run under the Arduino environment
https://forum.mcci.io/c/device-software/arduino-lmic/
MIT License
636 stars 207 forks source link

AS923 on TTNv3 keeps exchanging LinkADRReq - LinkADRAns NACK and doesn't use one channel #686

Open nielskoot opened 3 years ago

nielskoot commented 3 years ago

I did some analysis and I think the issue is that AS923 only has 2 default channels unlike other EU like regions that have 3.

In the handling of the join-accept (function processJoinAccept in lmic.c) the channels in the CFList are hard coded as 3-7:

    for( u1_t chidx=3; chidx<8; chidx++, dlen+=3 ) {

However for AS923 the channels in the CFList are 2-6 (see the regional parameters for AS923). The same applies for RU864, however that isn't supported yet in LMIC.

Due to the wrong index the channels configured in the device after a join with 5 extra channels is: 0,1,3,4,5,6,7 while the network thinks the channels are: 0,1,2,3,4,5,6.

This is not immediately an issue, the device supports channels 0-15. However when additional channels are added things start to go wrong. The first extra channel added by the network will have index 7, however the devices already has channel 7 so instead of adding the channel the channel is replaced in the device. This causes the replaced channel (channel 6 on the network) to be unused.

A worse problem occurs with ADR, the LinkADRReq ChMask set by the network will include channel 2, however the device doesn't have channel 2 so will return a LinkADRReq with a reject on the channel mask (Channel mask ACK = 0) as specified in the standard "The channel mask sent enables a yet undefined channel". E.g. on TTNv3 this causes the LinkADRReq to be retried endlessly causing a constant max. duty cycle load.

I didn't see this on TTNv2 before, not sure why, but I suspect TTNv2 never added extra channels beyond the ones in the join-accept CFList and maybe handled the ADR differently.

Processing the channels as 2-6 for AS923 in the above loop works, I tested that. However I am not sure about the best approach for a change that works for all regions. Using the number of default channels looks the most logical to me, something like this:

    for( u1_t chidx=NUM_DEFAULT_CHANNELS; chidx<(NUM_DEFAULT_CHANNELS+5); chidx++, dlen+=3 ) {

However NUM_DEFAULT_CHANNELS is not available in lmic.c, only in region specific c files.

terrillmoore commented 3 years ago

Thanks for the careful analysis. I think you're right; and I think my testing would not have found this. (I don't think the AS923 compliance test checks this exactly.) I'll look into how to fix this.

terrillmoore commented 3 years ago

The lack of NUM_DEFAULT_CHANNELS as a portable concept is a great pain. I ran into this doing https://github.com/mcci-catena/arduino-lorawan/pull/150, which properly saves/restores the LMIC state across system reset.