Patch Set 1: (7 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? Agree Line 129: nr = self.dbus_obj()[I_NETREG] > this being called from is_connected() would raise a KeyError if the I_NETRE Makes sense Line 138: return status == 'registered' or status == 'roaming' > are 'registered' and 'roaming' fixed definitions from ofono? I hope so. Esp Yes, they are string tokens defined by ofono, see: https://github.com/intgr/ofono/blob/master/doc/network-api.txt#L78 Not sure if it's that worth moving it into a constant as I would expect it not to change anyway. Line 140: def connect(self, nitb): > hmm, I see a name duality arising: register and connect. Maybe we should re I agree. I can create a 2nd patch to do the connect()->register() Line 148: self.log('Already registered with the network') > what if the nitb arg reflects another network than the one we're connected Yes, this is not really detailed but for now it's enough. Even in connect() we should actually pass some Network code to which we should be connecting to, or None in case we want to use auto + default network. Then we should use all the Operator API from ofono, but I think all this can wait for later steps and for now consider we always try to connect automatically to default network. Line 154: self.dbg('Registered with network successfully: current status is %s', self.get_netreg_status()) > could call get_netreg_status() less often... Do you think is really an issue? it's not like it's taking seconds to do it. 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 f Agree. I will use RuntimeError for now. -- 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 <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <[email protected]> Gerrit-Reviewer: Pau Espin Pedrol <[email protected]> Gerrit-Reviewer: neels <[email protected]> Gerrit-HasComments: Yes
