<blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">I'm fine with having lower layer APIs, but let's add low layer APIs which re-use data structures we have instead of having to pass parameters to them.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">It looks like we have different opinions on what is low level and what is high level. To me, your new stateful API is high level compared to the old code. Again, I am not against keeping all state parameters in a single record - this is definitely nice for those complex test cases you're mostly working on. Meanwhile, I am mostly writing simple test cases sending just a couple of messages to the IUT, so I don't really want/need any additional abstraction like MS object in my code.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I am not asking you to keep using the old API for complex test cases, while you insist that everybody should use your new API. Let's make everybody equally happy, and keep at least some part of the old code, in particular those three functions. And finally, let's get this change merged.</p><p>Patch set 8:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #d4ffd4;">Code-Review +2</span></p><p><a href="https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370">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/osmo-ttcn3-hacks/+/18370/8/pcu/GPRS_Components.ttcn">File pcu/GPRS_Components.ttcn:</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-ttcn3-hacks/+/18370/8/pcu/GPRS_Components.ttcn@361">Patch Set #8, Line 361:</a> <code style="font-family:monospace,monospace">  f_pcuif_tx_data_ind(data, ms.lqual_cb, fn);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">So you are the one hating extra abstraction layers and you are re-adding one here.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Well, I would remove this function too. I was just afraid that your pending test cases (in a private branch, or on the local host) might be using it. In your original change, f_pcuif_tx_data_ind() was just renamed and moved here - no functional changes. I kept the original function for small and simple test cases, where I don't want to dance with GprsMS just because I need to send a DATA.ind.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370/8/pcu/GPRS_Components.ttcn@561">Patch Set #8, Line 561:</a> <code style="font-family:monospace,monospace">function f_establish_tbf(out GsmRrMessage rr_imm_ass,</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">So now we'll have 2 APIs doing similart stuff (establishing a UL TBF), f_establish_tbf and f_ms_esta […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">We should just rename it to f_pcuif_tx_rach_rx_imm_ass(), because that's exactly what it does. Your high-level f_ms_establish_ul_tbf() gives you a TBF object, this function is low-level because it sends one message and receives another in response with a bit of pattern matching.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370/8/pcu/GPRS_Components.ttcn@591">Patch Set #8, Line 591:</a> <code style="font-family:monospace,monospace">function f_pcuif_tx_data_ind(octetstring data, int16_t lqual_cb := 0, uint32_t fn := 0)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I precisely wanted to get rid of these APIs where you stil labstract stuff but need to keep passing  […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Is it really that problematic to pass one (mandatory) parameter? I would agree with you if there were many, but definitely not in this case. The second one is used only in those test cases which test the link quality adaptation, while 'fn' is never passed explicitly in the current code.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370">change 18370</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-ttcn3-hacks/+/18370"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-ttcn3-hacks </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ib3fee37580f0ea0530a659dec83656799bf57288 </div>
<div style="display:none"> Gerrit-Change-Number: 18370 </div>
<div style="display:none"> Gerrit-PatchSet: 8 </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: 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, 28 May 2020 16:51:52 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Comment-In-Reply-To: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>