Patch Set 2: (3 comments)
https://gerrit.osmocom.org/#/c/3722/2//COMMIT_MSG Commit Message: Line 31: - 'a5 1' > (writing this last) All this is quite tricky and we should talk in person about it, indeed. > if a scenario says [a5_0, a5_1], it implies that a BTS must support *both* > a5_0 and a5_1 at the same time, not that it may support either of the two. I know, and that's the intention here. Because in the same suite I'm doing tests which require a50 and a51 on others. This means I need a BTS which can handle both. It's expected . Of course we can also split the suite into several suites and ask for one at each time, but I didn't require it for now because we have BTS supporting both. > A scenario is by nature imperative, and adding a scenario shall only add more > restriction, never enlarge the scope of permitted options. Yes, that's what it does. Adding a scenario enlarges the list of required option. As a result, adds more restriction. If you have a set A (resources), and a set B (resources/requirements), by adding an element to the array in B, you are enlarging set B. Thus, when intersecting A with B, the result C (valid resources) is same or smaller than it should have been if you didn't enlarge B with the field. As far as I understand, the final "resources" set B, is an aggregation (union) of sub sets, each sub set being one leave in the tree of resources (eg union(set(modem.label=foo), set(modem.has_cipher_a50_in_list), set(modem.has_cipher_a51_in_list)) = set(modem.label=foo, modem.has_cipher_a50_in_list, modem.has_cipher_a51_in_list). So, using a list is just a handy way of group naming specific attributes which can be used to form sets. Hopefully I made myself clearer and it makes sense. I had to think about this while doing this patch. > To change this, we'd need some 'a OR b' operator, but so far adding a > scenario is > strictly 'AND' implicitly. I agree, I also thought about this option, and we may require it at some point (having some way to OR scenarios instead of joining them). Something like: "osmo-gsm-tester -s sms:cipher-a50~cipher-a51" Note the "~" character instead of +. This would basically mean something like "try matching resources against cipher-a50, and if it fails, try with ciper-a51". This of course cold be nested: "osmo-gsm-tester -s sms:(cipher-a50+cipher-a51)~(cipher-a52+cipher-a53)" Which would mean something like "I want a modem/bts which supports a50 ANR a51, OR otherwise give me one which supports a52 AND a53." Of course from GSM point of view this example may not make sense, but hopefully I explain the idea. Do we need this for now? No. Can we work around need for this for now/ever? Yes, probably. https://gerrit.osmocom.org/#/c/3722/2/src/osmo_gsm_tester/config.py File src/osmo_gsm_tester/config.py: Line 249: assert type(dest[i]) == type(src[i]) > this is fragile: tuple and list are both iterable and can be combined, yet I think I never saw any checks for tuples in this code, so if I understand correctly we don't support using tuples here? Line 253: dest.append(src[i]) > I need to think hard here ... maybe a comment would be good. It's the case in which src and dst are lists containing "basic" values (ie. not lists or dictionaries), and then we need to merge (combine) elements from both lists. As the function combine() merges src into dst, we append src elements not present in dst into dst. This way, at the end dst is a superset of src, exactly what we are looking at here in general. When combine() finishes, dst is a superset of src. As a superset, then when intersecting with the available resources, it will provide a subset of available combinations, and it's what we are looking at. -- To view, visit https://gerrit.osmocom.org/3722 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib7a38f10eb9de338a77bf1fa3afceb9df1532015 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
