Attention is currently required from: dexter. laforge has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/39198?usp=email ) Change subject: javacard: add parser for JAVA-card CAP file format ...................................................................... Patch Set 2: (4 comments) File pySim/javacard.py: https://gerrit.osmocom.org/c/pysim/+/39198/comment/d23a8c26_74996a17?usp=em… : PS2, Line 47: __component AFAICT, you are basically declaring a _class_ variable here. It will be identical across all the instances of the class. However, it seems more like you're storing per-instance state in it? https://gerrit.osmocom.org/c/pysim/+/39198/comment/0a1e56aa_74c4dc93?usp=em… : PS2, Line 99: I think a generic implementation of a CAP file support shouldn't return the loadfile as hex-string but as bytes. The fact that whatever downstream apdu user we have may need hexstr should not have implications on a generic CAP file parser. Also, the function signature is missing the declaration of the return type ("-> Hexstr" in the current one, "-> bytes" in my proposal). https://gerrit.osmocom.org/c/pysim/+/39198/comment/336b0932_b5d57075?usp=em… : PS2, Line 118: def get_loadfile_aid(self): missing declaration of return type (-> Hexstr) https://gerrit.osmocom.org/c/pysim/+/39198/comment/e63bc638_b2b51e22?usp=em… : PS2, Line 127: def get_applet_aid(self, index:int = 0): missing declaration of return type (-> Hexstr) -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39198?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: I581483ccb9d8a254fcecc995fec3c811c5cf38eb Gerrit-Change-Number: 39198 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Fri, 03 Jan 2025 11:09:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No