<p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #d4ffd4;">Code-Review +1</span></p><p><a href="https://gerrit.osmocom.org/12287">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/12287/1/src/osmo-bts-trx/loops.c">File src/osmo-bts-trx/loops.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12287/1/src/osmo-bts-trx/loops.c@41">Patch Set #1, Line 41:</a> <code style="font-family:monospace,monospace"> *  \param lchan logical channel for which to compute (and in whihc to store) new power.</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">typo: which<br>"new power value" seems more appropiate.<br>Probably also add: "new power value from previous value"</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12287/1/src/osmo-bts-trx/loops.c@52">Patch Set #1, Line 52:</a> <code style="font-family:monospace,monospace">   /* compute new target MS output power level based on current value subtracted by 'diff/2' */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">s/current/previous would be more easy to catch. current may refer to new or old one.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Perhaps you want to explain somehow why diff/2. I think it's because we use steps of 2dB as granularity?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12287/1/src/osmo-bts-trx/loops.c@64">Patch Set #1, Line 64:</a> <code style="font-family:monospace,monospace">     if (arfcn >= 512 && arfcn <= 885) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Not directly related, but: this code looks like a good candidate to be a separate function and perhaps in libosmogsm? osmo_max_power_ms_by_band() or similar.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/12287">change 12287</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/12287"/><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-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Iafea07eb751ed85d29b214576bb0d31ea919cd72 </div>
<div style="display:none"> Gerrit-Change-Number: 12287 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 13 Dec 2018 10:58:16 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>