osmo-gsm-tester[master]: Use unique incrementing value for BTS LAC

[Neels Hofmeyr]

Patch Set 2: Code-Review-1

(5 comments)

https://gerrit.osmocom.org/#/c/4704/2/src/osmo_gsm_tester/bts_octphy.py
File src/osmo_gsm_tester/bts_octphy.py:

Line 162:                         'location_area_code': self.suite_run.lac(),
-1: see sysmo


https://gerrit.osmocom.org/#/c/4704/2/src/osmo_gsm_tester/bts_osmotrx.py
File src/osmo_gsm_tester/bts_osmotrx.py:

Line 140:                         'location_area_code': self.suite_run.lac(),
-1: see sysmo


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 "const" function. With this code, if a test case were to call conf_for_bsc() twice, then the same BTS object would exhibit mismatching LACs. I guess we should have a set_lac() function like we have set_msisdn() for the modems. We can call set_lac(suite_run.lac()) in bts_obj() in suite.py. Then overlay self.lac here. (either implement a set_lac() for each BTS class, or we could start having a BtsBase class which the others inherit from, so we have set_lac() implemented exactly once.)


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, ... ));

0: two spaces before pow;

0: In C I'd use (int(x)+1) & 0xffff but in python, ok;

0: The magic number "pow(2,16)" is used a lot, Does it make sense to have a LAC_MAX constant defined once somewhere? Is this the only one outside of the uint16() function?


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 time.


-- 
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 <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-HasComments: Yes