<p><a href="https://gerrit.osmocom.org/c/pysim/+/26165">View Change</a></p><p>8 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/pysim/+/26165/5/pySim-shell.py">File pySim-shell.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/26165/5/pySim-shell.py@101">Patch Set #5, Line 101:</a> <code style="font-family:monospace,monospace">  if type(profile) is CardProfileUICC or type(profile) is CardProfileUICCSIM:</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">In terms of readability, this is a better approach: […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/pysim/+/26165/5/pySim/filesystem.py">File pySim/filesystem.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/26165/5/pySim/filesystem.py@1508">Patch Set #5, Line 1508:</a> <code style="font-family:monospace,monospace">that is overloaded by specific dirived classes</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">If I understand correctly, this is an *abstract* static method. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/pysim/+/26165/5/pySim/profile.py">File pySim/profile.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/26165/5/pySim/profile.py@29">Patch Set #5, Line 29:</a> <code style="font-family:monospace,monospace"># In order for autodetection ...</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">So AFAIU, the order is important here. And this is why you're not using __subclasses__().</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Yes. You must first test for UICCSIM, then for UICC (without SIM) and the you can test for SIM. For example: When you test for SIM first, then you would e.g. recognize an UICCSIM as plain old SIM (which would even work, as they are compatible, but you would not be able to access its UICC features.)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/26165/5/pySim/profile.py@32">Patch Set #5, Line 32:</a> <code style="font-family:monospace,monospace">def profile_detect(scc:SimCardCommands):</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Missing type definition for returned value: […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/26165/5/pySim/profile.py@32">Patch Set #5, Line 32:</a> <code style="font-family:monospace,monospace">profile_detect</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I find it confusing that you create a new module called 'profile', which itself does not define clas […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">My plan was to implement it with a staticmethod, but I ran into problems with circular imports. I have tried it now again. Thanks to all_subcluasses function you provided it is not necessary to import the subclasses, so I managed to get around the problems and it worked this time.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/26165/5/pySim/profile.py@33">Patch Set #5, Line 33:</a> <code style="font-family:monospace,monospace"></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">ws</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/pysim/+/26165/5/pySim/ts_102_221.py">File pySim/ts_102_221.py:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/26165/5/pySim/ts_102_221.py@631">Patch Set #5, Line 631:</a> <code style="font-family:monospace,monospace">_match_sim</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">So this is basically a copy of CardProfileSIM.match_with_card()? […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/pysim/+/26165/5/pySim/ts_102_221.py@746">Patch Set #5, Line 746:</a> <code style="font-family:monospace,monospace">    # Add GSM specific files</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">tabs vs spaces</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">There is a huge problem with tabs vs spaces in this project. Someone created parts with an editor that used 4 space wide tabs. I wonder if there is some tool like indent for python out there.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/pysim/+/26165">change 26165</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/pysim/+/26165"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: pysim </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: If090d32551145f75c644657b90085a3ef5bfa691 </div>
<div style="display:none"> Gerrit-Change-Number: 26165 </div>
<div style="display:none"> Gerrit-PatchSet: 6 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: daniel <dwillmann@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 16 Nov 2021 14:54:57 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Comment-In-Reply-To: fixeria <vyanitskiy@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>