Attention is currently required from: keith.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/31131 )
Change subject: LCLS: Fix Global Call Reference generation
......................................................................
Patch Set 1:
(2 comments)
File src/libmsc/transaction.c:
https://gerrit.osmocom.org/c/osmo-msc/+/31131/comment/5d49ac94_b0fba89b
PS1, Line 165: 0x%x
0x04x to ensure consistent formatting
https://gerrit.osmocom.org/c/osmo-msc/+/31131/comment/fba7f7fd_f4fed6af
PS1, Line 165: %sfor
missing space between '%s' and 'for'
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/31131
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I9c33a89c819e8925d89ca833d7705ed5ced6b566
Gerrit-Change-Number: 31131
Gerrit-PatchSet: 1
Gerrit-Owner: keith <keith(a)rhizomatica.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: keith <keith(a)rhizomatica.org>
Gerrit-Comment-Date: Tue, 31 Jan 2023 21:04:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: laforge, dexter.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/24012 )
Change subject: filesystem: add unit tests for encoder/decoder methods
......................................................................
Patch Set 12: Code-Review+1
(3 comments)
Patchset:
PS12:
Looks good now!
File tests/test_files.py:
https://gerrit.osmocom.org/c/pysim/+/24012/comment/0038742d_bb179b05
PS12, Line 56: debug("Testing decode of %s" % name)
Not really critical (for tests), just a tip: documentation of the Python's logging framework recommends using lazy format-string composition instead of manual composition in-place. This can be achieved by passing format string arguments as arguments of the logging functions:
logging.debug("Testing decode of %s", name)
The idea is that the format string is evaluated only if it's actually going to be printed.
https://gerrit.osmocom.org/c/pysim/+/24012/comment/e42616b2_906e0d07
PS12, Line 160: for c in classes:
Can be done a bit simpler:
return filter(lambda c: c not in trans_rec_classes, classes)
Also, you can make it a class variable, so there would be no need to call get_classes() every time.
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/24012
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I02d884547f4982e0b8ed7ef21b8cda75237942e2
Gerrit-Change-Number: 24012
Gerrit-PatchSet: 12
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: Falkenber9 <robert.falkenberg(a)tu-dortmund.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 31 Jan 2023 20:44:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: fixeria, dexter.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/24012 )
Change subject: filesystem: add unit tests for encoder/decoder methods
......................................................................
Patch Set 12:
(2 comments)
Patchset:
PS8:
> I'm going to try to resurrect this after not receiving any updates for a very long time
Done
File pySim/filesystem.py:
https://gerrit.osmocom.org/c/pysim/+/24012/comment/9f6646ad_11a7f2d5
PS1, Line 383: file. To run this, simply define a _encode_decode_testvector[] testvector
> Mix of tabs and spaces, please do s/\t/ /g.
Done
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/24012
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I02d884547f4982e0b8ed7ef21b8cda75237942e2
Gerrit-Change-Number: 24012
Gerrit-PatchSet: 12
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: Falkenber9 <robert.falkenberg(a)tu-dortmund.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 31 Jan 2023 20:29:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
laforge has submitted this change. ( https://gerrit.osmocom.org/c/pysim/+/31123 )
Change subject: ts_31_102: Fix several bugs in EF_ECC encoder
......................................................................
ts_31_102: Fix several bugs in EF_ECC encoder
The encoder function apparently was never tested, it didn't match at all
the output of the decoder, not even in terms of the string keys of the
dict.
Change-Id: Id67bc39d52c4dfb39dc7756d8041cbd552ccbbc4
---
M pySim/ts_31_102.py
1 file changed, 6 insertions(+), 3 deletions(-)
Approvals:
Jenkins Builder: Verified
fixeria: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
diff --git a/pySim/ts_31_102.py b/pySim/ts_31_102.py
index 4d1c3dc..4125f4a 100644
--- a/pySim/ts_31_102.py
+++ b/pySim/ts_31_102.py
@@ -622,9 +622,12 @@
if in_json is None:
return b'\xff\xff\xff\xff'
code = EF_ECC.cc_construct.build(in_json['call_code'])
- svc_category = EF_ECC.category_construct.build(in_json['category'])
- alpha_id = EF_ECC.alpha_construct.build(in_json['alpha_id'])
- # FIXME: alpha_id needs padding up to 'record_length - 4'
+ svc_category = EF_ECC.category_construct.build(in_json['service_category'])
+ if 'alpha_id' in in_json:
+ alpha_id = EF_ECC.alpha_construct.build(in_json['alpha_id'])
+ # FIXME: alpha_id needs padding up to 'record_length - 4'
+ else:
+ alpha_id = b''
return code + alpha_id + svc_category
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/31123
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: Id67bc39d52c4dfb39dc7756d8041cbd552ccbbc4
Gerrit-Change-Number: 31123
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
laforge has submitted this change. ( https://gerrit.osmocom.org/c/pysim/+/31124 )
Change subject: Assume first record number if caller specifies none
......................................................................
Assume first record number if caller specifies none
This fixes a regression introduced in Change-Id
I02d6942016dd0631b21d1fd301711c13cb27962b which added support for
different encoding/decoding of records by their record number.
Change-Id: I0c5fd21a96d2344bfd9551f31030eba0769636bf
---
M pySim/filesystem.py
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Jenkins Builder: Verified
fixeria: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
diff --git a/pySim/filesystem.py b/pySim/filesystem.py
index 6dd1db7..ce1882b 100644
--- a/pySim/filesystem.py
+++ b/pySim/filesystem.py
@@ -945,7 +945,7 @@
self._construct = None
self._tlv = None
- def decode_record_hex(self, raw_hex_data: str, record_nr: int) -> dict:
+ def decode_record_hex(self, raw_hex_data: str, record_nr: int = 1) -> dict:
"""Decode raw (hex string) data into abstract representation.
A derived class would typically provide a _decode_record_bin() or _decode_record_hex()
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/31124
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I0c5fd21a96d2344bfd9551f31030eba0769636bf
Gerrit-Change-Number: 31124
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: laforge.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/31124 )
Change subject: Assume first record number if caller specifies none
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/31124
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: pysim
Gerrit-Branch: master
Gerrit-Change-Id: I0c5fd21a96d2344bfd9551f31030eba0769636bf
Gerrit-Change-Number: 31124
Gerrit-PatchSet: 2
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Tue, 31 Jan 2023 20:14:46 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment