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_2d35d... : 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_50105... : 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_322f1... : 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_fce64... : 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_882e7... : PS2, Line 912: tabs vs spaces