Attention is currently required from: jolly, laforge, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32647 )
Change subject: GSM_RR_Types: Add support for VBS/VGCS related L3 RR messages ......................................................................
Patch Set 2: Code-Review-1
(8 comments)
Patchset:
PS2: CR-1 due to missing `PRESENCE` attributes.
File library/GSM_RR_Types.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32647/comment/763acc1f_f998f... PS2, Line 590: HEX1 Below in `UplinkAccessIndTV` you're using `uint4_t`. Maybe use it here too or use HEX1 everywhere for the sake of consistency?
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32647/comment/da7a5d1e_72e1c... PS2, Line 597: } with { variant "TAG(cksn, tag = 'B'H)" };
doesn't this belong in line 592?
No, it's correct. This reads as: the optional `cksn` TV field is present if the `tag` field within this field is equal to 'B'H. Alternatively, you could have defined this as follows:
``` type record CipheringKeySeqNrTV { HEX1 tag, CipheringKeySeqNr cksn } with { variant "PRESENCE(tag = 'B'H)" }; ```
I find this a bit more readable and less confusing.
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32647/comment/72a46853_7c59f... PS2, Line 650: } with { variant (len) "LENGTHTO(es,spare,uai_rach,priority)" }; Please add `variant "PRESENCE(iei = '31'O)"`.
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32647/comment/ea1f5abb_ec163... PS2, Line 656: } with { variant "" }; Missing `variant "PRESENCE(iei = '32'O)"`.
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32647/comment/2b058380_15928... PS2, Line 663: } with { variant (len) "LENGTHTO(talker_id)" }; Missing `variant "PRESENCE(iei = '33'O)"`.
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32647/comment/281270bd_31dd4... PS2, Line 670: } with { variant "" }; Missing `variant "PRESENCE(iei = 8)"`.
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32647/comment/da299fa9_74cbf... PS2, Line 690: ws