Attention is currently required from: neels.

dexter has posted comments on this change by neels. ( 
https://gerrit.osmocom.org/c/pysim/+/40198?usp=email )

Change subject: personalization: implement reading back values from a PES
......................................................................


Patch Set 11: Code-Review+1

(5 comments)

Patchset:

PS11:
I think this patch looks ok. I would recommend to look at decimal_hex_to_str() 
a bit more closely to make sure it actually does the right thing.


File pySim/esim/saip/personalization.py:

https://gerrit.osmocom.org/c/pysim/+/40198/comment/37303680_0841eeb4?usp=email :
PS11, Line 209:     def get_values_from_pes(cls, pes: ProfileElementSequence) 
-> Generator:
As far as I understand ''' just marks a multiline comment, but shouldn't this 
be a doctstring?


https://gerrit.osmocom.org/c/pysim/+/40198/comment/df8adf24_6dd87150?usp=email :
PS11, Line 290:     def decimal_hex_to_str(cls, val):
also have not seen single quotes as comment chars before...


https://gerrit.osmocom.org/c/pysim/+/40198/comment/7ddc4bfe_68a1a8c5?usp=email :
PS11, Line 292:         if isinstance(val, bytes):
Is it always ensured that we won't get sometimes bytes and sometimes bytearrays?


https://gerrit.osmocom.org/c/pysim/+/40198/comment/501ac867_7f438d57?usp=email :
PS11, Line 297:             val = unrpad(val, c)
You are sure that you want to turn val to ascii, like "4141" => "AA"? (am a bit 
confused since the method name suggests more that the input should be a 
hexstring or a bytearray with decimal digits and the output should be a string 
with the same digits.)



--
To view, visit https://gerrit.osmocom.org/c/pysim/+/40198?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I234fc4317f0bdc1a486f0cee4fa432c1dce9b463
Gerrit-Change-Number: 40198
Gerrit-PatchSet: 11
Gerrit-Owner: neels <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Attention: neels <[email protected]>
Gerrit-Comment-Date: Fri, 06 Mar 2026 11:54:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

Reply via email to