Attention is currently required from: osmith, arehbein, laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31877 )
Change subject: gm_12_21.h: Add cross-component parameter defaults (NSE timing)
......................................................................
Patch Set 2: Code-Review-1
(1 comment)
File include/osmocom/gsm/protocol/gsm_12_21.h:
https://gerrit.osmocom.org/c/libosmocore/+/31877/comment/c9681c15_3731b5c3
PS2, Line 294: enum abis_nm_par_defaults {
it makes no sense to have this as enums, because they are not different types of the same thing nor need to have different values (as you can see below, values are repeated).
Please use defines here.
BTW, I guess all these default values come from specs (TS 12.21?) because if they don't come from some spec reference, they have no place here, since it would actually be some implementation specific value of each app that we chose.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31877
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I51d1fd8b596523ae2ac8fb6a186ce7a702334c27
Gerrit-Change-Number: 31877
Gerrit-PatchSet: 2
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Fri, 31 Mar 2023 15:04:16 +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-ttcn3-hacks/+/32153 )
Change subject: MGCP_Test: support multiple codecs
......................................................................
Patch Set 4: Code-Review+1
(4 comments)
Patchset:
PS4:
I expect you did run BTS_Tests/MGCP_Tests/etc. here to validate everything is fine :)
File library/RTP_Emulation.ttcn:
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32153/comment/be686028_53a9…
PS4, Line 364: private function f_check_fixed_rx_payloads(octetstring rtp_data, INT7b rtp_payload_type) runs on RTP_Emulation_CT {
it feels a bit strange having the rtp_Data before the rtp_payload_type, but not criticial.
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32153/comment/857ad355_e021…
PS4, Line 376: if (rtp_data == g_cfg.rx_payloads[i].fixed_payload and rtp_payload_type == g_cfg.rx_payloads[i].payload_type) {
split into 2 lines?
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32153/comment/3cc0b786_bb72…
PS4, Line 388: if (payload_present and payload_error) {
This function implementation seem overcmplex with all those bool vars. Why aren't you incrementing the counters in the respective paths in the loop above? it would make everything more simpler afaict.
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/32153
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I8422313fccad1bfcee52c933f643068bebdaf2d5
Gerrit-Change-Number: 32153
Gerrit-PatchSet: 4
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 31 Mar 2023 14:57:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: osmith, laforge.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmocore/+/31877
to look at the new patch set (#2).
Change subject: gm_12_21.h: Add cross-component parameter defaults (NSE timing)
......................................................................
gm_12_21.h: Add cross-component parameter defaults (NSE timing)
These defaults are communicated via OML, but are also set as defaults in osmo-bsc and osmo-bts
Related: OS#5335
Change-Id: I51d1fd8b596523ae2ac8fb6a186ce7a702334c27
---
M include/osmocom/gsm/protocol/gsm_12_21.h
1 file changed, 75 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/77/31877/2
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31877
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I51d1fd8b596523ae2ac8fb6a186ce7a702334c27
Gerrit-Change-Number: 31877
Gerrit-PatchSet: 2
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: fixeria, daniel, lynxis lazus.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/32176 )
Change subject: gprs: fix has_valid_nsvc(): permit local udp port 0
......................................................................
Patch Set 1: Code-Review+1
(2 comments)
Patchset:
PS1:
> I am guessing you meant: https://gerrit.osmocom.org/c/osmo-bsc/+/32169.
yes sorry, I got confused.
> You have to use a different bind port anyway when running everything on the same host
Not really, you can use also another IP address.
> I see random bind port selection as a useful option, removing the need to pick and configure it manually.
ACK.
> AFAIU, osmo-sgsn "learns" the PCU's bind port during the initial NS handshake, yes. So it needs not to be known by the SGSN in advance. There is no config options in osmo-sgsn to specify the bind addresses of PCUs.
Fine then!
PS1:
I'd welcome review from daniel and lynxis here.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32176
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I0afd83e77f3daeeb082e7db911610428b5bc587c
Gerrit-Change-Number: 32176
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: daniel <dwillmann(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Fri, 31 Mar 2023 14:50:59 +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