Attention is currently required from: osmith, fixeria.
Hello osmith, Jenkins Builder, laforge, fixeria,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-gprs/+/32050
to look at the new patch set (#2).
Change subject: gmm: Initial implementation of GPRS Detach
......................................................................
gmm: Initial implementation of GPRS Detach
This patch contains further work on several areas for the GMM layer,
like fixes and improvements in existing primitives, initial FSM
implementation, initial Tx and Rx of some GMM messages, etc.
Related: OS#5501
Change-Id: If6cbb1d425b3a9f713348f1dea4747e2b6be0a44
---
M include/osmocom/gprs/gmm/gmm_ms_fsm.h
M include/osmocom/gprs/gmm/gmm_pdu.h
M include/osmocom/gprs/gmm/gmm_prim.h
M include/osmocom/gprs/gmm/gmm_private.h
M src/gmm/gmm.c
M src/gmm/gmm_ms_fsm.c
M src/gmm/gmm_pdu.c
M src/gmm/gmm_prim.c
M tests/gmm/gmm_prim_test.c
M tests/gmm/gmm_prim_test.err
M tests/gmm/gmm_prim_test.ok
11 files changed, 300 insertions(+), 34 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-gprs refs/changes/50/32050/2
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/32050
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: If6cbb1d425b3a9f713348f1dea4747e2b6be0a44
Gerrit-Change-Number: 32050
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: osmith, fixeria.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/32050 )
Change subject: gmm: Initial implementation of GPRS Detach
......................................................................
Patch Set 1:
(3 comments)
Commit Message:
https://gerrit.osmocom.org/c/libosmo-gprs/+/32050/comment/7c1c11e0_8e1b6bbe
PS1, Line 11: implementation, initial Tx and Rx of some GMM messages, etc.
> (would probably be easier to review if this was split into multiple patches, but I guess you had a r […]
This is all really an initial implementation filling up base code in several places. I just submit the bunch of improvements every now and then when I have another feature more or less filled.
File src/gmm/gmm.c:
https://gerrit.osmocom.org/c/libosmo-gprs/+/32050/comment/72f828ed_0812dd8c
PS1, Line 376: /* TODO:
> kept this on purpose?
Yes, I still need to find out how to fill up those. I'm writing now most of the bulk code so that later on each detail like this one can be improved/fixed in parallel with others.
https://gerrit.osmocom.org/c/libosmo-gprs/+/32050/comment/eb32f629_adc5d424
PS1, Line 471: force_stanbdy_indicated
> typo in standby
Ack
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/32050
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: If6cbb1d425b3a9f713348f1dea4747e2b6be0a44
Gerrit-Change-Number: 32050
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 27 Mar 2023 15:39:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin, fixeria.
osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/32050 )
Change subject: gmm: Initial implementation of GPRS Detach
......................................................................
Patch Set 1: Code-Review+1
(3 comments)
Commit Message:
https://gerrit.osmocom.org/c/libosmo-gprs/+/32050/comment/e8830507_35898898
PS1, Line 11: implementation, initial Tx and Rx of some GMM messages, etc.
(would probably be easier to review if this was split into multiple patches, but I guess you had a reason for doing it that way)
File src/gmm/gmm.c:
https://gerrit.osmocom.org/c/libosmo-gprs/+/32050/comment/e744dceb_247dd55f
PS1, Line 376: /* TODO:
kept this on purpose?
https://gerrit.osmocom.org/c/libosmo-gprs/+/32050/comment/7bfdf7cc_a470880f
PS1, Line 471: force_stanbdy_indicated
typo in standby
--
To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/32050
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-gprs
Gerrit-Branch: master
Gerrit-Change-Id: If6cbb1d425b3a9f713348f1dea4747e2b6be0a44
Gerrit-Change-Number: 32050
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 27 Mar 2023 15:35:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: osmith.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/32085 )
Change subject: msc_main: close SMS db on startup error
......................................................................
Patch Set 1: Code-Review-1
(2 comments)
File src/osmo-msc/msc_main.c:
https://gerrit.osmocom.org/c/osmo-msc/+/32085/comment/d1a2dedf_0b490d23
PS1, Line 805: db_fini();
really looks like you want to use a goto here....
https://gerrit.osmocom.org/c/osmo-msc/+/32085/comment/fd52a6db_3bbe3b7e
PS1, Line 829: db_fini();
to here....
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/32085
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I9bb799048db5fcdb2a2520107bd75d5f7a865459
Gerrit-Change-Number: 32085
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 27 Mar 2023 14:34:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/27391 )
Change subject: Revert "mgcp_codec: do not differentiate between oa and bwe when comparing codec"
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/27391
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I0b2854ef2397f38606fab3425be586a3d0ca27d1
Gerrit-Change-Number: 27391
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 27 Mar 2023 14:33:14 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/32084 )
Change subject: mgcp_codec: cosmetic: remove line break in api-doc
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I sometimes add them, I don't really see anything wrong with this tbh.
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32084
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I025bbf36287ef014e294cc18a6435d7e2f9c1bff
Gerrit-Change-Number: 32084
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 27 Mar 2023 14:30:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: falconia, laforge.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/32078 )
Change subject: osmo_rtp2trau() for FR & EFR: correctly handle the no-data case
......................................................................
Patch Set 1:
(2 comments)
File src/trau/trau_rtp_conv.c:
https://gerrit.osmocom.org/c/libosmo-abis/+/32078/comment/89c42035_f31a6b87
PS1, Line 282: } else if (data_len == 0) {
: /* C1 .. C5: idle speech */
: tf->c_bits[0] = 0;
: tf->c_bits[1] = 1;
: tf->c_bits[2] = 1;
: tf->c_bits[3] = 1;
: tf->c_bits[4] = 0;
> Shouldn't this clause come before/independent of the UL/DL if/else clause? […]
i also wonder if it is intentional to indicate idle speech only for, which is it, DL? I guess if it can be a standalone 'if (data_len == 0)' condition acting on both DL and UL, that would already make the flow easier to read.
https://gerrit.osmocom.org/c/libosmo-abis/+/32078/comment/9bd5d1dd_8cb46b3f
PS1, Line 441: if (data_len == 0 && tf->dir == OSMO_TRAU_DIR_DL) {
(wondering as above: why only for DL?)
--
To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/32078
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Change-Id: I7f21e2210bba9ef87f1af515a001a0cfc1c0a1d8
Gerrit-Change-Number: 32078
Gerrit-PatchSet: 1
Gerrit-Owner: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Mon, 27 Mar 2023 14:27:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment
Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/32083 )
Change subject: mgcp_codec: refactor payload type converstion
......................................................................
Patch Set 1:
(3 comments)
File tests/mgcp/mgcp_test.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5466):
https://gerrit.osmocom.org/c/osmo-mgw/+/32083/comment/090e783f_4b9ab825
PS1, Line 2059: } else {
else is not generally useful after a break or return
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5466):
https://gerrit.osmocom.org/c/osmo-mgw/+/32083/comment/31efe181_9f0114c9
PS1, Line 2067: } else {
else is not generally useful after a break or return
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-5466):
https://gerrit.osmocom.org/c/osmo-mgw/+/32083/comment/aa30afac_3d9a9b42
PS1, Line 2122: result = pt_translate(conn, 0, 1,expect->payload_type_map[0]);
space required after that ',' (ctx:VxV)
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/32083
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I085260a2ca8cfecdb58656b7a046c536189e238d
Gerrit-Change-Number: 32083
Gerrit-PatchSet: 1
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-CC: Jenkins Builder
Gerrit-Comment-Date: Mon, 27 Mar 2023 13:57:21 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment