Attention is currently required from: laforge.

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

Change subject: commands.py: Add support for multiple logical channels.
......................................................................


Patch Set 2:

(3 comments)

File pySim/commands.py:

https://gerrit.osmocom.org/c/pysim/+/34846/comment/6e5fcbd2_098448ac
PS2, Line 42:         if cla >> 4 in [0x0, 0xA, 0x8]:
I find this a bit difficult to read. Maybe cla & 0xF0 in [0x00, 0x0A, 0x80] is 
better. Also I am a bit lost here. The positions of the logical channel match 
ISO/IEC 7816, section 5.1.1 but 0xA0 and 0x80 are proprietary classes that are 
not encoded as per ISO/IEC 7816? Maybe a spec reference would help?


https://gerrit.osmocom.org/c/pysim/+/34846/comment/0701763c_40903ebc
PS2, Line 93:         if not cla:
the caller must decide when to access the cache and when not?


https://gerrit.osmocom.org/c/pysim/+/34846/comment/c4f8e6be_6324b2ab
PS2, Line 526:         return self._tp.send_apdu_checksw(self.cla_byte + 
self.cla4lchan('e0') + '0000%02x%s' % (len(payload)//2, payload))
Does this work? The class is E0 but E0 >> 4 is not in [0x0, 0xA, 0x8] (line 42)



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

Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Ibe5650dedc0f7681acf82018a86f83377ba81d30
Gerrit-Change-Number: 34846
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pma...@sysmocom.de>
Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de>
Gerrit-Attention: laforge <lafo...@osmocom.org>
Gerrit-Comment-Date: Tue, 24 Oct 2023 10:29:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to