Attention is currently required from: jolly, pespin. fixeria has posted comments on this change by jolly. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41094?usp=email )
Change subject: S1GW: Add test case to test release of e-RABs during handover preperation ...................................................................... Patch Set 2: (5 comments) File s1gw/S1GW_ConnHdlr.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41094/comment/8539cb30_2d35d1c2?usp=email : PS2, Line 1210: in ERabIdxList erabs_release := {}) > I vote for stopping adding more "in" churn :D For `ERabIdxList`, yes, we don't really need `in/out`. However we should keep using them for `ERabList`, to let the caller know if the given list is going to be modified or not. https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41094/comment/d9b9440a_501055eb?usp=email : PS2, Line 1720: in ERabIdxList erabs_forward, > No more "in" churn please. Why are you so obsessed with this? https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41094/comment/af12930b_322f1255?usp=email : PS2, Line 1718: function f_ConnHdlr_handover_cmd(MME_UE_S1AP_ID mme_ue_id, : ENB_UE_S1AP_ID enb_ue_id, : in ERabIdxList erabs_forward, : Nitpick: in the existing API, arguments `mme_ue_id` and `enb_ue_id` usually follow `ERabList`/`ERabIdxList` arguments. For the sake of consistency, I suggest: ```suggestion function f_ConnHdlr_handover_cmd(ERabIdxList erabs_forward, ERabIdxList erabs_release, MME_UE_S1AP_ID mme_ue_id, ENB_UE_S1AP_ID enb_ue_id) ``` File s1gw/S1GW_Tests.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41094/comment/702e923f_fce64b5e?usp=email : PS2, Line 911: for (var integer i := 0; i < lengthof(erabs_forward); i := i + 1) { I see this logic repeated in several places, so I think it's worth having a function in `S1GW_ConnHdlr` (next to `f_ERabList_find_by_pdu()`): ``` function f_ERabList_compose(ERabIdxList idx_list) runs on ConnHdlr return ERabList { var ERabList erabs; for (var integer i := 0; i < lengthof(idx_list); i := i + 1) { var ERabIdx idx := idx_list[i]; erabs[i] := g_pars.erabs[idx]; } return erabs; } ``` This way here you could do: ``` if (lengthof(erabs_forward) > 0) { var ERabList erabs := f_ERabList_compose(erabs_forward); f_ConnHdlr_erab_release_cmd(erabs, g_pars.mme_ue_id, g_pars.idx); f_ConnHdlr_erab_release_rsp(erabs, g_pars.mme_ue_id, g_pars.idx); } ``` https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41094/comment/49ea8775_882e7226?usp=email : PS2, Line 912: tabs vs spaces -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/41094?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Ic70ba19c0a6e349f63aae124607d075b6d19e779 Gerrit-Change-Number: 41094 Gerrit-PatchSet: 2 Gerrit-Owner: jolly <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <[email protected]> Gerrit-CC: pespin <[email protected]> Gerrit-Attention: jolly <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Comment-Date: Sat, 13 Sep 2025 08:21:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <[email protected]>
