This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Patch Set 6:
(13 comments)
I like the way this looks now. Let's resolve these minor issues...
https://gerrit.osmocom.org/#/c/2779/6/src/osmo_gsm_tester/ofono_client.py
File src/osmo_gsm_tester/ofono_client.py:
Line 83: def __async_result_handler(obj, result, user_data):
leading double underscores should be avoided, use one underscore
Line 107: def dbus_async_call(proxymethod, instance, *args, **kwargs):
I would prefer writing out the kwargs timeout, result_handler, error_handler and user_data. IIUC like this?
def dbus_async_call(proxymethod, instance,
*proxymethod_args,
result_handler=None, error_handler=None,
user_data=None, timeout=30,
**proxymethod_kwargs)
Line 401: # waiting for that. Make it async and try to regsiter when the scan is
(typo regsiter)
Line 407: self.raise_exn()
no message? Let's make it an explicit scan_error_cb and have a meaningful message?
Line 409: def scan_result_cb(self, obj, result, user_data):
'result' should have a meaningful name. scanned_operators?
Line 412: def register(self, ops):
'ops' should be more descriptive: scanned_operators?
Line 414: if self.mcc is None or self.mnc is None:
better: self.mcc_mnc is None
Line 415: if not self.is_connected():
shouldn't this check be independent of whether mcc+mnc were passed or not?
And, wouldn't this check make more sense before we start scanning?
Line 427: self.log('Already registered with the network')
should we also check this before we start scanning?
Line 454: self.mcc, self.mnc = mcc_mnc
why not also store as self.mcc_mnc
Line 455: if (self.mcc is None and self.mnc is not None) or (self.mcc is not None and self.mnc is None):
len(mcc_mnc) != 2 or None in mcc_mnc
(we could also add arbitrary input validation, string type of non-whitespace integer number..... but I guess that's not necessary)
https://gerrit.osmocom.org/#/c/2779/6/suites/debug/interactive.py
File suites/debug/interactive.py:
Line 30: wait(m.is_connected)
we could reverse the order, i.e. first wait for the modems to be connected. The nitb or msc checks do repeated CTRL requests that would arguably bloat the pcaps; the is_connected() would query DBus. (same for the other interactive.py and the mo_mt_sms.py and register* test scripts)
https://gerrit.osmocom.org/#/c/2779/6/suites/netreg/register.py
File suites/netreg/register.py:
Line 16: ms.connect(nitb.mcc_mnc())
wait(is_connected)?
--
To view, visit https://gerrit.osmocom.org/2779
To unsubscribe, visit https://gerrit.osmocom.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8d9eb47eac1044550d3885adb55105c304b0c15c
Gerrit-PatchSet: 6
Gerrit-Project: osmo-gsm-tester
Gerrit-Branch: master
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-HasComments: Yes