<p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/25654">View Change</a></p><p>8 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7//COMMIT_MSG">Commit Message:</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/osmo-bsc/+/25654/7//COMMIT_MSG@10">Patch Set #7, Line 10:</a> <code style="font-family:monospace,monospace">* Imports power_control.c from osmo-bts project.</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Please add a commit reference here.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7//COMMIT_MSG@11">Patch Set #7, Line 11:</a> <code style="font-family:monospace,monospace">* Removes unused C/I code from power_control.</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">from osmo-bts's power_control.c</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/include/osmocom/bsc/gsm_data.h">File include/osmocom/bsc/gsm_data.h:</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/osmo-bsc/+/25654/7/include/osmocom/bsc/gsm_data.h@825">Patch Set #7, Line 825:</a> <code style="font-family:monospace,monospace"> struct gsm_power_ctrl_meas_proc_state rxlev_meas_proc;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Put this inside a struct lchan_power_ctrl_state {} similar to what's done in osmo-bts.<br>This way we keep similar code base and it's easier to re-use it later ifw e want to implement bs-power control.<br>We would ideally move    uint8_t bs_power_db and uint8_t ms_power; from above inside this struct as "uint8_t current" too.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/src/osmo-bsc/power_control.c">File src/osmo-bsc/power_control.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/osmo-bsc/+/25654/7/src/osmo-bsc/power_control.c@170">Patch Set #7, Line 170:</a> <code style="font-family:monospace,monospace"> // int16_t ul_lqual_cb_avg;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">can be dropped</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/src/osmo-bsc/power_control.c@171">Patch Set #7, Line 171:</a> <code style="font-family:monospace,monospace">    uint8_t ms_power_lvl = ms_pwr_ctl_lvl(band, mr->ms_l1.pwr);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I'd say this is wrong. mr->ms_l1.pwr as received from the MS contains already a MS Power Level right? not dBm. Or are we converting it to dBm somewhere?<br>IIUC this line should be:<br>uint8_t ms_power_lvl = mr->ms_l1.pwr;</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/src/osmo-bsc/power_control.c@183">Patch Set #7, Line 183:</a> <code style="font-family:monospace,monospace">   if (bts->dtxu == GSM48_DTX_SHALL_BE_USED)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">IIRC this can be obtained from the MR? it depends on the channel.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/src/osmo-bsc/power_control.c@188">Patch Set #7, Line 188:</a> <code style="font-family:monospace,monospace">        /* Shall we skip current block based on configured interval? */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Let's move this further up before the dtxu block.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-bsc/+/25654/7/src/osmo-bsc/power_control.c@258">Patch Set #7, Line 258:</a> <code style="font-family:monospace,monospace"> lchan->ms_power = new_power_lvl;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Here for sure lchan_update_ms_power_ctrl_level() must be called instead.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bsc/+/25654">change 25654</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-bsc/+/25654"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-bsc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ibc307e758697eb5ca3fb86622f35709d6077db9e </div>
<div style="display:none"> Gerrit-Change-Number: 25654 </div>
<div style="display:none"> Gerrit-PatchSet: 7 </div>
<div style="display:none"> Gerrit-Owner: keith <keith@rhizomatica.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 01 Oct 2021 09:42:07 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>