Attention is currently required from: Hoernchen.
dexter has posted comments on this change by Hoernchen. ( https://gerrit.osmocom.org/c/pysim/+/43559?usp=email )
Change subject: cat: fix Supported Radio Access Technologies state flag ......................................................................
Patch Set 2: Code-Review+2
(6 comments)
Patchset:
PS2: The patch looks ok to me, but I think the comments are a bit short. At least this time it was hard to follow, even though the patch is actually very simple.
File tests/unittests/test_cat.py:
https://gerrit.osmocom.org/c/pysim/+/43559/comment/93a90179_42629fa6?usp=ema... : PS2, Line 34: def test_enabled_is_encodable(self): maybe call this "test_encode_technology_enabled"
https://gerrit.osmocom.org/c/pysim/+/43559/comment/e01dc8c8_047e15df?usp=ema... : PS2, Line 35: """The flag used to have a bm of 0 so enabled -> 00 (that is disabled..)""" I don't understand what a 'bm' (bit mask?) is, but I guess you refer to the state?
https://gerrit.osmocom.org/c/pysim/+/43559/comment/f127d637_36d53a53?usp=ema... : PS2, Line 40: def test_disabled_differs_from_enabled(self): maybe call this "test_encode_technology_disabled"
https://gerrit.osmocom.org/c/pysim/+/43559/comment/e8550b52_3f6c2150?usp=ema... : PS2, Line 45: def test_state_byte_is_read_back_faithfully(self): maybe call this "test_decode_technology", maybe then also assert on the technology ('eutran')?
https://gerrit.osmocom.org/c/pysim/+/43559/comment/df0c4932_27e3a0ee?usp=ema... : PS2, Line 53: def test_multiple_technologies(self): ... then you could call this "test_decode_technology_multiple"
those are only suggestions, I don't want to block here.