Attention is currently required from: laforge.
dexter has posted comments on this change by dexter. (
https://gerrit.osmocom.org/c/pysim/+/38195?usp=email )
Change subject: filesystem: pass total_len to construct of when encoding file contents
......................................................................
Patch Set 6:
(4 comments)
File pySim/filesystem.py:
https://gerrit.osmocom.org/c/pysim/+/38195/comment/50437558_f503ed26?usp=em… :
PS2, Line 1338: def _decode_bin(self, raw_bin_data: bytearray):
: chunks = [raw_bin_data[i:i+self.rec_len]
: for i in range(0, len(raw_bin_data), self.rec_len)]
: return [self.decode_record_bin(x) for x in chunks]
:
: def _encode_bin(self, abstract_data) -> bytes:
: chunks = [self.encode_record_bin(x) for x in abstract_data]
: # FIXME: pad to file size
: return b''.join(chunks)
There's nothing wrong with anticipating future
extensions and passing through `**kwargs`. […]
Acknowledged
File pySim/filesystem.py:
https://gerrit.osmocom.org/c/pysim/+/38195/comment/9c11083f_e8b068e2?usp=em… :
PS5, Line 746: __get_size
so this function now actually doesn't return the
size, but it returns a dict with a single element […]
Yes, that's really odd.
I thought it would simplify things a bit when we were using it only with
build_construct(), but now we use it in other places too. I have changed it now.
https://gerrit.osmocom.org/c/pysim/+/38195/comment/5cec70e2_a0facbdf?usp=em… :
PS5, Line 1343: def _encode_bin(self, abstract_data, total_len: int = None, **kwargs)
-> bytes:
this also doesn't really match the code above. […]
Done
File pySim/gsm_r.py:
https://gerrit.osmocom.org/c/pysim/+/38195/comment/92766fc6_cf8b1b28?usp=em… :
PS5, 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 we should use **kwargs
to transfer total_len. In most cases total_len is not needed at all, so I think **kwargs
is the better option here.
--
To view, visit
https://gerrit.osmocom.org/c/pysim/+/38195?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I1b7a51594fbc5d9fe01132c39354a2fa88d53f9b
Gerrit-Change-Number: 38195
Gerrit-PatchSet: 6
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Thu, 26 Sep 2024 09:37:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>