Attention is currently required from: Hoernchen. dexter has posted comments on this change by Hoernchen. ( https://gerrit.osmocom.org/c/python/pyosmocom/+/43171?usp=email )
Change subject: tlv: preserve the comprehension bit ...................................................................... Patch Set 1: Code-Review+1 (5 comments) Patchset: PS1: I have looked through your patch and I think it is plausible, however it introduces a state that other TLV classes don't have. Maybe this needs some explanation. Commit Message: https://gerrit.osmocom.org/c/python/pyosmocom/+/43171/comment/fec56f4a_fb906217?usp=email : PS1, Line 35: instead of setting the flag, currently tag=0x8123 -> 0x81a3. I would split this in a separate patch (only if you think this would speed things up, as I think this part would be ready at least.) File src/osmocom/tlv.py: https://gerrit.osmocom.org/c/python/pyosmocom/+/43171/comment/4228f038_52c1dbdc?usp=email : PS1, Line 658: # True/False: as received Maybe it makes sense to explain this property in greater detail? This is an additional state an API user would expect right away. I understand this as a state that flips to the state of the last parsed CR bit when from_tlv() has been used. This basically means that when I have used from_tlv() once, the API user cannot rely on the CR bit set as originally defined. This is probably a corner case as the IE objects are usually short lived and probably not wildly mixed. Decode, change something, Re-Encode is probably the most exciting usecase to expect but I still think we should have an explanatory comment here. File tests/test_tlv.py: https://gerrit.osmocom.org/c/python/pyosmocom/+/43171/comment/873ef7ec_0ebdc16a?usp=email : PS1, Line 83: I would add a comment that explains that the first vector has the CR bit set, but the second has not. And both must come out the same when they are decoded and re-encoded. https://gerrit.osmocom.org/c/python/pyosmocom/+/43171/comment/81a1304c_23ed10f1?usp=email : PS1, Line 86: ie.from_tlv(encoded) maybe add an assert that checks that ie.comprension has changed state as expected? -- To view, visit https://gerrit.osmocom.org/c/python/pyosmocom/+/43171?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: python/pyosmocom Gerrit-Branch: master Gerrit-Change-Id: Ifae37785e2a586d9130d154bc7244f35fa6f2a55 Gerrit-Change-Number: 43171 Gerrit-PatchSet: 1 Gerrit-Owner: Hoernchen <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Attention: Hoernchen <[email protected]> Gerrit-Comment-Date: Fri, 07 Aug 2026 11:30:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes
