Attention is currently required from: dexter.

View Change

3 comments:

• File pySim/filesystem.py:

  • Patch Set #5, Line 746: __get_size

    so this function now actually doesn't return the size, but it returns a dict with a single element `{'total_len': xxx}`. This is

    a) odd API design. I would expect suhc a method to return just the `Optional[int]`, i.e. an integer or None
    b) down below you actually use it with named arguments like return `method(..., total_len = self.__get_size()` This means you are also passing a dict and not an integer to that argument...

  • Patch Set #5, Line 1343: def _encode_bin(self, abstract_data, total_len: int = None, **kwargs) -> bytes:

    this also doesn't really match the code above. total_len is *not* receiving an integer value but a dict `{'total_len': Optional[int]}`.

• File pySim/gsm_r.py:

  • Patch Set #5, Line 293: def _encode_record_bin(self, abstract_data : dict, record_nr : int, **kwargs) -> bytearray:

    _encode_record_bin now just has **kwargs, while the _encode_bin has a dedicated total_len argument. I think it should be consistent. Either both methods take a **kwargs as new argument (and then have to use `kwargs.get('total_len', None)` to get to the actual value), OR they should both be extended with an explicity `, total_len: Optional[int] = None, **kwargs`. But let's not make them inconsistent.

    _encode_record_bin should have the same signature as _encode_bin, with the exception that [of course] a record number must be passed to the former and not to the latter.

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