Patch Set 2: (3 comments)
https://gerrit.osmocom.org/#/c/4704/2/src/osmo_gsm_tester/bts_sysmo.py File src/osmo_gsm_tester/bts_sysmo.py: Line 147: 'location_area_code': self.suite_run.lac(), > -1: Each BTS object must get a LAC *once*, conf_for_bsc() should be a "cons On practise it doesn't matter because we only call conf_for_bsc once per bsc, but I agree it's better to move it as you suggest. I'll leave the BtsBase class for later as it requires several changes and it's not entirely related to this topic. https://gerrit.osmocom.org/#/c/4704/2/src/osmo_gsm_tester/resource.py File src/osmo_gsm_tester/resource.py: Line 215: return self.next_persistent_value('lac', '1', schema.uint16, lambda x: str((int(x)+1) % pow(2,16)), origin) > -1: What if x starts out as -23? maybe str(max(0, ... )); * X cannot start out as -23 because the default is 1 and in any case if a negative value is read, schema.unit16() is going to throw and exception before it reaches the increment function. * Good catch, I'll fix it. * Me too, but in python I think it's clearer this way. * I don't think it makes sense, as it's only being used here. https://gerrit.osmocom.org/#/c/4704/2/src/osmo_gsm_tester/schema.py File src/osmo_gsm_tester/schema.py: Line 78: if n >= pow(2,16): > 0: lol, why not just > 65535 ... python goes on to calculate 2 ** 16 every because it's clearer, but I can use 65535 and then write in a comment 2^16-1 -- To view, visit https://gerrit.osmocom.org/4704 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9f864bac05e39ec2fc305f774194799c3d8fe1b0 Gerrit-PatchSet: 2 Gerrit-Project: osmo-gsm-tester Gerrit-Branch: master Gerrit-Owner: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Neels Hofmeyr <nhofm...@sysmocom.de> Gerrit-Reviewer: Pau Espin Pedrol <pes...@sysmocom.de> Gerrit-HasComments: Yes