Attention is currently required from: laforge.

View Change

2 comments:

• File pySim/esim/saip/personalization.py:

  • Patch Set #3, Line 408: return (pe for pe in l if pe.type == wanted_type)

    ProfileElementSequence.pe_by_type, yes! and also ProfileElementSequence. […]

    No!

    First, apply_val() looks up

    ~~~
    pes.pes_by_naa['mf']
    ~~~
       
    The result looks like this:

    ~~~
    >>> mf = pes.pes_by_naa['mf']
    >>> pprint.pprint(mf)
    [[<pySim.esim.saip.ProfileElementMF object at 0x7fcd2285e3c0>,
      <pySim.esim.saip.ProfileElementPuk object at 0x7fcd2285e900>,
      <pySim.esim.saip.ProfileElementPin object at 0x7fcd2285ea50>,
      <pySim.esim.saip.ProfileElementTelecom object at 0x7fcd2285eba0>,
      <pySim.esim.saip.ProfileElementGFM object at 0x7fcd2285ee40>,
      <pySim.esim.saip.ProfileElementPin object at 0x7fcd22856850>,
      <pySim.esim.saip.ProfileElementGFM object at 0x7fcd22856990>]]
    ~~~

    It then picks the first element [0] which is now a

    ~~~
    >>> type(mf[0])
    <class 'list'>
    ~~~

    So now we have just a py list of various ProfileElement subclass instances.
    Not a ProfileElementSequence.

    I can do

    ~~~
    >>> all_pincodes = pes.pe_by_type['pinCodes']
    >>> pprint.pprint(all_pincodes)
    [<pySim.esim.saip.ProfileElementPin object at 0x7fcd2285ea50>,
     <pySim.esim.saip.ProfileElementPin object at 0x7fcd22856850>,
     <pySim.esim.saip.ProfileElementPin object at 0x7fcd22856ad0>,
     <pySim.esim.saip.ProfileElementPin object at 0x7fcd22845cd0>,
     <pySim.esim.saip.ProfileElementPin object at 0x7fcd22845940>]
     ~~~

    But the result is different from the code I found, which looks up only the 'mf' naa:

    ~~~
    >>> mf_pincodes = list(pe for pe in mf[0] if pe.type == 'pinCodes')
    >>> pprint.pprint(mf_pincodes)
    [<pySim.esim.saip.ProfileElementPin object at 0x7fcd2285ea50>,
     <pySim.esim.saip.ProfileElementPin object at 0x7fcd22856850>]
    ~~~

    There are two questions for me:

    1. Is it correct to limit to 'mf'? Until now I didn't know there were other pinCodes in other naas too. (I did not create this code, just working with code I got)

    2. the pySim API: it seems slightly quirky design to have a ProfileElementSequence class, that in turn contains plain lists. That way those lookup members of ProfileElementSequence are not available in inner list levels. Would it make sense to use ProfileElementSequence instances on all inner lists, too? I think not, because it falls out of the asn1 decoding like this, right? Does it maybe make sense to have *only* global lookup functions that work on both ProfileElementSequence and list(ProfileElement) to avoid code dup? that would then defy those lookup dicts. Then again, are those dicts important, like, do they dramatically decrease lookup time?

    If we answer 1. with yes, limit to 'mf', and 2 with no, it's fine just as it is, then this patch is also fine as it is -- besides maybe moving the new function to a more prominent place.

    What are your thoughts? Thanks!

  • Patch Set #3, Line 452: def _apply_pinvalue(pe: ProfileElement, keyReference, val_bytes):

    so according to this type annotation, 'pe' is a single ProfileElement. […]

    it is indeed a List[ProfileElement] as i found out now .. related to my long comment. closing this speech bubble so we have one place to discuss.

To view, visit change 39744. To unsubscribe, or for help writing mail filters, visit settings.