Attention is currently required from: arehbein, fixeria, laforge, neels, pespin.
Hello Jenkins Builder, fixeria, laforge, neels, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email
to look at the new patch set (#13).
The following approvals got outdated and were removed:
Code-Review+1 by laforge, Code-Review-1 by fixeria, Code-Review-1 by neels, Verified+1 by Jenkins Builder
Change subject: Introduce per-BTS timers, RLC timer commands
......................................................................
Introduce per-BTS timers, RLC timer commands
- Add per-BTS timer groups ('rlc' only for now, others to follow)
- Add vty commands & tests for those timers
Related: OS#5335
Change-Id: I2c24110d8c977d6cc74c3c8e77bcc709ad9d2675
---
M include/osmocom/bsc/bts.h
M include/osmocom/bsc/gsm_data.h
M include/osmocom/bsc/vty.h
M src/osmo-bsc/bsc_init.c
M src/osmo-bsc/bsc_vty.c
M src/osmo-bsc/bts.c
M src/osmo-bsc/bts_init.c
M src/osmo-bsc/bts_ipaccess_nanobts_omlattr.c
M src/osmo-bsc/bts_vty.c
M src/osmo-bsc/pcu_sock.c
M tests/bts_features.vty
M tests/nanobts_omlattr/nanobts_omlattr_test.c
12 files changed, 475 insertions(+), 50 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/78/31878/13
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31878?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I2c24110d8c977d6cc74c3c8e77bcc709ad9d2675
Gerrit-Change-Number: 31878
Gerrit-PatchSet: 13
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/35139?usp=email )
Change subject: add comment in codec_mapping.c
......................................................................
add comment in codec_mapping.c
Related: OS#6258
Change-Id: I0905a1264cd8f940c7a9964addf241091425fe72
---
M src/libmsc/codec_mapping.c
1 file changed, 11 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/39/35139/1
diff --git a/src/libmsc/codec_mapping.c b/src/libmsc/codec_mapping.c
index 4063514..8eea242 100644
--- a/src/libmsc/codec_mapping.c
+++ b/src/libmsc/codec_mapping.c
@@ -111,6 +111,7 @@
},
{
.sdp = {
+ /* 112 is just what we use by default. The other call leg may impose a different number. */
.payload_type = 112,
.subtype_name = "AMR",
.rate = 8000,
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/35139?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I0905a1264cd8f940c7a9964addf241091425fe72
Gerrit-Change-Number: 35139
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: falconia, jolly, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35132?usp=email )
Change subject: Transmit invalid AMR speech blocks instead of dummy FACCH
......................................................................
Patch Set 2: -Code-Review
(1 comment)
File src/osmo-bts-trx/sched_lchan_tchf.c:
https://gerrit.osmocom.org/c/osmo-bts/+/35132/comment/8a135342_7e834baf
PS2, Line 541: * - If the channel mode is AMR, transmit a dummy with speech
> This is not specific to TCH/AHS at all, and is already briefly mentioned in the existing code. […]
And yes, as @falcon@freecalypso.org mentioned, it's actually FACCH/H replacing two speech frames; FACCH/F takes just one. My previous comment was in response to pespin's comment.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35132?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I056f379715c91ad968f198e112d363a9009dc1c3
Gerrit-Change-Number: 35132
Gerrit-PatchSet: 2
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Attention: falconia <falcon(a)freecalypso.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 27 Nov 2023 18:18:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: jolly, pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35132?usp=email )
Change subject: Transmit invalid AMR speech blocks instead of dummy FACCH
......................................................................
Patch Set 2:
(2 comments)
File src/osmo-bts-trx/sched_lchan_tchf.c:
https://gerrit.osmocom.org/c/osmo-bts/+/35132/comment/c1f9471e_0c017d04
PS2, Line 541: * - If the channel mode is AMR, transmit a dummy with speech
> I'd document here the rationale about "FACCH displaces two speech frames rather than one".
This is not specific to TCH/AHS at all, and is already briefly mentioned in the existing code. It's also just one of the reasons; another reason is that some Qualcomm basebands don't seem to like dummy FACCH and report bad RxQual.
So I would be fine with the current state of this patch.
https://gerrit.osmocom.org/c/osmo-bts/+/35132/comment/2cc61db2_e210eeb4
PS2, Line 578: dummy_facch:
> Maybe moving the GSM48_CMODE_SPEECH_AMR case inside the default with an "if" block is cleaner than a […]
Alternatively, we can move generation of dummy FACCH into a `static inline` function (`src/osmo-bts-trx/sched_utils.h` would be a good place for it) and just call it, hoping that the compiler would inline it properly for us.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35132?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I056f379715c91ad968f198e112d363a9009dc1c3
Gerrit-Change-Number: 35132
Gerrit-PatchSet: 2
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 27 Nov 2023 18:13:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: jolly, pespin.
falconia has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35132?usp=email )
Change subject: Transmit invalid AMR speech blocks instead of dummy FACCH
......................................................................
Patch Set 2:
(5 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-bts/+/35132/comment/501505b4_b36add36
PS2, Line 12: TCH/HS
TCH/AHS, not TCH/HS.
File src/osmo-bts-trx/sched_lchan_tchf.c:
https://gerrit.osmocom.org/c/osmo-bts/+/35132/comment/4d3c04d4_04757578
PS2, Line 541: AMR
Maybe say TCH/AFS rather than just AMR, for consistency.
https://gerrit.osmocom.org/c/osmo-bts/+/35132/comment/1d489c6e_ac89dd64
PS2, Line 541: * - If the channel mode is AMR, transmit a dummy with speech
> I'd document here the rationale about "FACCH displaces two speech frames rather than one".
This C module and function are for TCH/F only, hence the TCH/H-specific problem you are referring to does not apply here. We could keep doing dummy FACCH for all TCH/F modes, but the idea is to be consistent with what we do in TCH/H.
File src/osmo-bts-trx/sched_lchan_tchh.c:
https://gerrit.osmocom.org/c/osmo-bts/+/35132/comment/89f63bda_a3d1555f
PS2, Line 453: /* - If the channel mode is TCH/HS or TCH/EFS, transmit a dummy
The overall structure of the comment after your change no longer makes sense. Either cover both TCH/HS and TCH/AHS cases in one clause (with "CRC3 or CRC6" wording), or leave the non-AMR TCH/HS clause alone (like you did in TCH/F) and add a new clause for TCH/AHS as in TCH/H version of AMR.
https://gerrit.osmocom.org/c/osmo-bts/+/35132/comment/193d7373_6cc69db0
PS2, Line 453: TCH/EFS
If we are keeping the whole comment structure as-is (see my other comment), this part should read TCH/AHS and not TCH/EFS.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/35132?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I056f379715c91ad968f198e112d363a9009dc1c3
Gerrit-Change-Number: 35132
Gerrit-PatchSet: 2
Gerrit-Owner: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: falconia <falcon(a)freecalypso.org>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: jolly <andreas(a)eversberg.eu>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 27 Nov 2023 18:07:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment