fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370 )
Change subject: pcu: Refactor GPRS infrastructure to keep state and simplify tests ...................................................................... Patch Set 5: (8 comments) https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370/5/pcu/GPRS_Components.ttcn File pcu/GPRS_Components.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370/5/pcu/GPRS_Components.ttcn@78 PS5, Line 78: uint8_t > I iitially used a boolean but then had issues passing it to some lower layer > template in RAW_PCUIF o […] Ah, yeah, TITAN does not support implicit boolean-integer conversion. Neither there are functions for that like int2bool() and bool2int(). We should just change the field type in PCUFI_Types to boolean with FIELDLENGTH(8). I am fine with doing it later, in a separate change. On the other hand, using bitstring would simplify the things - it's easy to convert it to what PCUIF needs: type bitstring GsmRa length(8..11) with { variant "" }; const GsmRa chan_req_def := '01111000'B; // Just before sending RACH.ind var uint16_t ra := bit2int(chan_req_def); var uint8_t is_11bit; if (lengthof(chan_req_def) == 11) { is_11bit := 1 } else { is_11bit := 0 } https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370/5/pcu/GPRS_Components.ttcn@79 PS5, Line 79: PCUIF_BurstType > Ack, will simply apply it when needed. Thanks! https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370/5/pcu/GPRS_Components.ttcn@249 PS5, Line 249: ms.burst_type := BURST_TYPE_0; > I will simply do this check before sending RACH.ind to send the correct > BURST_TYPE. Just let the caller some freedom, there is also BURST_TYPE_2 (and even more in the specs). https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370/5/pcu/GPRS_Components.ttcn@327 PS5, Line 327: uint16_t > will check the template. Actually, template wants type integer TimingAdvance (0..219). https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370/5/pcu/GPRS_Components.ttcn@362 PS5, Line 362: uint32_t fn := 0 > no, fn=0 is fine here, iirc it's a special value meaning "next one". Ah, right. Nevermind. https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370/5/pcu/PCU_Tests.ttcn File pcu/PCU_Tests.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370/5/pcu/PCU_Tests.ttcn@269 PS5, Line 269: f_ms_establish_ul_tbf > it was already calling f_establish_tbf, there's no much new overhead. Well, f_establish_tbf() has a bit inaccurate name. Under the hood there is no TBF establishment, it simply sends RACH.ind, receives DATA.req on AGCH, matches it against tr_IMM_TBF_ASS and returns to the caller. This is exactly what's needed here. So there is overhead. Please keep low level API for small and simple test cases. https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370/5/pcu/PCU_Tests.ttcn@584 PS5, Line 584: if (match(dl_block, tr_RLCMAC_DUMMY_CTRL())) { : continue; : } : if (not match(dl_block, tr_RLCMAC_UL_ACK_NACK_GPRS(ul_tfi := ?)) and : not match(dl_block, tr_RLCMAC_UL_ACK_NACK_EGPRS(ul_tfi := ?))) { : setverdict(fail, "Failed to match Packet Uplink ACK / NACK:", dl_block); : f_shutdown(__BFILE__, __LINE__); : } > Not really, I'm fixing the test since it was not behaving correctly as an MS > would. […] I would prefer this to be done in a separate change, but if it's really needed here - let's keep it. https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370/5/pcu/PCU_Tests.ttcn@609 PS5, Line 609: f_shutdown > I think this is fine. You want me to remove it? ok, I will remove it. The proble is that I don't see how it's related to this patch. Feel free to do in a separate change with some description of its purpose. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/18370 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Ib3fee37580f0ea0530a659dec83656799bf57288 Gerrit-Change-Number: 18370 Gerrit-PatchSet: 5 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: pespin <[email protected]> Gerrit-Comment-Date: Fri, 22 May 2020 05:18:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <[email protected]> Comment-In-Reply-To: fixeria <[email protected]> Gerrit-MessageType: comment
