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

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">power_control.c: Clarify loop algo vars and use correct ones during log<br><br>Rename some variables so that:<br>* Variables containing power control levels end up with "_lvl".<br>* Variables containing power levels end up with _dbm.<br>* Move old current_dbm var to be ms_dbm, to match its power control<br>level counterpart ms_power_lvl, and add current_dbm to match its<br>counterpart ns_power_ctrl.current.<br><br>Now that variables are more clear, it also becomes clear that old "diff ><br>0" condition, apart from difficult, was currently wrong, since in order<br>to print the raise/low verb we want to compare between old and new<br>values, not between received and new values. Let's fix that in this same<br>commit.<br><br>Change-Id: I4e279a6b93fbcc5da25bf8c9213310939fd493ce<br>---<br>M src/common/power_control.c<br>1 file changed, 18 insertions(+), 18 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/common/power_control.c b/src/common/power_control.c</span><br><span>index f6138d6..e9215fc 100644</span><br><span>--- a/src/common/power_control.c</span><br><span>+++ b/src/common/power_control.c</span><br><span>@@ -38,29 +38,29 @@</span><br><span> </span><br><span>  /*! compute the new MS POWER LEVEL communicated to the MS and store it in lchan.</span><br><span>   *  \param lchan logical channel for which to compute (and in which to store) new power value.</span><br><span style="color: hsl(0, 100%, 40%);">-  *  \param[in] ms_power MS Power Level received from Uplink L1 SACCH Header in SACCH block.</span><br><span style="color: hsl(120, 100%, 40%);">+  *  \param[in] ms_power_lvl MS Power Level received from Uplink L1 SACCH Header in SACCH block.</span><br><span>   *  \param[in] rxLevel Signal level of the received SACCH block, in dBm.</span><br><span>   */</span><br><span> int lchan_ms_pwr_ctrl(struct gsm_lchan *lchan,</span><br><span style="color: hsl(0, 100%, 40%);">-               const uint8_t ms_power, const int rxLevel)</span><br><span style="color: hsl(120, 100%, 40%);">+                    const uint8_t ms_power_lvl, const int rxLevel)</span><br><span> {</span><br><span>    int diff;</span><br><span>    struct gsm_bts_trx *trx = lchan->ts->trx;</span><br><span>      struct gsm_bts *bts = trx->bts;</span><br><span>   enum gsm_band band = bts->band;</span><br><span style="color: hsl(0, 100%, 40%);">-      int8_t new_power; /* TS 05.05 power level */</span><br><span style="color: hsl(0, 100%, 40%);">-    int8_t new_dbm, current_dbm, bsc_max_dbm;</span><br><span style="color: hsl(120, 100%, 40%);">+     int8_t new_power_lvl; /* TS 05.05 power level */</span><br><span style="color: hsl(120, 100%, 40%);">+      int8_t ms_dbm, new_dbm, current_dbm, bsc_max_dbm;</span><br><span> </span><br><span>        if (!trx_ms_pwr_ctrl_is_osmo(lchan->ts->trx))</span><br><span>          return 0;</span><br><span>    if (lchan->ms_power_ctrl.fixed)</span><br><span>           return 0;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   current_dbm = ms_pwr_dbm(band, ms_power);</span><br><span style="color: hsl(0, 100%, 40%);">-       if (current_dbm < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+     ms_dbm = ms_pwr_dbm(band, ms_power_lvl);</span><br><span style="color: hsl(120, 100%, 40%);">+      if (ms_dbm < 0) {</span><br><span>                 LOGPLCHAN(lchan, DLOOP, LOGL_NOTICE,</span><br><span>                           "Failed to calculate dBm for power ctl level %" PRIu8 " on band %s\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                      ms_power, gsm_band_name(band));</span><br><span style="color: hsl(120, 100%, 40%);">+                       ms_power_lvl, gsm_band_name(band));</span><br><span>                return 0;</span><br><span>    }</span><br><span>    bsc_max_dbm = ms_pwr_dbm(band, lchan->ms_power_ctrl.max);</span><br><span>@@ -83,7 +83,7 @@</span><br><span>     else if (diff < -MS_LOWER_MAX_DB)</span><br><span>                 diff = -MS_LOWER_MAX_DB;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-    new_dbm = current_dbm + diff;</span><br><span style="color: hsl(120, 100%, 40%);">+ new_dbm = ms_dbm + diff;</span><br><span> </span><br><span>         /* Make sure new_dbm is never negative. ms_pwr_ctl_lvl() can later on</span><br><span>           cope with any unsigned dbm value, regardless of band minimal value. */</span><br><span>@@ -94,33 +94,33 @@</span><br><span>      if (new_dbm > bsc_max_dbm)</span><br><span>                new_dbm = bsc_max_dbm;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-      new_power = ms_pwr_ctl_lvl(band, new_dbm);</span><br><span style="color: hsl(0, 100%, 40%);">-      if (new_power < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+       new_power_lvl = ms_pwr_ctl_lvl(band, new_dbm);</span><br><span style="color: hsl(120, 100%, 40%);">+        if (new_power_lvl < 0) {</span><br><span>          LOGPLCHAN(lchan, DLOOP, LOGL_NOTICE,</span><br><span>                           "Failed to retrieve power level for %" PRId8 " dBm on band %d\n",</span><br><span>                        new_dbm, band);</span><br><span>            return 0;</span><br><span>    }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-   if (lchan->ms_power_ctrl.current == new_power) {</span><br><span style="color: hsl(120, 100%, 40%);">+   if (lchan->ms_power_ctrl.current == new_power_lvl) {</span><br><span>              LOGPLCHAN(lchan, DLOOP, LOGL_INFO, "Keeping MS power at control level %d, %d dBm "</span><br><span>                           "(rx-ms-pwr-lvl %" PRIu8 ", max-ms-pwr-lvl %" PRIu8 ", rx-current %d dBm, rx-target %d dBm)\n",</span><br><span style="color: hsl(0, 100%, 40%);">-                   new_power, ms_pwr_dbm(band, new_power),</span><br><span style="color: hsl(0, 100%, 40%);">-                         ms_power, lchan->ms_power_ctrl.max,</span><br><span style="color: hsl(120, 100%, 40%);">+                        new_power_lvl, new_dbm,</span><br><span style="color: hsl(120, 100%, 40%);">+                       ms_power_lvl, lchan->ms_power_ctrl.max,</span><br><span>                           rxLevel, bts->ul_power_target);</span><br><span>                 return 0;</span><br><span>    }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ current_dbm = ms_pwr_dbm(band, lchan->ms_power_ctrl.current);</span><br><span>     LOGPLCHAN(lchan, DLOOP, LOGL_INFO, "%s MS power from control level %d (%d dBm) to %d, %d dBm "</span><br><span>               "(rx-ms-pwr-lvl %" PRIu8 ", max-ms-pwr-lvl %" PRIu8 ", rx-current %d dBm, rx-target %d dBm)\n",</span><br><span style="color: hsl(0, 100%, 40%);">-           (diff > 0) ? "Raising" : "Lowering",</span><br><span style="color: hsl(0, 100%, 40%);">-             lchan->ms_power_ctrl.current, ms_pwr_dbm(band, lchan->ms_power_ctrl.current),</span><br><span style="color: hsl(0, 100%, 40%);">-             new_power, ms_pwr_dbm(band, new_power),</span><br><span style="color: hsl(0, 100%, 40%);">-                 ms_power, lchan->ms_power_ctrl.max,</span><br><span style="color: hsl(120, 100%, 40%);">+                (new_dbm > current_dbm) ? "Raising" : "Lowering",</span><br><span style="color: hsl(120, 100%, 40%);">+              lchan->ms_power_ctrl.current, current_dbm, new_power_lvl, new_dbm,</span><br><span style="color: hsl(120, 100%, 40%);">+                 ms_power_lvl, lchan->ms_power_ctrl.max,</span><br><span>                   rxLevel, bts->ul_power_target);</span><br><span> </span><br><span>     /* store the resulting new MS power level in the lchan */</span><br><span style="color: hsl(0, 100%, 40%);">-       lchan->ms_power_ctrl.current = new_power;</span><br><span style="color: hsl(120, 100%, 40%);">+  lchan->ms_power_ctrl.current = new_power_lvl;</span><br><span>     bts_model_adjst_ms_pwr(lchan);</span><br><span> </span><br><span>   return 1;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bts/+/16440">change 16440</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/+/16440"/><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: I4e279a6b93fbcc5da25bf8c9213310939fd493ce </div>
<div style="display:none"> Gerrit-Change-Number: 16440 </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>