<p style="white-space: pre-wrap; word-wrap: break-word;">This change is ready for review.</p><p><a href="https://gerrit.osmocom.org/c/osmo-bts/+/15757">View Change</a></p><p>4 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/+/15757/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/+/15757/1/src/osmo-bts-trx/scheduler_trx.c@1230">Patch Set #1, Line 1230:</a> <code style="font-family:monospace,monospace">FN_REMAP_FACCH_F</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">You can't be sure at this point as you don't know whether it's FACCH or speech.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I noticed the problem. I have now moved the l1if_process_meas_res function call to the positions where the payload indication is generated. This is moving us also closer to the next task where we will merge the measurement data into the payload indication. I am not sure if it is correct now, but I am not sure what happens if the the frame gets stolen by FACCH, maybe we are calling l1if_process_meas_res now two times, once for the FAACH and once for the stolen frame that isn't actually there?</p><p style="white-space: pre-wrap; word-wrap: break-word;">I think that if this gets to complicated to resolve, we should consider to do this in a separate patch.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-bts/+/15757/1/src/osmo-bts-trx/scheduler_trx.c@1318">Patch Set #1, Line 1318:</a> <code style="font-family:monospace,monospace">FN_REMAP_FACCH_F</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">That's actually not FACCH.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-bts/+/15757/1/src/osmo-bts-trx/scheduler_trx.c@1442">Patch Set #1, Line 1442:</a> <code style="font-family:monospace,monospace">FN_REMAP_TCH_H0</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Same here, what if the received frame is FACCH/H?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">(see above)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-bts/+/15757/1/src/osmo-bts-trx/scheduler_trx.c@1473">Patch Set #1, Line 1473:</a> <code style="font-family:monospace,monospace">FN_REMAP_FACCH_H0</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Can be simplified: FN_REMAP_FACCH_H0 + lchan->nr.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't think that it is a good idea to add values to an enum, we should keep it like it is to be more explicit.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-bts/+/15757">change 15757</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/+/15757"/><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: I37601ddd85e4287dd9e41ad4a8cb7d314de1a83d </div>
<div style="display:none"> Gerrit-Change-Number: 15757 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </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-CC: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 22 Oct 2019 13:56:53 +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: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>