<p>pespin <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/25442">View Change</a></p><div style="white-space:pre-wrap">Approvals:
pespin: Looks good to me, approved; Verified
laforge: Looks good to me, but someone else must approve
osmith: Looks good to me, but someone else must approve
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">MS Power Control Loop: Feed UL C/I from correct measurement period<br><br>As per 3GPP TS 45.008 sec 4.2, the ms_pwr received in L1 SACCH Header is<br>the value used over previous measurement period. Hence, we need to feed<br>the algo with the measurements taken over that same period.<br><br>Related: SYS#4917<br>Change-Id: I13c0014fdd73f823ae5b1256c35bfa7d97cfa334<br>---<br>M include/osmo-bts/gsm_data.h<br>M src/common/l1sap.c<br>M src/common/measurement.c<br>M tests/meas/meas_testcases.h<br>4 files changed, 51 insertions(+), 27 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/include/osmo-bts/gsm_data.h b/include/osmo-bts/gsm_data.h</span><br><span>index e2a1ae9..0869426 100644</span><br><span>--- a/include/osmo-bts/gsm_data.h</span><br><span>+++ b/include/osmo-bts/gsm_data.h</span><br><span>@@ -105,8 +105,8 @@</span><br><span> uint16_t ber10k;</span><br><span> /* timing advance offset (in 1/256 bits) */</span><br><span> int16_t ta_offs_256bits;</span><br><span style="color: hsl(0, 100%, 40%);">- /* C/I ratio in dB */</span><br><span style="color: hsl(0, 100%, 40%);">- float c_i;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* C/I ratio in cB */</span><br><span style="color: hsl(120, 100%, 40%);">+ int16_t c_i;</span><br><span> /* flags */</span><br><span> uint8_t is_sub:1;</span><br><span> /* RSSI in dBm * -1 */</span><br><span>@@ -339,6 +339,8 @@</span><br><span> struct rsl_l1_info l1_info;</span><br><span> struct gsm_meas_rep_unidir ul_res;</span><br><span> int16_t ms_toa256;</span><br><span style="color: hsl(120, 100%, 40%);">+ int16_t ul_ci_cb_full;</span><br><span style="color: hsl(120, 100%, 40%);">+ int16_t ul_ci_cb_sub;</span><br><span> /* Frame number of the last measurement indication receceived */</span><br><span> uint32_t last_fn;</span><br><span> /* Osmocom extended measurement results, see LC_UL_M_F_EXTD_VALID */</span><br><span>diff --git a/src/common/l1sap.c b/src/common/l1sap.c</span><br><span>index a641dab..40e818d 100644</span><br><span>--- a/src/common/l1sap.c</span><br><span>+++ b/src/common/l1sap.c</span><br><span>@@ -681,6 +681,7 @@</span><br><span> uint8_t is_sub;</span><br><span> int16_t ta_offs_256bits;</span><br><span> uint16_t ber10k;</span><br><span style="color: hsl(120, 100%, 40%);">+ int16_t ci_cb;</span><br><span> const char *ind_name;</span><br><span> </span><br><span> switch (ind_type) {</span><br><span>@@ -693,6 +694,7 @@</span><br><span> is_sub = info_meas_ind->is_sub;</span><br><span> ta_offs_256bits = info_meas_ind->ta_offs_256bits;</span><br><span> ber10k = info_meas_ind->ber10k;</span><br><span style="color: hsl(120, 100%, 40%);">+ ci_cb = info_meas_ind->c_i_cb;</span><br><span> ind_name = "MPH INFO";</span><br><span> break;</span><br><span> case PRIM_TCH:</span><br><span>@@ -705,6 +707,7 @@</span><br><span> is_sub = ph_tch_ind->is_sub;</span><br><span> ta_offs_256bits = ph_tch_ind->ta_offs_256bits;</span><br><span> ber10k = ph_tch_ind->ber10k;</span><br><span style="color: hsl(120, 100%, 40%);">+ ci_cb = ph_tch_ind->lqual_cb;</span><br><span> ind_name = "TCH";</span><br><span> break;</span><br><span> case PRIM_PH_DATA:</span><br><span>@@ -717,6 +720,7 @@</span><br><span> is_sub = ph_data_ind->is_sub;</span><br><span> ta_offs_256bits = ph_data_ind->ta_offs_256bits;</span><br><span> ber10k = ph_data_ind->ber10k;</span><br><span style="color: hsl(120, 100%, 40%);">+ ci_cb = ph_data_ind->lqual_cb;</span><br><span> ind_name = "DATA";</span><br><span> break;</span><br><span> default:</span><br><span>@@ -732,9 +736,9 @@</span><br><span> }</span><br><span> </span><br><span> DEBUGPFN(DL1P, fn,</span><br><span style="color: hsl(0, 100%, 40%);">- "%s %s meas ind, ta_offs_256bits=%d, ber10k=%d, inv_rssi=%u\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ "%s %s meas ind, ta_offs_256bits=%d, ber10k=%d, inv_rssi=%u, C/I=%d cB\n",</span><br><span> gsm_lchan_name(lchan), ind_name, ta_offs_256bits, ber10k,</span><br><span style="color: hsl(0, 100%, 40%);">- inv_rssi);</span><br><span style="color: hsl(120, 100%, 40%);">+ inv_rssi, ci_cb);</span><br><span> </span><br><span> /* in the GPRS case we are not interested in measurement</span><br><span> * processing. The PCU will take care of it */</span><br><span>@@ -744,6 +748,7 @@</span><br><span> memset(&ulm, 0, sizeof(ulm));</span><br><span> ulm.ta_offs_256bits = ta_offs_256bits;</span><br><span> ulm.ber10k = ber10k;</span><br><span style="color: hsl(120, 100%, 40%);">+ ulm.c_i = ci_cb;</span><br><span> ulm.inv_rssi = inv_rssi;</span><br><span> ulm.is_sub = is_sub;</span><br><span> </span><br><span>@@ -1526,11 +1531,11 @@</span><br><span> uint8_t chan_nr, link_id;</span><br><span> uint8_t tn;</span><br><span> uint32_t fn;</span><br><span style="color: hsl(0, 100%, 40%);">- int8_t rssi;</span><br><span> enum osmo_ph_pres_info_type pr_info = data_ind->pdch_presence_info;</span><br><span> struct gsm_sacch_l1_hdr *l1_hdr;</span><br><span style="color: hsl(120, 100%, 40%);">+ int8_t ul_rssi;</span><br><span style="color: hsl(120, 100%, 40%);">+ int16_t ul_ci_cb;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- rssi = data_ind->rssi;</span><br><span> chan_nr = data_ind->chan_nr;</span><br><span> link_id = data_ind->link_id;</span><br><span> fn = data_ind->fn;</span><br><span>@@ -1578,7 +1583,7 @@</span><br><span> </span><br><span> /* PDTCH / PACCH frame handling */</span><br><span> pcu_tx_data_ind(&trx->ts[tn], PCU_IF_SAPI_PDTCH, fn, trx->arfcn,</span><br><span style="color: hsl(0, 100%, 40%);">- L1SAP_FN2MACBLOCK(fn), data, len, rssi, data_ind->ber10k,</span><br><span style="color: hsl(120, 100%, 40%);">+ L1SAP_FN2MACBLOCK(fn), data, len, data_ind->rssi, data_ind->ber10k,</span><br><span> data_ind->ta_offs_256bits/64, data_ind->lqual_cb);</span><br><span> return 0;</span><br><span> }</span><br><span>@@ -1606,7 +1611,15 @@</span><br><span> </span><br><span> radio_link_timeout(lchan, true);</span><br><span> lchan_ms_ta_ctrl(lchan, lchan->ta_ctrl.current, lchan->meas.ms_toa256);</span><br><span style="color: hsl(0, 100%, 40%);">- lchan_ms_pwr_ctrl(lchan, lchan->ms_power_ctrl.current, data_ind->rssi, data_ind->lqual_cb);</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If DTx is active on Downlink, use the '-SUB', otherwise '-FULL': */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan->tch.dtx.dl_active) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ul_rssi = rxlev2dbm(lchan->meas.ul_res.sub.rx_lev);</span><br><span style="color: hsl(120, 100%, 40%);">+ ul_ci_cb = lchan->meas.ul_ci_cb_full;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ ul_rssi = rxlev2dbm(lchan->meas.ul_res.full.rx_lev);</span><br><span style="color: hsl(120, 100%, 40%);">+ ul_ci_cb = lchan->meas.ul_ci_cb_sub;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan_ms_pwr_ctrl(lchan, lchan->ms_power_ctrl.current, ul_rssi, ul_ci_cb);</span><br><span> }</span><br><span> return -EINVAL;</span><br><span> }</span><br><span>@@ -1616,7 +1629,6 @@</span><br><span> handover_frame(lchan);</span><br><span> </span><br><span> if (L1SAP_IS_LINK_SACCH(link_id)) {</span><br><span style="color: hsl(0, 100%, 40%);">- int8_t ul_rssi;</span><br><span> radio_link_timeout(lchan, false);</span><br><span> le = &lchan->lapdm_ch.lapdm_acch;</span><br><span> /* save the SACCH L1 header in the lchan struct for RSL MEAS RES */</span><br><span>@@ -1644,16 +1656,15 @@</span><br><span> * feed the Control Loop with the measurements for the same</span><br><span> * period (the previous one), which is stored in lchan->meas(.ul_res): */</span><br><span> lchan_ms_ta_ctrl(lchan, l1_hdr->ta, lchan->meas.ms_toa256);</span><br><span style="color: hsl(0, 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(0, 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(0, 100%, 40%);">- * 2- It contains measurement data for *current* meas period, not *previous* one.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span> /* If DTx is active on Downlink, use the '-SUB', otherwise '-FULL': */</span><br><span style="color: hsl(0, 100%, 40%);">- if (lchan->tch.dtx.dl_active)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan->tch.dtx.dl_active) {</span><br><span> ul_rssi = rxlev2dbm(lchan->meas.ul_res.sub.rx_lev);</span><br><span style="color: hsl(0, 100%, 40%);">- else</span><br><span style="color: hsl(120, 100%, 40%);">+ ul_ci_cb = lchan->meas.ul_ci_cb_full;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span> ul_rssi = rxlev2dbm(lchan->meas.ul_res.full.rx_lev);</span><br><span style="color: hsl(0, 100%, 40%);">- lchan_ms_pwr_ctrl(lchan, l1_hdr->ms_pwr, ul_rssi, data_ind->lqual_cb);</span><br><span style="color: hsl(120, 100%, 40%);">+ ul_ci_cb = lchan->meas.ul_ci_cb_sub;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan_ms_pwr_ctrl(lchan, l1_hdr->ms_pwr, ul_rssi, ul_ci_cb);</span><br><span> lchan_bs_pwr_ctrl(lchan, (const struct gsm48_hdr *) &data[5]);</span><br><span> } else</span><br><span> le = &lchan->lapdm_ch.lapdm_dcch;</span><br><span>diff --git a/src/common/measurement.c b/src/common/measurement.c</span><br><span>index a4cc668..a1c91a9 100644</span><br><span>--- a/src/common/measurement.c</span><br><span>+++ b/src/common/measurement.c</span><br><span>@@ -341,7 +341,7 @@</span><br><span> if (!ulm->is_sub)</span><br><span> dest->is_sub = ts45008_83_is_sub(lchan, fn);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- DEBUGPFN(DMEAS, fn, "%s adding measurement (ber10k=%u, ta_offs=%d, ci=%0.2f, is_sub=%u, rssi=-%u), num_ul_meas=%d, fn_mod=%u\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ DEBUGPFN(DMEAS, fn, "%s adding measurement (ber10k=%u, ta_offs=%d, ci_cB=%d, is_sub=%u, rssi=-%u), num_ul_meas=%d, fn_mod=%u\n",</span><br><span> gsm_lchan_name(lchan), ulm->ber10k, ulm->ta_offs_256bits,</span><br><span> ulm->c_i, dest->is_sub, ulm->inv_rssi, lchan->meas.num_ul_meas,</span><br><span> fn_mod);</span><br><span>@@ -555,8 +555,10 @@</span><br><span> struct gsm_meas_rep_unidir *mru;</span><br><span> uint32_t ber_full_sum = 0;</span><br><span> uint32_t irssi_full_sum = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ int32_t ci_full_sum = 0;</span><br><span> uint32_t ber_sub_sum = 0;</span><br><span> uint32_t irssi_sub_sum = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ int32_t ci_sub_sum = 0;</span><br><span> int32_t ta256b_sum = 0;</span><br><span> unsigned int num_meas_sub = 0;</span><br><span> unsigned int num_meas_sub_actual = 0;</span><br><span>@@ -624,11 +626,13 @@</span><br><span> m = &lchan->meas.uplink[i + num_ul_meas_excess];</span><br><span> if (m->is_sub) {</span><br><span> irssi_sub_sum += m->inv_rssi;</span><br><span style="color: hsl(120, 100%, 40%);">+ ci_sub_sum += m->c_i;</span><br><span> num_meas_sub_actual++;</span><br><span> is_sub = true;</span><br><span> }</span><br><span> irssi_full_sum += m->inv_rssi;</span><br><span> ta256b_sum += m->ta_offs_256bits;</span><br><span style="color: hsl(120, 100%, 40%);">+ ci_full_sum += m->c_i;</span><br><span> </span><br><span> num_ul_meas_actual++;</span><br><span> } else {</span><br><span>@@ -697,27 +701,32 @@</span><br><span> else</span><br><span> irssi_full_sum = irssi_full_sum / num_ul_meas_actual;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!num_ul_meas_actual)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!num_ul_meas_actual) {</span><br><span> ta256b_sum = lchan->meas.ms_toa256;</span><br><span style="color: hsl(0, 100%, 40%);">- else</span><br><span style="color: hsl(120, 100%, 40%);">+ ci_full_sum = lchan->meas.ul_ci_cb_full;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span> ta256b_sum = ta256b_sum / (signed)num_ul_meas_actual;</span><br><span style="color: hsl(120, 100%, 40%);">+ ci_full_sum = ci_full_sum / (signed)num_ul_meas_actual;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span> if (!num_meas_sub)</span><br><span> ber_sub_sum = MEASUREMENT_DUMMY_BER;</span><br><span> else</span><br><span> ber_sub_sum = ber_sub_sum / num_meas_sub;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (!num_meas_sub_actual)</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!num_meas_sub_actual) {</span><br><span> irssi_sub_sum = MEASUREMENT_DUMMY_IRSSI;</span><br><span style="color: hsl(0, 100%, 40%);">- else</span><br><span style="color: hsl(120, 100%, 40%);">+ ci_sub_sum = lchan->meas.ul_ci_cb_sub;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span> irssi_sub_sum = irssi_sub_sum / num_meas_sub_actual;</span><br><span style="color: hsl(120, 100%, 40%);">+ ci_sub_sum = ci_sub_sum / (signed)num_meas_sub_actual;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> </span><br><span> LOGPLCHAN(lchan, DMEAS, LOGL_INFO,</span><br><span style="color: hsl(0, 100%, 40%);">- "Computed TA256(% 4d) BER-FULL(%2u.%02u%%), RSSI-FULL(-%3udBm), "</span><br><span style="color: hsl(0, 100%, 40%);">- "BER-SUB(%2u.%02u%%), RSSI-SUB(-%3udBm)\n",</span><br><span style="color: hsl(0, 100%, 40%);">- ta256b_sum, ber_full_sum / 100, ber_full_sum % 100,</span><br><span style="color: hsl(0, 100%, 40%);">- irssi_full_sum, ber_sub_sum / 100, ber_sub_sum % 100,</span><br><span style="color: hsl(0, 100%, 40%);">- irssi_sub_sum);</span><br><span style="color: hsl(120, 100%, 40%);">+ "Computed TA256(% 4d), BER-FULL(%2u.%02u%%), RSSI-FULL(-%3udBm), C/I-FULL(% 4d cB), "</span><br><span style="color: hsl(120, 100%, 40%);">+ "BER-SUB(%2u.%02u%%), RSSI-SUB(-%3udBm), C/I-SUB(% 4d cB)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ ta256b_sum, ber_full_sum / 100, ber_full_sum % 100, irssi_full_sum, ci_full_sum,</span><br><span style="color: hsl(120, 100%, 40%);">+ ber_sub_sum / 100, ber_sub_sum % 100, irssi_sub_sum, ci_sub_sum);</span><br><span> </span><br><span> /* store results */</span><br><span> mru = &lchan->meas.ul_res;</span><br><span>@@ -726,6 +735,8 @@</span><br><span> mru->full.rx_qual = ber10k_to_rxqual(ber_full_sum);</span><br><span> mru->sub.rx_qual = ber10k_to_rxqual(ber_sub_sum);</span><br><span> lchan->meas.ms_toa256 = ta256b_sum;</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->meas.ul_ci_cb_full = ci_full_sum;</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->meas.ul_ci_cb_sub = ci_sub_sum;</span><br><span> </span><br><span> LOGPLCHAN(lchan, DMEAS, LOGL_INFO,</span><br><span> "UL MEAS RXLEV_FULL(%u), RXLEV_SUB(%u), RXQUAL_FULL(%u), RXQUAL_SUB(%u), "</span><br><span>diff --git a/tests/meas/meas_testcases.h b/tests/meas/meas_testcases.h</span><br><span>index d7eee5c..90f0f85 100644</span><br><span>--- a/tests/meas/meas_testcases.h</span><br><span>+++ b/tests/meas/meas_testcases.h</span><br><span>@@ -1,5 +1,5 @@</span><br><span> #define ULM(ber, ta, sub, neg_rssi) \</span><br><span style="color: hsl(0, 100%, 40%);">- { .ber10k = (ber), .ta_offs_256bits = (ta), .c_i = 1.0, .is_sub = sub, .inv_rssi = (neg_rssi) }</span><br><span style="color: hsl(120, 100%, 40%);">+ { .ber10k = (ber), .ta_offs_256bits = (ta), .c_i = 10, .is_sub = sub, .inv_rssi = (neg_rssi) }</span><br><span> </span><br><span> struct meas_testcase {</span><br><span> const char *name;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bts/+/25442">change 25442</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/+/25442"/><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: I13c0014fdd73f823ae5b1256c35bfa7d97cfa334 </div>
<div style="display:none"> Gerrit-Change-Number: 25442 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </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: 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>