<p>pespin <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/25386">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Jenkins Builder: Verified
  osmith: Looks good to me, but someone else must approve
  pespin: Looks good to me, approved

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">TA loop: Take into account UL SACCH 'Actual Timing advance' field<br><br>First step improving and reworking TA loop:<br>Move trigger of the loop to similar place done by BS/MS Power Control<br>Loop, that is, upon receivial of UL SACCH block, which contains<br>information about the TA used to transmit the block encode in L1SACCH<br>Header. Hence, from computed received TOA and TA used when transmitting<br>from the MS, we can infer the desired TA to be used by the MS, which<br>will send back to it later during DL SACCH block.<br><br>The values taken are actually the ones calculated for the previous SACCH<br>block and stored in lchan->meas.ms_toa256. That's because L1SACCH<br>contains the TA used for the previous reporting period, similarly to<br>what's specified for MS Power Control loop (TS 45.008 sec 4.2):<br>"""<br>The MS shall confirm the power control level that it is currently employing<br>in the SACCH L1 header on each uplink  channel. The indicated value shall<br>be the power control level actually used by the mobile for the last burst<br>of the  previous SACCH period.<br>"""<br>(The reader may observe that currently this is not properly done for MS<br>Power Control loop when calling lchan_ms_pwr_ctrl(): this is a bug.)<br><br>This new method also permits changing TA quicker, since we have more<br>confidence that the TA we request is aligned with the one used to<br>transmit, and we don't simply increment/decrement based on the value we<br>request to transmit.<br><br>Related: SYS#5371<br>Change-Id: I2d0f128c8dcac93ee382283a1c91fca76623b8fc<br>---<br>M include/osmo-bts/ta_control.h<br>M src/common/l1sap.c<br>M src/common/measurement.c<br>M src/common/ta_control.c<br>M tests/ta_control/ta_control_test.c<br>5 files changed, 74 insertions(+), 28 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmo-bts/ta_control.h b/include/osmo-bts/ta_control.h</span><br><span>index 168f14a..bf99331 100644</span><br><span>--- a/include/osmo-bts/ta_control.h</span><br><span>+++ b/include/osmo-bts/ta_control.h</span><br><span>@@ -2,4 +2,4 @@</span><br><span> </span><br><span> #include <osmo-bts/gsm_data.h></span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-void lchan_ms_ta_ctrl(struct gsm_lchan *lchan);</span><br><span style="color: hsl(120, 100%, 40%);">+void lchan_ms_ta_ctrl(struct gsm_lchan *lchan, uint8_t ms_tx_ta, int16_t toa256);</span><br><span>diff --git a/src/common/l1sap.c b/src/common/l1sap.c</span><br><span>index e9d58ce..92abbff 100644</span><br><span>--- a/src/common/l1sap.c</span><br><span>+++ b/src/common/l1sap.c</span><br><span>@@ -53,6 +53,7 @@</span><br><span> #include <osmo-bts/bts_model.h></span><br><span> #include <osmo-bts/handover.h></span><br><span> #include <osmo-bts/power_control.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmo-bts/ta_control.h></span><br><span> #include <osmo-bts/msg_utils.h></span><br><span> #include <osmo-bts/pcuif_proto.h></span><br><span> #include <osmo-bts/cbch.h></span><br><span>@@ -1604,6 +1605,7 @@</span><br><span>                      rsl_tx_meas_res(lchan, NULL, 0, le);</span><br><span> </span><br><span>                     radio_link_timeout(lchan, true);</span><br><span style="color: hsl(120, 100%, 40%);">+                      lchan_ms_ta_ctrl(lchan, lchan->rqd_ta, lchan->meas.ms_toa256);</span><br><span>                         lchan_ms_pwr_ctrl(lchan, lchan->ms_power_ctrl.current, data_ind->rssi, data_ind->lqual_cb);</span><br><span>                 }</span><br><span>            return -EINVAL;</span><br><span>@@ -1632,6 +1634,19 @@</span><br><span>             lchan->meas.l1_info.ta = l1_hdr->ta;</span><br><span>           lchan->meas.flags |= LC_UL_M_F_L1_VALID;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+               /* 3GPP TS 45.008 sec 4.2: UL L1 SACCH Header contains TA and</span><br><span style="color: hsl(120, 100%, 40%);">+          * MS_PWR used "for the last burst of the previous SACCH</span><br><span style="color: hsl(120, 100%, 40%);">+          * period". Since MS must use the values provided in DL SACCH</span><br><span style="color: hsl(120, 100%, 40%);">+             * starting at next meas period, the value of the "last burst"</span><br><span style="color: hsl(120, 100%, 40%);">+               * is actually the value used in the entire meas period. Since</span><br><span style="color: hsl(120, 100%, 40%);">+                 * it contains info about the previous meas period, we want to</span><br><span style="color: hsl(120, 100%, 40%);">+                 * feed the Control Loop with the measurements for the same</span><br><span style="color: hsl(120, 100%, 40%);">+            * period (the previous one), which is stored in lchan->meas(.ul_res): */</span><br><span style="color: hsl(120, 100%, 40%);">+          lchan_ms_ta_ctrl(lchan, l1_hdr->ta, lchan->meas.ms_toa256);</span><br><span style="color: hsl(120, 100%, 40%);">+             /* FIXME: lchan_ms_pwr_ctrl() is currently being passed data_ind->lqual_cb, which is wrong because:</span><br><span style="color: hsl(120, 100%, 40%);">+                 * 1- It contains measurement data for 1 SACCH block only, not the average over the entire period</span><br><span style="color: hsl(120, 100%, 40%);">+              * 2- It contains measurement data for *current* meas period, not *previous* one.</span><br><span style="color: hsl(120, 100%, 40%);">+              */</span><br><span>          lchan_ms_pwr_ctrl(lchan, l1_hdr->ms_pwr, data_ind->rssi, data_ind->lqual_cb);</span><br><span>               lchan_bs_pwr_ctrl(lchan, (const struct gsm48_hdr *) &data[5]);</span><br><span>   } else</span><br><span>diff --git a/src/common/measurement.c b/src/common/measurement.c</span><br><span>index 4c49dc9..a4cc668 100644</span><br><span>--- a/src/common/measurement.c</span><br><span>+++ b/src/common/measurement.c</span><br><span>@@ -10,7 +10,6 @@</span><br><span> #include <osmo-bts/measurement.h></span><br><span> #include <osmo-bts/scheduler.h></span><br><span> #include <osmo-bts/rsl.h></span><br><span style="color: hsl(0, 100%, 40%);">-#include <osmo-bts/ta_control.h></span><br><span> </span><br><span> /* Tables as per TS 45.008 Section 8.3 */</span><br><span> static const uint8_t ts45008_83_tch_f[] = { 52, 53, 54, 55, 56, 57, 58, 59 };</span><br><span>@@ -739,11 +738,6 @@</span><br><span> </span><br><span>    lchan_meas_compute_extended(lchan);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Compute new ta_req value. This has to be done here since the value</span><br><span style="color: hsl(0, 100%, 40%);">-    * in lchan->meas.num_ul_meas together with lchan->meas.ms_toa256</span><br><span style="color: hsl(0, 100%, 40%);">-  * is needed for the computation. */</span><br><span style="color: hsl(0, 100%, 40%);">-    lchan_ms_ta_ctrl(lchan);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>     lchan->meas.num_ul_meas = 0;</span><br><span> </span><br><span>  /* return 1 to indicate that the computation has been done and the next</span><br><span>diff --git a/src/common/ta_control.c b/src/common/ta_control.c</span><br><span>index ccb60e2..ac9f255 100644</span><br><span>--- a/src/common/ta_control.c</span><br><span>+++ b/src/common/ta_control.c</span><br><span>@@ -1,6 +1,7 @@</span><br><span> /* Loop control for Timing Advance */</span><br><span> </span><br><span> /* (C) 2013 by Andreas Eversberg <jolly@eversberg.eu></span><br><span style="color: hsl(120, 100%, 40%);">+ * (C) 2021 by sysmocom - s.m.f.c. GmbH <info@sysmocom.de></span><br><span>  *</span><br><span>  * All Rights Reserved</span><br><span>  *</span><br><span>@@ -19,6 +20,8 @@</span><br><span>  *</span><br><span>  */</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* Related specs: 3GPP TS 45.010 sections 5.5, 5.6 */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #include <osmo-bts/gsm_data.h></span><br><span> #include <osmo-bts/logging.h></span><br><span> </span><br><span>@@ -29,22 +32,60 @@</span><br><span> #define TA_MIN 0</span><br><span> #define TA_MAX 63</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-void lchan_ms_ta_ctrl(struct gsm_lchan *lchan)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">-       int16_t toa256 = lchan->meas.ms_toa256;</span><br><span style="color: hsl(120, 100%, 40%);">+/* TODO: make configurable over osmo-bts VTY? Pass it BSC->BTS? */</span><br><span style="color: hsl(120, 100%, 40%);">+#define TA_MAX_INC_STEP 1</span><br><span style="color: hsl(120, 100%, 40%);">+#define TA_MAX_DEC_STEP 1</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-     if (toa256 < -TOA256_9OPERCENT && lchan->rqd_ta > TA_MIN) {</span><br><span style="color: hsl(0, 100%, 40%);">-            LOGPLCHAN(lchan, DLOOP, LOGL_INFO,</span><br><span style="color: hsl(0, 100%, 40%);">-                        "TOA is too early (%d), now lowering TA from %d to %d\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                     toa256, lchan->rqd_ta, lchan->rqd_ta - 1);</span><br><span style="color: hsl(0, 100%, 40%);">-              lchan->rqd_ta--;</span><br><span style="color: hsl(0, 100%, 40%);">-     } else if (toa256 > TOA256_9OPERCENT && lchan->rqd_ta < TA_MAX) {</span><br><span style="color: hsl(0, 100%, 40%);">-              LOGPLCHAN(lchan, DLOOP, LOGL_INFO,</span><br><span style="color: hsl(0, 100%, 40%);">-                        "TOA is too late (%d), now raising TA from %d to %d\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                       toa256, lchan->rqd_ta, lchan->rqd_ta + 1);</span><br><span style="color: hsl(0, 100%, 40%);">-              lchan->rqd_ta++;</span><br><span style="color: hsl(0, 100%, 40%);">-     } else</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+/*! compute the new "Ordered Timing Advance" communicated to the MS and store it in lchan.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param lchan logical channel for which to compute (and in which to store) new power value.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] ms_tx_ta The TA used by the MS and reported in L1SACCH, see struct gsm_sacch_l1_hdr field "ta".</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] toa256 Time of Arrival (in 1/256th bits) computed at Rx side</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+void lchan_ms_ta_ctrl(struct gsm_lchan *lchan, uint8_t ms_tx_ta, int16_t toa256)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+  int16_t new_ta;</span><br><span style="color: hsl(120, 100%, 40%);">+       /* Shall we skip current block based on configured interval? */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+     /*TODO: implement P_CON_INTERVAL for TA loop */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+     int16_t delta_ta = toa256/256;</span><br><span style="color: hsl(120, 100%, 40%);">+        if (toa256 >= 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+         if ((toa256 - (256 * delta_ta)) > TOA256_9OPERCENT)</span><br><span style="color: hsl(120, 100%, 40%);">+                        delta_ta++;</span><br><span style="color: hsl(120, 100%, 40%);">+           if (delta_ta > TA_MAX_INC_STEP)</span><br><span style="color: hsl(120, 100%, 40%);">+                    delta_ta = TA_MAX_INC_STEP;</span><br><span style="color: hsl(120, 100%, 40%);">+   } else {</span><br><span style="color: hsl(120, 100%, 40%);">+              if ((toa256 - (256 * delta_ta)) < -TOA256_9OPERCENT)</span><br><span style="color: hsl(120, 100%, 40%);">+                       delta_ta--;</span><br><span style="color: hsl(120, 100%, 40%);">+           if (delta_ta < -TA_MAX_DEC_STEP)</span><br><span style="color: hsl(120, 100%, 40%);">+                   delta_ta = -TA_MAX_DEC_STEP;</span><br><span style="color: hsl(120, 100%, 40%);">+  }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   new_ta = ms_tx_ta + delta_ta;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+       /* Make sure new_ta is never negative: */</span><br><span style="color: hsl(120, 100%, 40%);">+     if (new_ta < TA_MIN)</span><br><span style="color: hsl(120, 100%, 40%);">+               new_ta = TA_MIN;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+    /* Don't ask for out of range TA: */</span><br><span style="color: hsl(120, 100%, 40%);">+      if (new_ta > TA_MAX)</span><br><span style="color: hsl(120, 100%, 40%);">+               new_ta = TA_MAX;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+    if (lchan->rqd_ta == (uint8_t)new_ta) {</span><br><span>           LOGPLCHAN(lchan, DLOOP, LOGL_DEBUG,</span><br><span style="color: hsl(0, 100%, 40%);">-                       "TOA is correct (%d), keeping current TA of %d\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                    toa256, lchan->rqd_ta);</span><br><span style="color: hsl(120, 100%, 40%);">+                    "Keeping current TA at %u: TOA was %d\n",</span><br><span style="color: hsl(120, 100%, 40%);">+                   lchan->rqd_ta, toa256);</span><br><span style="color: hsl(120, 100%, 40%);">+          return;</span><br><span style="color: hsl(120, 100%, 40%);">+       }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+   LOGPLCHAN(lchan, DLOOP, LOGL_INFO,</span><br><span style="color: hsl(120, 100%, 40%);">+              "%s TA %u => %u: TOA was too %s (%d)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+                 (uint8_t)new_ta > lchan->rqd_ta ? "Raising" : "Lowering",</span><br><span style="color: hsl(120, 100%, 40%);">+           lchan->rqd_ta, (uint8_t)new_ta,</span><br><span style="color: hsl(120, 100%, 40%);">+            (uint8_t)new_ta > lchan->rqd_ta ? "late" : "early",</span><br><span style="color: hsl(120, 100%, 40%);">+                 toa256);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+  /* store the resulting new TA in the lchan */</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->rqd_ta = (uint8_t)new_ta;</span><br><span> }</span><br><span>diff --git a/tests/ta_control/ta_control_test.c b/tests/ta_control/ta_control_test.c</span><br><span>index 2e981b3..12305cb 100644</span><br><span>--- a/tests/ta_control/ta_control_test.c</span><br><span>+++ b/tests/ta_control/ta_control_test.c</span><br><span>@@ -35,9 +35,6 @@</span><br><span>    uint8_t rqd_ta_before;</span><br><span>       int16_t toa256 = toa256_start;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-      /* Arbitrary value, high enough so that a computation can happen. */</span><br><span style="color: hsl(0, 100%, 40%);">-    lchan.meas.num_ul_meas = 10;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span>         printf("toa256_start = %u / 256 = %u, steps = %u\n", toa256_start,</span><br><span>                toa256_start / 256, steps);</span><br><span> </span><br><span>@@ -49,8 +46,7 @@</span><br><span> </span><br><span>               rqd_ta_before = lchan.rqd_ta;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-               lchan.meas.ms_toa256 = toa256;</span><br><span style="color: hsl(0, 100%, 40%);">-          lchan_ms_ta_ctrl(&lchan);</span><br><span style="color: hsl(120, 100%, 40%);">+         lchan_ms_ta_ctrl(&lchan, rqd_ta_before, toa256);</span><br><span> </span><br><span>             rqd_ta_after = lchan.rqd_ta;</span><br><span>                 toa256 -= (rqd_ta_after - rqd_ta_before) * 256;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bts/+/25386">change 25386</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-bts/+/25386"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-bts </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I2d0f128c8dcac93ee382283a1c91fca76623b8fc </div>
<div style="display:none"> Gerrit-Change-Number: 25386 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>