<p><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302">View Change</a></p><p>48 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2//COMMIT_MSG">Commit Message:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2//COMMIT_MSG@32">Patch Set #2, Line 32:</a> <code style="font-family:monospace,monospace">Infrastructure explaination:</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">You probably want to add all this documentarion into the UserManual too (osmo-gsm-tester.git/doc/), feel free to add a new section or whatever if you want.<br>fyi here's the nightly output of the documentation: http://ftp.osmocom.org/docs/latest/osmo-gsm-tester-manual.pdf</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2//COMMIT_MSG@55">Patch Set #2, Line 55:</a> <code style="font-family:monospace,monospace">dropbearmulti dropbear -F -E -p 130 -R -T /data/local/tmp/authorized_keys  -U 0 -G 0 -N root -A</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">what's this dropbearmulti?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2//COMMIT_MSG@87">Patch Set #2, Line 87:</a> <code style="font-family:monospace,monospace">  ue_serial: '8d9d79a9'</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">what's this for? can you check if you can reuse any of the existing fields? like "path" ?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2//COMMIT_MSG@88">Patch Set #2, Line 88:</a> <code style="font-family:monospace,monospace">  apn_name: 'srsapn'</code></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">looks like you want to have a node here:<br>apn:<br> name: 'bla'<br> mcc: '901'<br> mnc: '70'<br> sel: True</pre></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2//COMMIT_MSG@100">Patch Set #2, Line 100:</a> <code style="font-family:monospace,monospace">    ue_ssh_port: 130</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">ssh_port</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/bitrate_monitor.py">File src/osmo_gsm_tester/obj/bitrate_monitor.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/bitrate_monitor.py@3">Patch Set #2, Line 3:</a> <code style="font-family:monospace,monospace"># Copyright (C) 2020 by sysmocom - s.f.m.c. GmbH</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">That's not from sysmocom, probably from SRS ;)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/bitrate_monitor.py@44">Patch Set #2, Line 44:</a> <code style="font-family:monospace,monospace">    def run_androidue_cmd(self, name, popen_args, sync=True):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Better don't do the launch() here and simply return the proc object and let the caller decide what to do with it.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/bitrate_monitor.py@58">Patch Set #2, Line 58:</a> <code style="font-family:monospace,monospace">            self.rem_host.remote_user = 'root'</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">You should ideally use sudo with sudoers file limit use of certain commands, that what we usually did so far, but I'm not going to block this.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/bitrate_monitor.py@72">Patch Set #2, Line 72:</a> <code style="font-family:monospace,monospace">        popen_args_rx_mon = ['while', 'true;', 'do',</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think it'd make more sense to use something like: ['sh', '-c', 'while true; do echo; cat /sys/class/net/ + self.ue_data_intf + '/statistics/rx_bytes']</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/bitrate_monitor.py@79">Patch Set #2, Line 79:</a> <code style="font-family:monospace,monospace">        self.testenv.remember_to_stop(self.rx_monitor_proc)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">As I shared before, return run_androidue_cmd without calling launch() on it, then AFTER calling remember_to_stop() you call proc.launch().<br>Imagine someone sent a SIGINT to OGT in between, you end up with dangling processes.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/bitrate_monitor.py@88">Patch Set #2, Line 88:</a> <code style="font-family:monospace,monospace">        brate_rx_raw = self.rx_monitor_proc.get_stdout().split('\n')</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">if you run the process locally you may find out the output you are looking for is in stderr, be careful here (when we launch stuff on ssh due to tty (-t -t) related topics everything ends up in stdout locally)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/iperf3.py">File src/osmo_gsm_tester/obj/iperf3.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/iperf3.py@235">Patch Set #2, Line 235:</a> <code style="font-family:monospace,monospace">    def prepare_test_proc(self, dir=None, netns=None, time_sec=None, proto=None, bitrate=0, tos=None, ue=None):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Drop ue param, add new param "adb_serial=None" and only pass that (see coments below, you don't need the ue at all).</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/iperf3.py@267">Patch Set #2, Line 267:</a> <code style="font-family:monospace,monospace"></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">drop ws line change</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/iperf3.py@305">Patch Set #2, Line 305:</a> <code style="font-family:monospace,monospace">            self.process = self.rem_host.RemoteProcess(self.name(), popen_args, remote_port=remote_port, env={})</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">remote_port should already be part of rem_host (see previous patch comments) so this doesn't need to change.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/iperf3.py@320">Patch Set #2, Line 320:</a> <code style="font-family:monospace,monospace">        if netns:</code></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">if netns:<br> ...<br>elif adb_serial:<br> ...<br>else:<br> ... process.Process</pre><p style="white-space: pre-wrap; word-wrap: break-word;">or do you use both netns and adb at the same time?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py">File src/osmo_gsm_tester/obj/ms_android.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@3">Patch Set #2, Line 3:</a> <code style="font-family:monospace,monospace"># Copyright (C) 2020 by sysmocom - s.f.m.c. GmbH</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">wrong copyright</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@43">Patch Set #2, Line 43:</a> <code style="font-family:monospace,monospace">        'ue_serial': schema.STR,</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">you can use "path" here instead, from base class.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@105">Patch Set #2, Line 105:</a> <code style="font-family:monospace,monospace">    def _restart_adb_inst(self, as_root=False):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Why do you need to run this everytime? Sounds like a system configuration which should be done once at startup?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@118">Patch Set #2, Line 118:</a> <code style="font-family:monospace,monospace">    def check_device_availability(self):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">where is this used? looks like a private/protected method.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@123">Patch Set #2, Line 123:</a> <code style="font-family:monospace,monospace">    def get_assigned_addr(self, ipv6=False):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">private/protected?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@125">Patch Set #2, Line 125:</a> <code style="font-family:monospace,monospace">        proc = self.run_androidue_cmd("get-ip", ['ip', 'addr', 'show'])</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think we already have some python code to get addresses in util.py, give it a look.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@134">Patch Set #2, Line 134:</a> <code style="font-family:monospace,monospace">    def get_carrier_id(self, carrier_name):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">private/protected?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@145">Patch Set #2, Line 145:</a> <code style="font-family:monospace,monospace">    def set_new_carrier(self, apn_parameter, carr_id):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">private/protected?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@170">Patch Set #2, Line 170:</a> <code style="font-family:monospace,monospace">    def set_preferred_apn(self, carr_id):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">private/protected?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@175">Patch Set #2, Line 175:</a> <code style="font-family:monospace,monospace">    def select_apn(self, carr_name):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">private/protected?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@185">Patch Set #2, Line 185:</a> <code style="font-family:monospace,monospace">    def delete_apn(self, carr_name):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">private/protected?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@192">Patch Set #2, Line 192:</a> <code style="font-family:monospace,monospace">    def run_androidue_cmd(self, name, popen_args, sync=True):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I remember seeing this exact same method somewhere else, am I right?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@207">Patch Set #2, Line 207:</a> <code style="font-family:monospace,monospace">            proc = self.rem_host.RemoteProcess(name, popen_args, remote_env={}, remote_port=self._run_node.remote_port())</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">remote_port inside rem_host</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@228">Patch Set #2, Line 228:</a> <code style="font-family:monospace,monospace">                proc.launch()</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">self.testenv.remember_to_stop beforehand, or simply don't call launch() inside here.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@231">Patch Set #2, Line 231:</a> <code style="font-family:monospace,monospace">    def run_netns_wait(self, name, popen_args):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is a public function IIRC (used by tests), so move it below to the public section.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@239">Patch Set #2, Line 239:</a> <code style="font-family:monospace,monospace">        if not self.check_device_availability():</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">how can this happen?sounds like this should be done by the system at startup (have some startup script running a loop checking this until all show up). How can that change during runtime?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@246">Patch Set #2, Line 246:</a> <code style="font-family:monospace,monospace">    def stop(self):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I see in general you wanting to stop too much stuff manually. That's why we have "remember_to_stop", so that the infrastructure takes care of processes, no need to explicitly stop them. Of course if you need to do other stuff then it's fine.</p><p style="white-space: pre-wrap; word-wrap: break-word;">stop() method is not used outside of the class btw, so it's private/protected.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@260">Patch Set #2, Line 260:</a> <code style="font-family:monospace,monospace">    def configure(self):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">private/protected</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@281">Patch Set #2, Line 281:</a> <code style="font-family:monospace,monospace">            "carrier": str(values['ue']['apn_name'] or 'default'),</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">As I mentioned it probably makes more sense to have a subnode for all apn settings in ms resources.conf, like you are doing here internally.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@320">Patch Set #2, Line 320:</a> <code style="font-family:monospace,monospace">                                      self.rem_host, self.ue_serial, self._run_node.remote_port())</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">remote_port in rem_hosy, you don't need specific param.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@346">Patch Set #2, Line 346:</a> <code style="font-family:monospace,monospace">            timer -= 1</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">so for each 2 seconds you sleep you decrease timer 1? this looks wrong (probably everybody would expect self.connect_timeout to be in seconds?)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@400">Patch Set #2, Line 400:</a> <code style="font-family:monospace,monospace">    def get_remote_port(self):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Not needed, info is available in run_node</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@403">Patch Set #2, Line 403:</a> <code style="font-family:monospace,monospace">    def set_airplane_mode(self, apm_state):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is probably private/protected.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@410">Patch Set #2, Line 410:</a> <code style="font-family:monospace,monospace">    def set_apn(self, apn_param, sel_apn):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is probably private/protected? is it used in tests?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/ms_android.py@421">Patch Set #2, Line 421:</a> <code style="font-family:monospace,monospace">    def scp_back_pcap(self):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is probably private/protected.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/qc_diag.py">File src/osmo_gsm_tester/obj/qc_diag.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/qc_diag.py@47">Patch Set #2, Line 47:</a> <code style="font-family:monospace,monospace">    def _clear_work_dirs(self):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This seems to be repeated from UE class?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/qc_diag.py@67">Patch Set #2, Line 67:</a> <code style="font-family:monospace,monospace">    def run_androidue_cmd(self, name, popen_args, sync=True):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Sounds like you want to have a base class with this methods implemented once instead of 3 times?<br>Something like "AndroidHost" or "AdbSshRemoteHost" or something like that.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/qc_diag.py@120">Patch Set #2, Line 120:</a> <code style="font-family:monospace,monospace">    def stop(self):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is probably protected/private</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/qc_diag.py@124">Patch Set #2, Line 124:</a> <code style="font-family:monospace,monospace">    def write_pcap(self):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is probably protected/private</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/qc_diag.py@158">Patch Set #2, Line 158:</a> <code style="font-family:monospace,monospace">    def get_rrc_state(self):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Is this used by tests?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/qc_diag.py@167">Patch Set #2, Line 167:</a> <code style="font-family:monospace,monospace">    def get_paging_counter(self):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Is this used by tests?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/src/osmo_gsm_tester/obj/qc_diag.py@171">Patch Set #2, Line 171:</a> <code style="font-family:monospace,monospace">    def running(self):</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">That's probably used internally only.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/sysmocom/suites/4g/iperf3_dl.py">File sysmocom/suites/4g/iperf3_dl.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302/2/sysmocom/suites/4g/iperf3_dl.py@26">Patch Set #2, Line 26:</a> <code style="font-family:monospace,monospace">proc = iperf3cli.prepare_test_proc(iperf3cli.DIR_DL, ue.netns(), bitrate=max_rate, ue=ue)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">ue.path() instead of pasing ue? Or ue.serial_device()?<br>Of even better, check ue.features('qcdiag') or whatever?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302">change 21302</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-gsm-tester/+/21302"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-gsm-tester </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I79a5d803e869a868d4dac5e0d4c2feb38038dc5c </div>
<div style="display:none"> Gerrit-Change-Number: 21302 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: ninjab3s <nils.fuerste@softwareradiosystems.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-CC: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 23 Nov 2020 20:38:13 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>