Attention is currently required from: jolly, laforge.

fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33979 )

Change subject: ASCI: Add tests for voice group/broadcast calls at MSC
......................................................................


Patch Set 1:

(3 comments)

File msc/MSC_Tests_ASCI.ttcn:

https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33979/comment/b85e6ea8_377e5124
PS1, Line 50: type integer ASCI_TEST;
            : const ASCI_TEST ASCI_TEST_NO_CALLREF      := 1;
            : const ASCI_TEST ASCI_TEST_SETUP_REFUSE    := 2;
            : const ASCI_TEST ASCI_TEST_ASSIGN_FAIL     := 3;
            : const ASCI_TEST ASCI_TEST_COMPLETE_VGCS   := 4;
            : const ASCI_TEST ASCI_TEST_COMPLETE_VBS    := 5;
> I think this could/should go into an enum, not an integer?
Looks like it's done this way because the `BSC_ConnHdlr_Coord_PT` can only 
deliver charstrings and integers (added by preceding patch). I propose to add a 
new port definition, dedicated/specific to this module, which would allow 
sending/receiving enumerate types (also specific to this module).


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33979/comment/85622915_b2e153ab
PS1, Line 58: type component asci_CT extends MTC_CT {
Just a friendly note (not critical): it's better to mark module-local types, 
altsteps, and functions as such (using the `private` keyword). This improves 
the compilation time, especially if some other module imports something from 
this one. And generally good for readability.


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33979/comment/b4235e1b_097185a8
PS1, Line 65: const charstring COORD_SETUP := "SETUP";
> you don't really need to do this with charstring. […]
full ack



--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/33979
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: I4bbe739ea55ecf9f7ebf9ee413df69f29aa642f8
Gerrit-Change-Number: 33979
Gerrit-PatchSet: 1
Gerrit-Owner: jolly <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-CC: fixeria <[email protected]>
Gerrit-Attention: jolly <[email protected]>
Gerrit-Attention: laforge <[email protected]>
Gerrit-Comment-Date: Thu, 27 Jul 2023 21:44:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <[email protected]>
Gerrit-MessageType: comment

Reply via email to