Attention is currently required from: fixeria, laforge, osmith.

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

Change subject: global_platform: refactor gen_install_parameters() ......................................................................

Patch Set 4: Code-Review+2 Verified+1

(4 comments)

Patchset:

PS4: I have now looked at this again. I think the code is 100% correct now. I also re-tested the applet installation and it still works. I think we can merge this now.

File pySim/global_platform/install_param.py:

https://gerrit.osmocom.org/c/pysim/+/42378/comment/7d353d7d_0e6225af?usp=ema... : PS3, Line 60:

this looks correct, but I would add some comments to make the code easier to understand: […]

Done

https://gerrit.osmocom.org/c/pysim/+/42378/comment/fdb01422_854d156b?usp=ema... : PS3, Line 67: system_specific_params.append({'stk_parameter': stk_parameter})

Well, this is literally what the code does: […]

I am not sure. This was only a suggestion. If you think this is too much verbosity, than this is fine with me.

File tests/unittests/test_globalplatform.py:

https://gerrit.osmocom.org/c/pysim/+/42378/comment/5e3dfa33_3bc54d60?usp=ema... : PS3, Line 298: load_parameters = gen_install_parameters()

Not sure what you mean where, but let me explain why I am changing this line. […]

Thanks. I now understand. This worked before because of the bug in install_param.py, which is now fixed.