<p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/15918">View Change</a></p><p>5 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/15918/7/src/common/l1sap.c">File src/common/l1sap.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-bts/+/15918/7/src/common/l1sap.c@735">Patch Set #7, Line 735:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">if (!gsm_bts_has_feature(trx->bts, BTS_FEAT_MEAS_PAYLOAD_COMB))<br>                    process_l1sap_meas_data(trx, l1sap, PRIM_MPH_INFO);<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">we sould probably print an error message here (once? rate-limited)?  Or even have an OSMO_ASSERT()?  […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/common/scheduler.c">File src/common/scheduler.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-bts/+/15918/1/src/common/scheduler.c@760">Patch Set #1, Line 760:</a> <code style="font-family:monospace,monospace">l1sap->u.tch.</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I believe I explained this elsewhere here in gerrit or in redmine. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">The is_sub flag is, as far as I know only set by the higher layers. From what I can see the higher layers are not setting it, at least not in l1sap.c. However when we the data ind and the measurement report are one indication the decision would be easy to make in l1sap.c</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/15918/1/src/osmo-bts-trx/scheduler_trx.c">File src/osmo-bts-trx/scheduler_trx.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-bts/+/15918/1/src/osmo-bts-trx/scheduler_trx.c@a205">Patch Set #1, Line 205:</a> <code style="font-family:monospace,monospace">/* FIXME: use actual values for BER etc */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">the best would be if those values simply wouldn't count. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">We could use rssi_sum rssi_num from the chan_state struct to do that. I see it in other places too so I use this method now. However, I am still not sure if this is right. The value is now averaged so when the signal fades it should be realistic, but what if there is a sudden signal drop then the lost SACCH frames we compensate here would reflect better signal conditions as they actually are. All in all it might not matter much as the measurement results are averaged anyway in the higher layers.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/15918/8/src/osmo-bts-trx/scheduler_trx.c">File src/osmo-bts-trx/scheduler_trx.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-bts/+/15918/8/src/osmo-bts-trx/scheduler_trx.c@198">Patch Set #8, Line 198:</a> <code style="font-family:monospace,monospace">*rssi_sum / *rssi_num</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This is a TX burst handler, so there can be no RSSI measurements. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Yes, in TX the division by zero case is very likely. However, I see the division with rssi and toa in many other places. Do you think it makes sense to protect those as well?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/15918/9/src/osmo-bts-trx/scheduler_trx.c">File src/osmo-bts-trx/scheduler_trx.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-bts/+/15918/9/src/osmo-bts-trx/scheduler_trx.c@401">Patch Set #9, Line 401:</a> <code style="font-family:monospace,monospace">toa_num</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Same here.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Its probably the wrong approach to calculate those values from the chan_state. Instead I suggest to remember the last good toa256 and the last good rssi and use those instead. However I still do not understand why there aren't any toa256_sum values in the chan state. During the channel establishment phase, before the MS transmitting I see that there can not be any values but what happens when a TCH gets lost while the channel is up?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bts/+/15918">change 15918</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/+/15918"/><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: I710d0b7cf193afa8515807836ee69b8b7db84a84 </div>
<div style="display:none"> Gerrit-Change-Number: 15918 </div>
<div style="display:none"> Gerrit-PatchSet: 9 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Thu, 09 Jan 2020 14:54:41 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Comment-In-Reply-To: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Comment-In-Reply-To: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>