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.orgPatch Set 1: (7 comments) looks good in principle, please don't be discouraged from the number of comments :) https://gerrit.osmocom.org/#/c/2449/1/src/osmo_gsm_tester/ofono_client.py File src/osmo_gsm_tester/ofono_client.py: Line 116: if interface_name == I_NETREG: elif? Line 129: nr = self.dbus_obj()[I_NETREG] this being called from is_connected() would raise a KeyError if the I_NETREG was not in the list. Instead, is_connected() should probably return False? So maybe return None in case there is no I_NETREG. Maybe this works: nr = self.dbus_obj().get(I_NETREG) if nr is None: return None What do you think? Line 138: return status == 'registered' or status == 'roaming' are 'registered' and 'roaming' fixed definitions from ofono? I hope so. Especially when we're using the same one several times ('roaming'), I would prefer using constants defined above, sort of like the I_NETREG. Line 140: def connect(self, nitb): hmm, I see a name duality arising: register and connect. Maybe we should rename all of them to 'register'? Line 148: self.log('Already registered with the network') what if the nitb arg reflects another network than the one we're connected to? I assume we usually want to disconnect and re-connect. Line 154: self.dbg('Registered with network successfully: current status is %s', self.get_netreg_status()) could call get_netreg_status() less often... Line 156: raise Exception('Failed to register with the network, current status is %s' % self.get_netreg_status()) so far I've been using RuntimeError (but we should also probably use more fine grained exceptions, like introduce an OfonoExn or ModemExn...) -- To view, visit https://gerrit.osmocom.org/2449 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1db8c7cba8a83746c16e1ca45f4b8aa0d595caf8 Gerrit-PatchSet: 1 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-Reviewer: neels <nhofmeyr at sysmocom.de> Gerrit-HasComments: Yes