<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/16070">View Change</a></p><div style="white-space:pre-wrap">Approvals:
osmith: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
Jenkins Builder: Verified
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">power_control.c: Apply latests improvements from loops.c<br><br>Several improvements have been made lately to MS Power Control loop from<br>osmo-bts-trx in loops.c. Let's port these to the common algorithm.<br><br>Related: OS#1851<br>Change-Id: I579967cc8bb69dc76a315c6c9d3a351f5961d92f<br>---<br>M src/common/power_control.c<br>M tests/power/power_test.c<br>2 files changed, 73 insertions(+), 43 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 38a7fb7..106e2a8 100644</span><br><span>--- a/src/common/power_control.c</span><br><span>+++ b/src/common/power_control.c</span><br><span>@@ -22,6 +22,7 @@</span><br><span> #include <stdint.h></span><br><span> #include <unistd.h></span><br><span> #include <errno.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <inttypes.h></span><br><span> </span><br><span> #include <osmo-bts/logging.h></span><br><span> #include <osmo-bts/bts.h></span><br><span>@@ -30,67 +31,87 @@</span><br><span> #include <osmo-bts/bts_model.h></span><br><span> #include <osmo-bts/l1sap.h></span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/*</span><br><span style="color: hsl(0, 100%, 40%);">- * Check if manual power control is needed</span><br><span style="color: hsl(0, 100%, 40%);">- * Check if fixed power was selected</span><br><span style="color: hsl(0, 100%, 40%);">- * Check if the MS is already using our level if not</span><br><span style="color: hsl(0, 100%, 40%);">- * the value is bogus..</span><br><span style="color: hsl(0, 100%, 40%);">- * TODO: Add a timeout.. e.g. if the ms is not capable of reaching</span><br><span style="color: hsl(0, 100%, 40%);">- * the value we have set.</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(120, 100%, 40%);">+ /*! compute the new MS POWER LEVEL 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_power MS Power Level received from Uplink L1 SACCH Header in SACCH block.</span><br><span style="color: hsl(120, 100%, 40%);">+ * \param[in] rxLevel Signal level of the received SACCH block, in dBm.</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span> int lchan_ms_pwr_ctrl(struct gsm_lchan *lchan,</span><br><span> const uint8_t ms_power, const int rxLevel)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- int rx;</span><br><span style="color: hsl(0, 100%, 40%);">- int cur_dBm, new_dBm, new_pwr;</span><br><span style="color: hsl(0, 100%, 40%);">- struct gsm_bts *bts = lchan->ts->trx->bts;</span><br><span style="color: hsl(0, 100%, 40%);">- const enum gsm_band band = bts->band;</span><br><span style="color: hsl(120, 100%, 40%);">+ int diff;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm_bts_trx *trx = lchan->ts->trx;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gsm_bts *bts = trx->bts;</span><br><span style="color: hsl(120, 100%, 40%);">+ enum gsm_band band = bts->band;</span><br><span style="color: hsl(120, 100%, 40%);">+ int8_t new_power; /* TS 05.05 power level */</span><br><span style="color: hsl(120, 100%, 40%);">+ int8_t 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%);">- /* The phone hasn't reached the power level yet */</span><br><span style="color: hsl(120, 100%, 40%);">+ /* The phone hasn't reached the power level yet.</span><br><span style="color: hsl(120, 100%, 40%);">+ TODO: store .last and check if MS is trying to move towards current. */</span><br><span> if (lchan->ms_power_ctrl.current != ms_power)</span><br><span> return 0;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* What is the difference between what we want and received? */</span><br><span style="color: hsl(0, 100%, 40%);">- rx = bts->ul_power_target - rxLevel;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* How many dBs measured power should be increased (+) or decreased (-)</span><br><span style="color: hsl(120, 100%, 40%);">+ to reach expected power. */</span><br><span style="color: hsl(120, 100%, 40%);">+ diff = bts->ul_power_target - rxLevel;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- cur_dBm = ms_pwr_dbm(band, ms_power);</span><br><span style="color: hsl(0, 100%, 40%);">- new_dBm = cur_dBm + rx;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* power levels change in steps of 2 dB, so a smaller diff will end up in no change */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (diff < 2 && diff > -2)</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Clamp negative values and do it depending on the band */</span><br><span style="color: hsl(0, 100%, 40%);">- if (new_dBm < 0)</span><br><span style="color: hsl(0, 100%, 40%);">- new_dBm = 0;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- switch (band) {</span><br><span style="color: hsl(0, 100%, 40%);">- case GSM_BAND_1800:</span><br><span style="color: hsl(0, 100%, 40%);">- /* If MS_TX_PWR_MAX_CCH is set the values 29,</span><br><span style="color: hsl(0, 100%, 40%);">- * 30, 31 are not used. Avoid specifying a dBm</span><br><span style="color: hsl(0, 100%, 40%);">- * that would lead to these power levels. The</span><br><span style="color: hsl(0, 100%, 40%);">- * phone might not be able to reach them. */</span><br><span style="color: hsl(0, 100%, 40%);">- if (new_dBm > 30)</span><br><span style="color: hsl(0, 100%, 40%);">- new_dBm = 30;</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(0, 100%, 40%);">- default:</span><br><span style="color: hsl(0, 100%, 40%);">- break;</span><br><span style="color: hsl(120, 100%, 40%);">+ current_dbm = ms_pwr_dbm(band, lchan->ms_power_ctrl.current);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (current_dbm < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPLCHAN(lchan, DLOOP, LOGL_NOTICE,</span><br><span style="color: hsl(120, 100%, 40%);">+ "Failed to calculate dBm for power ctl level %" PRIu8 " on band %s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->ms_power_ctrl.current, gsm_band_name(band));</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ bsc_max_dbm = ms_pwr_dbm(band, lchan->ms_power_ctrl.max);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (bsc_max_dbm < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPLCHAN(lchan, DLOOP, LOGL_NOTICE,</span><br><span style="color: hsl(120, 100%, 40%);">+ "Failed to calculate dBm for power ctl level %" PRIu8 " on band %s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->ms_power_ctrl.max, gsm_band_name(band));</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- new_pwr = ms_pwr_ctl_lvl(band, new_dBm);</span><br><span style="color: hsl(120, 100%, 40%);">+ new_dbm = current_dbm + diff;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- /* Don't ask for smaller ms power level than the one set</span><br><span style="color: hsl(0, 100%, 40%);">- * by BSC upon RSL CHAN ACT</span><br><span style="color: hsl(0, 100%, 40%);">- */</span><br><span style="color: hsl(0, 100%, 40%);">- if (new_pwr < lchan->ms_power_ctrl.max)</span><br><span style="color: hsl(0, 100%, 40%);">- new_pwr = lchan->ms_power_ctrl.max;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Make sure new_dbm is never negative. ms_pwr_ctl_lvl() can later on</span><br><span style="color: hsl(120, 100%, 40%);">+ cope with any unsigned dbm value, regardless of band minimal value. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (new_dbm < 0)</span><br><span style="color: hsl(120, 100%, 40%);">+ new_dbm = 0;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (lchan->ms_power_ctrl.current != new_pwr) {</span><br><span style="color: hsl(0, 100%, 40%);">- lchan->ms_power_ctrl.current = new_pwr;</span><br><span style="color: hsl(0, 100%, 40%);">- bts_model_adjst_ms_pwr(lchan);</span><br><span style="color: hsl(0, 100%, 40%);">- return 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Don't ask for smaller ms power level than the one set by BSC upon RSL CHAN ACT */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (new_dbm > bsc_max_dbm)</span><br><span style="color: hsl(120, 100%, 40%);">+ new_dbm = bsc_max_dbm;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ new_power = ms_pwr_ctl_lvl(band, new_dbm);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (new_power < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPLCHAN(lchan, DLOOP, LOGL_NOTICE,</span><br><span style="color: hsl(120, 100%, 40%);">+ "Failed to retrieve power level for %" PRId8 " dBm on band %d\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ new_dbm, band);</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (lchan->ms_power_ctrl.current == new_power) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPLCHAN(lchan, DLOOP, LOGL_INFO, "Keeping MS new_power at control level %d (%d dBm)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ new_power, ms_pwr_dbm(band, new_power));</span><br><span style="color: hsl(120, 100%, 40%);">+ return 0;</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, "%s MS new_power from control level %d (%d dBm) to %d (%d dBm)\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ (diff > 0) ? "Raising" : "Lowering",</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->ms_power_ctrl.current, ms_pwr_dbm(band, lchan->ms_power_ctrl.current),</span><br><span style="color: hsl(120, 100%, 40%);">+ new_power, ms_pwr_dbm(band, new_power));</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* store the resulting new MS power level in the lchan */</span><br><span style="color: hsl(120, 100%, 40%);">+ lchan->ms_power_ctrl.current = new_power;</span><br><span style="color: hsl(120, 100%, 40%);">+ bts_model_adjst_ms_pwr(lchan);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ return 1;</span><br><span> }</span><br><span>diff --git a/tests/power/power_test.c b/tests/power/power_test.c</span><br><span>index dbae8fa..30fbb64 100644</span><br><span>--- a/tests/power/power_test.c</span><br><span>+++ b/tests/power/power_test.c</span><br><span>@@ -17,7 +17,11 @@</span><br><span> * along with this program. If not, see <http://www.gnu.org/licenses/>.</span><br><span> */</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/core/talloc.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmocom/core/application.h></span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #include <osmo-bts/bts.h></span><br><span style="color: hsl(120, 100%, 40%);">+#include <osmo-bts/logging.h></span><br><span> #include <osmo-bts/l1sap.h></span><br><span> #include <osmo-bts/power_control.h></span><br><span> </span><br><span>@@ -80,6 +84,11 @@</span><br><span> {</span><br><span> printf("Testing power loop...\n");</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ tall_bts_ctx = talloc_named_const(NULL, 1, "OsmoBTS context");</span><br><span style="color: hsl(120, 100%, 40%);">+ msgb_talloc_ctx_init(tall_bts_ctx, 0);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ osmo_init_logging2(tall_bts_ctx, &bts_log_info);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> test_power_loop();</span><br><span> </span><br><span> printf("Power loop test OK\n");</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bts/+/16070">change 16070</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/+/16070"/><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: I579967cc8bb69dc76a315c6c9d3a351f5961d92f </div>
<div style="display:none"> Gerrit-Change-Number: 16070 </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>