3 Dec
2024
3 Dec
'24
11:59 a.m.
Attention is currently required from: JPM, dexter.
fixeria has posted comments on this change by JPM. (
https://gerrit.osmocom.org/c/pysim/+/38993?usp=email )
Change subject: Modem related fixes.
......................................................................
Patch Set 1: Code-Review-1
(5 comments)
Patchset:
PS1:
This patch should be split into smaller ones, ideally with some description and motivation
in their COMMIT_MSGs.
File pySim/cards.py:
https://gerrit.osmocom.org/c/pysim/+/38993/comment/cf65b68a_87491944?usp=em… :
PS1, Line 43: return [] #return empty ATR
This change looks suspicious. The type hint for this function is `Optional[Hexstr]`,
indicating that it returns either a `Hexstr` or `None`. Now you're changing it to
return a `list` instead of `None`. Why? What's the goal?
File pySim/transport/modem_atcmd.py:
https://gerrit.osmocom.org/c/pysim/+/38993/comment/afe707b3_cd79d3ee?usp=em… :
PS1, Line 59: timeout=1.5, patience=0.1
This should be proposed in a separate patch with some explanation why the values need to
be increased.
https://gerrit.osmocom.org/c/pysim/+/38993/comment/94c922c9_75f0d08a?usp=em… :
PS1, Line 111: def get_atr(self) -> Hexstr:
This also looks like a separate patch. I see we also have `get_atr()` in `PcscSimLink` and
`SerialSimLink`, but not in `CalypsoSimLink`. Neither this method is defined in `LinkBase`
(the abstract class). I don't remember seeing this method when writing this module...
https://gerrit.osmocom.org/c/pysim/+/38993/comment/8643752e_9d3cd0b3?usp=em… :
PS1, Line 137: Rebooting the modem to force card reset
AFAICS, you're reading the current `AT+CFUN` (operational) mode and then sending
`AT+CFUN` with exactly that same mode value that was read. Could you explain and/or give a
spec. reference on how is that supposed to reset the card?
I would expect doing `AT+CFUN=0` followed by `AT+CFUN=1` to reset the card, since the
modem would likely power off the card to ensure that the "minimum power is
drawn".
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/38993?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: I3e45603a13496fff41b9f859de486143252fb28d
Gerrit-Change-Number: 38993
Gerrit-PatchSet: 1
Gerrit-Owner: JPM <jean-pierre.marcotte.1(a)ens.etsmtl.ca>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: JPM <jean-pierre.marcotte.1(a)ens.etsmtl.ca>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 03 Dec 2024 11:59:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes