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

ADR does not change data rate #3901

Closed
frbehrens opened this issue Mar 6, 2021 · 4 comments · Fixed by #4034
Closed

ADR does not change data rate #3901

frbehrens opened this issue Mar 6, 2021 · 4 comments · Fixed by #4034
Assignees
Labels
c/network server This is related to the Network Server in progress We're working on it
Milestone

Comments

@frbehrens
Copy link

Summary

After a (manually) change of data rate and using ADR the network calculates
new settings and changes TX power, but not the data rate.

That behaviour is different to the TTN v2 stack and I believe it is
caused by a missing transfer of last received data rate to the internal
MAC state.

Steps to Reproduce

  1. The device joins and sends a confirmed uplink.
  2. The device receives a downlink with adr request:
    link_adr_req: dataRate=5 txPower=15 chMask=00ff chMaskCntl=0 nbTrans=1
  3. The device sends some (unconfirmed) uplinks.
  4. The data rate on device is changed (manually) to DR2/SF10.
  5. With the next uplink a downlink is received with adr request to decrease power:
    link_adr_req: dataRate=15 txPower=4 chMask=00ff chMaskCntl=0 nbTrans=1
  6. With the next uplink the adr request is confirmed by device
    link_adr_ans: powerOK=true dataRateOK=true channelMaskOK=true
    and the device receives a new request in order to further decrease the power:
    link_adr_req: dataRate=15 txPower=7 chMask=00ff chMaskCntl=0 nbTrans=1
  7. That request is confirmed as well, but the device does not receive any further adr
    change requests, even after more than 100 downlinks.

The internal ADR mac state displayed by ttn-lw-cli is:

  "mac_settings": {
  },
  "mac_state": {
    "current_parameters": {
      "adr_data_rate_index": 5
    },
    "desired_parameters": {
      "adr_data_rate_index": 5
    },
    "last_adr_change_f_cnt_up": 8
  }

So it looks to me, that the currently asssumed data rate on
network is not correct. When I overwrite the setting with

ttn-lw-cli dev set ... --mac-state.current-parameters.adr-data-rate-index 2

the device receives the expected adr request with next uplink:
link_adr_req: dataRate=5 txPower=0 chMask=00ff chMaskCntl=0 nbTrans=1

What do you see now?

The device receives an adr request with power change, but no rate change:
link_adr_req: dataRate=15 txPower=4 chMask=00ff chMaskCntl=0 nbTrans=1

What do you want to see instead?

The device should receive an adr request with rate change back to DR5/SF7:
link_adr_req: dataRate=5 txPower=0 chMask=00ff chMaskCntl=0 nbTrans=1

Environment

I use a LoRaWAN 1.0.4 compatible node, that was already used on the TTN v2 stack.
The region is EU-868 on eu1.cloud.thethings.network, v3.11.1.
The transmission conditions are very good, there is no packet loss with DR5/SF7
and SNR on network is between 9 and 12.

How do you propose to implement this?

I believe that the data rate of last received uplink is not copied to internal
MAC state. At a first glance I could not find in the code the assignment for
MACState.CurrentParameters.ADRDataRateIndex.

How do you propose to test this?

see steps above.

Can you do this yourself and submit a Pull Request?

No, I don't have a test environment.

@github-actions github-actions bot added the needs/triage We still need to triage this label Mar 6, 2021
@rvolosatovs rvolosatovs self-assigned this Mar 9, 2021
@rvolosatovs rvolosatovs added the c/network server This is related to the Network Server label Mar 9, 2021
@rvolosatovs rvolosatovs added this to the v3.11.3 milestone Mar 9, 2021
@johanstokking johanstokking removed the needs/triage We still need to triage this label Mar 9, 2021
@rvolosatovs
Copy link
Contributor

Thank you for reporting. This is actually a great idea to use the uplink data rate index and will solve quite some issues with LinkADR in US-like bands.

@frbehrens
Copy link
Author

Thank you for handling the problem.
But I don't see this as an idea, isn't that the way ADR works and that it has been implemented in the TTN v2 stack for a long time? How should the network calculate a new rate if it does not use the current uplink data rate as a basis?

@rvolosatovs
Copy link
Contributor

rvolosatovs commented Mar 10, 2021

2021-03-10-16:25:14-screenshot
This is what the 1.0.4 spec has to say about it.

So, data rate index 0 is expected to be used initially as per the spec, unless device is physically incapable of using it (in which case that should be communicated out-of-band). Additionally, once ADR procedure is in place, device might lose connectivity and may lower data rate.

Given both these facts and no way for NS to figure out the minimum data rate of the device it indeed makes sense to rely on the data rate of the uplink and assume it's either the "default" value or device lost connectivity and switched to lower data rate. In all other cases the data rate must be equal to what NS configured in LoRaWAN-compliant devices.

So, to answer your question - yes and no, according to the LoRaWAN spec the uplink data rate can only differ from what NS expects when device loses connectivity, and, hence, switch of data rate, marks a "reset" of ADR algorithm with uplink data rate used as the "default" value and is the only time NS should actually use it as a basis for ADR.

@rvolosatovs rvolosatovs modified the milestones: v3.11.3, v3.11.4, v3.12.0 Mar 16, 2021
@htdvisser htdvisser modified the milestones: v3.12.0, 2021 Q2 Mar 26, 2021
@rvolosatovs rvolosatovs modified the milestones: 2021 Q2, v3.12.0 Mar 26, 2021
@rvolosatovs rvolosatovs added the in progress We're working on it label Mar 26, 2021
@nejraselimovic nejraselimovic modified the milestones: v3.12.0, v3.12.1 Apr 6, 2021
@johanstokking johanstokking removed the in progress We're working on it label Apr 7, 2021
@johanstokking
Copy link
Member

This one looks stale. @rvolosatovs please re-add s/in progress if in progress.

@rvolosatovs rvolosatovs added the in progress We're working on it label Apr 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c/network server This is related to the Network Server in progress We're working on it
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants