Attention is currently required from: JPM, dexter.
Patch set 1:Code-Review -1
5 comments:
Patchset:
This patch should be split into smaller ones, ideally with some description and motivation in their COMMIT_MSGs.
File pySim/cards.py:
Patch Set #1, 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:
Patch Set #1, 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.
Patch Set #1, 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...
Patch Set #1, 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 change 38993. To unsubscribe, or for help writing mail filters, visit settings.