Attention is currently required from: dexter.

fixeria 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:

(4 comments)

File pySim/global_platform/__init__.py:

https://gerrit.osmocom.org/c/pysim/+/42378/comment/6a6df36a_1ea13900?usp=ema... : PS3, Line 901: # `--install-parameters-*` are all optional

Better fix this in the patch before.

Done

File pySim/global_platform/install_param.py:

https://gerrit.osmocom.org/c/pysim/+/42378/comment/7c197ba7_3a7ad2ba?usp=ema... : PS3, Line 58: if non_volatile_memory_quota and volatile_memory_quota and stk_parameter:

I think the error here is that it should be 'or' instead of 'and'. […]

You're checking each parameter individually anyway, so this top-level if-statement is not needed. It's redundant, so I am removing it in this patch.

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

# If system specific parameters are present, add them to the install parameters

Well, this is literally what the code does:

* `if system_specific_params:` * If system specific parameters are present, * `install_params_dict.append(...)` * add them to the install parameters.

Do we really need that level of verbosity in comments?

File tests/unittests/test_globalplatform.py:

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

interesting that this does not change in behavior.

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

An empty string is not `None`, so `gen_install_parameters()` appends a single empty parameter. This causes the unit test to fail, so this is why I am removing the arguments here (this way they all become `None`).