Change in osmo-ttcn3-hacks[master]: bts: Add RTP payload testing to performance tests

[pespin]

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

Change subject: bts: Add RTP payload testing to performance tests
......................................................................


Patch Set 1: Code-Review-1

(9 comments)

https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/17285/1/bts/BTS_Tests_perf.ttcn 
File bts/BTS_Tests_perf.ttcn:

https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/17285/1/bts/BTS_Tests_perf.ttcn@53 
PS1, Line 53: function f_uin32t2ip(uint32_t uint32t) return charstring
These functions look like good candidates to be introduced in a more generic already existing files, such as Misc_helpers.ttcn or similar.


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/17285/1/bts/BTS_Tests_perf.ttcn@60 
PS1, Line 60: 	octet_1 := uint32t / 16777216;
Having a param called uint32t is at least very confusing.


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/17285/1/bts/BTS_Tests_perf.ttcn@86 
PS1, Line 86: 	addr := octet_1 * 16777216 + octet_2 * 65536 + octet_3 * 255 + octet_4;
all the octet_* stuff can be dropped, simply use segAddr[0], etc. directly here.


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/17285/1/bts/BTS_Tests_perf.ttcn@113 
PS1, Line 113: 	var integer channels := sizeof(chan_nr);
channels suggest it's an array of channels. What about num_channels? or num_chans.


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/17285/1/bts/BTS_Tests_perf.ttcn@124 
PS1, Line 124: 				 ts_RSL_ChanMode(RSL_CHRT_TCH_H, RSL_CMOD_SP_GSM1 /* AMR*/)),
So I did look at the wrong place when I did this?


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/17285/1/bts/BTS_Tests_perf.ttcn@138 
PS1, Line 138: 		/* Netotiate RTP TX (BTS) port for activated channel */
typo: Negotiate
Tx


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/17285/1/bts/BTS_Tests_perf.ttcn@148 
PS1, Line 148: 		f_rtpem_configure(RTPEM[0], rtp_cfg);
should be RTPEM[i] right?


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/17285/1/bts/BTS_Tests_perf.ttcn@165 
PS1, Line 165: 		f_sleep(1.0);
Can we split the loop into two parts (two loops) so we only f_sleep(1.0) here?
I mean:
foreach(chan) f_rtpem_mode()
f_sleep(1)
foreach(chan) f_rsl_transceive()


https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/17285/1/bts/RTP_CodecPort_CtrlFunct.ttcn 
File bts/RTP_CodecPort_CtrlFunct.ttcn:

https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/17285/1/bts/RTP_CodecPort_CtrlFunct.ttcn@1 
PS1, Line 1: module RTP_CodecPort_CtrlFunct {
just wondering: why do we need all this? Didn't we already have an RTP_CodecPort in some other place? Why not reusing it from lib/ ?



-- 
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/17285
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: I21a610533d5be2e1647cc6a72423e056b39d1217
Gerrit-Change-Number: 17285
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-Comment-Date: Thu, 27 Feb 2020 10:38:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200227/8b27abb0/attachment.htm>