Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RegionAU915.c - incorrectly rejecting LinkADRReq #533

Closed
DominusFulguris opened this issue Aug 20, 2018 · 4 comments
Closed

RegionAU915.c - incorrectly rejecting LinkADRReq #533

DominusFulguris opened this issue Aug 20, 2018 · 4 comments

Comments

@DominusFulguris
Copy link

Due to a change in regional parameters for AU915, the initial minimum data rate was changed from DR0 (UplinkDwellTime=0) to DR2 (UplinkDwellTime=1). The result is that a newer device connecting to an older gateway will incorrectly reject a LinkADRReq because the requested data rate is lower than the current minimum, even though the device's minimum data rate is valid for the channels in the channel mask.

The relevant statement from the LoRaWAN specification (1.02, 1.03, 1.1) is:

"A bit in the ChMask field set to 1 means that the corresponding channel can be used for uplink transmissions if this channel allows the data rate currently used by the end-device."

As the device's minimum data rate at the time is still valid for the channel, it is not valid to reject the LinkADRReq.

The function RegionAU915LinkAdrReq() should be altered to allow the requested LinkADRReq data rate to be modified (upwards) to the current minimum.

// Get the minimum possible datarate
getPhy.Attribute = PHY_MIN_TX_DR;
getPhy.UplinkDwellTime = linkAdrReq->UplinkDwellTime;
phyParam = RegionAU915GetPhyParam( &getPhy );

// If the requested data rate is lower than the minimum data rate,
// use the minimum instead.
if( linkAdrParams.Datarate < ( int8_t )phyParam.Value )
{
	linkAdrParams.Datarate = ( int8_t )phyParam.Value;
}

Note: This issue may also affect RegionUS915.c

@jamesl-dm
Copy link

Surely the proper solution to this problem is to instruct your network to use the newer regional params for this device? If a device receives a command to switch to a not-currently-achievable datarate, you'd expect it to reject the command, not re-interpret it?

@DominusFulguris
Copy link
Author

It is a good question and I have put some thought into it.

In terms of semantics, the precedent is already there in the same command - if ther server requests a Tx power level that the node cannot produce, it will use the tx power it can, thus "re-interpreting" the request.

Secondly, if the server was updated to request DR2 instead of DR0, this would only end up achieving the same result: the ADR sequence using DR2.

Thirdly, for AU915, the UplinkDwellTime = 1 is not fixed. The regional parameters allow it to be changed by the server. If the server assumes DR2 only is valid, then it cannot do a DR0-based ADR retrain at some future point when UplinkDwellTime = 0 unless it also tracked the nodes state, which would be unreliable.

Fourthly, I see the current situation as a "breaking" change as newer nodes are not compatible with older network server versions. The change I am suggesting resolves the backwards compatibility issue. Remember, the consequence of the current situation is a severe reduction in the performance of the node (as it is sending data across all 72 channels) while also wasting/polluting the bandwidth for the same reason.

@DominusFulguris
Copy link
Author

Looks like this issue may affect all regions. Perhaps the fix should be placed in RegionCommon.c/RegionCommonLinkAdrReqVerifyParams() instead.

if( datarate < verifyParams->MinDatarate)
{
	datarate = verifyParams->MinDatarate;
}

@mluis1
Copy link
Contributor

mluis1 commented Feb 12, 2019

We believe that the current implementation is compliant to the LoRaWAN Specification v1.0.3 and LoRaWAN Regional Parameters v1.0.3revA specifications.

It means that the network server must also be using the same set of specifications.

@mluis1 mluis1 closed this as completed Feb 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants