Patch Set 1: (4 comments)
I guess I'm not removing "modem[].path" from resource.py:RESOURCES_SCHEMA because it can still be used as explained on my other comments. https://gerrit.osmocom.org/#/c/2676/1/src/osmo_gsm_tester/ofono_client.py File src/osmo_gsm_tester/ofono_client.py: Line 96: test.poll() > (if you ever come up with a better idea than my test.poll() that would be w We could move the glib mainloop outside of suiterun and make it global, then create a function to call it. But I think it's not a big issue for now. Line 143: self.path = conf.get('path') > there is no 'path' in the conf anymore, though? Are you allowing both, eith the SimManager interface is not mandatory afaik and thus we cannot force a feature based on that. If anybody ends up using a modem which doesn't support SimManager iface or it has a bug, then they can still use osmo-gsm-tester by setting up the "path" field. Also some people may be fine with hardcoding that instead of loosing time discovering the modems. Line 148: raise RuntimeError('Unable to find modem with IMSI %s (%s)' % (self.imsi, self.label)) > rather use self.raise_exn(), see log.Origin.raise_exn(). Ok Line 156: def imsi(self): > I guess we can drop the function entirely? Ok I'll keep using conf() stuff and then still use imsi() function instead of self.imsi var. -- To view, visit https://gerrit.osmocom.org/2676 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib9f4de81abc18e8db0c15df965e4811b6513e1b1 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-HasComments: Yes
