<p>dexter has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/17929">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">measurement: expect at least 1 SUB frame for AMR<br><br>The amount of SUB frames that may occur in AMR is not fixed, nor can it<br>be determined by some formular or lookup table. The reason for this is<br>that the DTX period may negotiated dynamicly. This means that the lower<br>layers must make the decision if an AMR sub frame is a SUB frame early<br>and tag the repective measurement / frame they hand up to the upper<br>layers accordingly. However, regardles of the occurrence of DTX periods<br>the amount of SUB frames in AMR must be always 1 or more because SACCH<br>frames always count as SUB frames as well. Lets make sure that this is<br>respected in the debug log as well as in the logic that tries to<br>substitue missing SUB frame measuremnets.<br><br>Change-Id: I1fd91b576ff7274caa6d4356bcd7a4fa4311219d<br>Related: OS#4465<br>---<br>M src/common/measurement.c<br>1 file changed, 49 insertions(+), 19 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/29/17929/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/common/measurement.c b/src/common/measurement.c</span><br><span>index c5cbbb0..4ea820a 100644</span><br><span>--- a/src/common/measurement.c</span><br><span>+++ b/src/common/measurement.c</span><br><span>@@ -569,10 +569,11 @@</span><br><span> if (lchan->tch_mode != GSM48_CMODE_SPEECH_AMR)</span><br><span> num_meas_sub_expect = lchan_meas_sub_num_expected(lchan);</span><br><span> else {</span><br><span style="color: hsl(0, 100%, 40%);">- /* FIXME: the amount of SUB Measurements is a dynamic parameter</span><br><span style="color: hsl(0, 100%, 40%);">- * in AMR and can not be determined by using a lookup table.</span><br><span style="color: hsl(0, 100%, 40%);">- * See also: OS#2978 */</span><br><span style="color: hsl(0, 100%, 40%);">- num_meas_sub_expect = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* When AMR is used, we expect at least one SUB frame, since</span><br><span style="color: hsl(120, 100%, 40%);">+ * the SACCH will always be SUB frame. There may occur more</span><br><span style="color: hsl(120, 100%, 40%);">+ * SUB frames but since DTX periods in AMR are dynamic, we</span><br><span style="color: hsl(120, 100%, 40%);">+ * can not know how much exactly. */</span><br><span style="color: hsl(120, 100%, 40%);">+ num_meas_sub_expect = 1;</span><br><span> }</span><br><span> </span><br><span> if (lchan->meas.num_ul_meas > num_ul_meas_expect)</span><br><span>@@ -616,9 +617,17 @@</span><br><span> num_ul_meas_actual++;</span><br><span> } else {</span><br><span> m = &measurement_dummy;</span><br><span style="color: hsl(0, 100%, 40%);">- if (num_ul_meas_expect - i <= num_meas_sub_expect - num_meas_sub) {</span><br><span style="color: hsl(0, 100%, 40%);">- num_meas_sub_subst++;</span><br><span style="color: hsl(0, 100%, 40%);">- is_sub = true;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* For AMR the amount of SUB frames is defined by the</span><br><span style="color: hsl(120, 100%, 40%);">+ * the occurrence of DTX periods, which are dynamically</span><br><span style="color: hsl(120, 100%, 40%);">+ * negotiated in AMD, so we can not know if and how many</span><br><span style="color: hsl(120, 100%, 40%);">+ * SUB frames are missing. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan->tch_mode != GSM48_CMODE_SPEECH_AMR) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (num_ul_meas_expect - i <=</span><br><span style="color: hsl(120, 100%, 40%);">+ num_meas_sub_expect - num_meas_sub) {</span><br><span style="color: hsl(120, 100%, 40%);">+ num_meas_sub_subst++;</span><br><span style="color: hsl(120, 100%, 40%);">+ is_sub = true;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> num_ul_meas_subst++;</span><br><span>@@ -631,21 +640,42 @@</span><br><span> }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DMEAS, LOGL_DEBUG, "%s received UL measurements contain %u SUB measurements, expected %u\n",</span><br><span style="color: hsl(0, 100%, 40%);">- gsm_lchan_name(lchan), num_meas_sub_actual, num_meas_sub_expect);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan->tch_mode != GSM48_CMODE_SPEECH_AMR) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DMEAS, LOGL_DEBUG,</span><br><span style="color: hsl(120, 100%, 40%);">+ "%s received UL measurements contain %u SUB measurements, expected %u\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ gsm_lchan_name(lchan), num_meas_sub_actual,</span><br><span style="color: hsl(120, 100%, 40%);">+ num_meas_sub_expect);</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DMEAS, LOGL_DEBUG,</span><br><span style="color: hsl(120, 100%, 40%);">+ "%s received UL measurements contain %u SUB measurements, expected at least %u\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ gsm_lchan_name(lchan), num_meas_sub_actual,</span><br><span style="color: hsl(120, 100%, 40%);">+ num_meas_sub_expect);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> LOGP(DMEAS, LOGL_DEBUG, "%s replaced %u measurements with dummy values, from which %u were SUB measurements\n",</span><br><span> gsm_lchan_name(lchan), num_ul_meas_subst, num_meas_sub_subst);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (num_meas_sub != num_meas_sub_expect) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGP(DMEAS, LOGL_ERROR, "%s Incorrect number of SUB measurements detected! (%u vs exp %u)\n",</span><br><span style="color: hsl(0, 100%, 40%);">- gsm_lchan_name(lchan), num_meas_sub, num_meas_sub_expect);</span><br><span style="color: hsl(0, 100%, 40%);">- /* Normally the logic above should make sure that there is</span><br><span style="color: hsl(0, 100%, 40%);">- * always the exact amount of SUB measurements taken into</span><br><span style="color: hsl(0, 100%, 40%);">- * account. If not then the logic that decides tags the received</span><br><span style="color: hsl(0, 100%, 40%);">- * measurements as is_sub works incorrectly. Since the logic</span><br><span style="color: hsl(0, 100%, 40%);">- * above only adds missing measurements during the calculation</span><br><span style="color: hsl(0, 100%, 40%);">- * it can not remove excess SUB measurements or add missing SUB</span><br><span style="color: hsl(0, 100%, 40%);">- * measurements when there is no more room in the interval. */</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Normally the logic above should make sure that there is</span><br><span style="color: hsl(120, 100%, 40%);">+ * always the exact amount of SUB measurements taken into</span><br><span style="color: hsl(120, 100%, 40%);">+ * account. If not then the logic that decides tags the received</span><br><span style="color: hsl(120, 100%, 40%);">+ * measurements as is_sub works incorrectly. Since the logic</span><br><span style="color: hsl(120, 100%, 40%);">+ * above only adds missing measurements during the calculation</span><br><span style="color: hsl(120, 100%, 40%);">+ * it can not remove excess SUB measurements or add missing SUB</span><br><span style="color: hsl(120, 100%, 40%);">+ * measurements when there is no more room in the interval. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan->tch_mode != GSM48_CMODE_SPEECH_AMR) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (num_meas_sub != num_meas_sub_expect) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DMEAS, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ "%s Incorrect number of SUB measurements detected! (%u vs exp %u)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ gsm_lchan_name(lchan), num_meas_sub,</span><br><span style="color: hsl(120, 100%, 40%);">+ num_meas_sub_expect);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (num_meas_sub < num_meas_sub_expect) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGP(DMEAS, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ "%s Incorrect number of SUB measurements detected! (%u vs exp >=%u)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ gsm_lchan_name(lchan), num_meas_sub,</span><br><span style="color: hsl(120, 100%, 40%);">+ num_meas_sub_expect);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> /* Measurement computation step 2: divide */</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bts/+/17929">change 17929</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/+/17929"/><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: I1fd91b576ff7274caa6d4356bcd7a4fa4311219d </div>
<div style="display:none"> Gerrit-Change-Number: 17929 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>