Attention is currently required from: dexter, fixeria, laforge, pespin.
falconia has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/libosmo-abis/+/38290?usp=email )
Change subject: libosmotrau: add CSD RA2 functions
......................................................................
Patch Set 2:
(2 comments)
File include/osmocom/trau/csd_ra2.h:
https://gerrit.osmocom.org/c/libosmo-abis/+/38290/comment/1bf10950_25d9444b… :
PS1, Line 58:
> here he nbytes indicates the number of _output_ bytes.
See the other comment.
https://gerrit.osmocom.org/c/libosmo-abis/+/38290/comment/a63b900b_ead988b7… :
PS1, Line 66: ubit_t
> so here the nbytes indicats the number of _input_ bytes. I find that a bit confusing. […]
The suggested change has been made in the new patch version.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/38290?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Change-Id: I7b98b958651b3fc1a814d119d1b8644c91f98676
Gerrit-Change-Number: 38290
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 26 Sep 2024 23:01:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Attention is currently required from: dexter, falconia, fixeria, laforge, pespin.
Hello Jenkins Builder, dexter, fixeria, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-abis/+/38290?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review+1 by laforge, Code-Review+1 by pespin, Verified+1 by Jenkins Builder
Change subject: libosmotrau: add CSD RA2 functions
......................................................................
libosmotrau: add CSD RA2 functions
Rate adaption function RA2 for GSM CSD calls is defined in 3GPP TS
48.020 chapter 13. While it is nothing but one special case of I.460
submultiplexing and we do have i460_mux infrastructure in Osmocom,
that fully-general I.460 infra is too heaviweight for CSD encoding
and decoding functions that need simple, single-input and single-output
RA2 packing and unpacking functions.
Right now ad hoc RA2 functions are used in the trau_rtp_conv module
and in OsmoBTS csd_v110; same functions will also be needed for the
upcoming RAA' unit test addition. Add proper RA2 packing and unpacking
functions that can be used in all of these places.
Change-Id: I7b98b958651b3fc1a814d119d1b8644c91f98676
---
M include/Makefile.am
A include/osmocom/trau/csd_ra2.h
M src/Makefile.am
A src/trau/csd_ra2.c
4 files changed, 140 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/90/38290/2
--
To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/38290?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Change-Id: I7b98b958651b3fc1a814d119d1b8644c91f98676
Gerrit-Change-Number: 38290
Gerrit-PatchSet: 2
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Attention is currently required from: dexter.
laforge has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/38117?usp=email )
Change subject: ts_51_011: replace encoding of EF.MSISDN with construct model
......................................................................
Patch Set 10:
(1 comment)
File pySim/ts_51_011.py:
https://gerrit.osmocom.org/c/pysim/+/38117/comment/10dc5393_c09c3c99?usp=em… :
PS10, Line 191: _test_de_encode = [
plese also add some test vectors for the letacy case with `{'msisdn'...` so we know that doesn't break.
--
To view, visit https://gerrit.osmocom.org/c/pysim/+/38117?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: I647f5c63f7f87902a86c0c5d8e92fdc7f4350a5a
Gerrit-Change-Number: 38117
Gerrit-PatchSet: 10
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 26 Sep 2024 21:35:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: dexter.
laforge 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: Code-Review-1
(5 comments)
Patchset:
PS6:
I think what we see from the several last few iterations of this patchset is that there is a lack of testing. I argue the code of patchset 5 and 6 are broken - yet we do not detect this in unit tests. Please add some tests that exercise those code paths.
File pySim/filesystem.py:
https://gerrit.osmocom.org/c/pysim/+/38195/comment/be057514_7059e409?usp=em… :
PS6, Line 746: def __get_size(self, total_len: Optional[int] = None) -> int:
must be `-> Optional[int]` as it also may return `None`
https://gerrit.osmocom.org/c/pysim/+/38195/comment/0884379a_54f82d98?usp=em… :
PS6, Line 1054: def __get_rec_len(self, total_len: Optional[int] = None) -> int:
likewise: `-> Optional[int]`
https://gerrit.osmocom.org/c/pysim/+/38195/comment/25bd5ecf_253bd16c?usp=em… :
PS6, Line 1274:
here we still don't return an Optional[int] but a dict? I think that's inconsistent comparing the __get_size situation above with the one here for record oriented EF?
https://gerrit.osmocom.org/c/pysim/+/38195/comment/1cf177e7_df86be34?usp=em… :
PS6, Line 1303:
and this will again break. Your __get_rec_len() method wil return a dict and you're prefixing that with total_len, so the result is something like `{'total_len': {'total_len': 23}`
--
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: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 26 Sep 2024 21:33:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes