Attention is currently required from: fixeria, osmith.
pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmocore/+/40295?usp=email )
Change subject: Introduce tests/osmo-config-merge testsuite
......................................................................
Patch Set 4:
(2 comments)
File tests/osmo-config-merge/atlocal.in:
PS4:
> do we need this empty file?
Yes, otherwise it failed when generating stuff iirc at configure time.
File tests/osmo-config-merge/testA.ok:
https://gerrit.osmocom.org/c/libosmocore/+/40295/comment/1f969dcf_2fd53942?… :
PS4, Line 74: power-ramp step-size 8000 mdB
: power-ramp step-interval 1
: phy 0 instance 0
: power-ramp step-size 2000 mdB
> is this correct? I would have expected that the new power-ramp step-size overwrites the previous one
Yes it's correct afaict, osmo-config-merge doesn't know about vty command internals and hence cannot know if 2 commands which are different test can actually be replaced.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40295?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ia172d4c280f4bae1b411ad805cca02355ca7fcc9
Gerrit-Change-Number: 40295
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 15 May 2025 14:29:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Attention is currently required from: fixeria, pespin.
osmith has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmocore/+/40295?usp=email )
Change subject: Introduce tests/osmo-config-merge testsuite
......................................................................
Patch Set 4:
(1 comment)
File tests/osmo-config-merge/Makefile.am:
https://gerrit.osmocom.org/c/libosmocore/+/40295/comment/3e85ca25_c5f03680?… :
PS4, Line 25: } >'$(srcdir)/package.m4'
> No idea tbh, copied it from tests/Makefile.am. Let me know if you know better about this stuff.
I'd try to remove it and see if it still works
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40295?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ia172d4c280f4bae1b411ad805cca02355ca7fcc9
Gerrit-Change-Number: 40295
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 15 May 2025 14:28:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Attention is currently required from: fixeria, osmith.
pespin has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmocore/+/40295?usp=email )
Change subject: Introduce tests/osmo-config-merge testsuite
......................................................................
Patch Set 4:
(4 comments)
File tests/osmo-config-merge/Makefile.am:
https://gerrit.osmocom.org/c/libosmocore/+/40295/comment/0c9de919_e0d9af3e?… :
PS4, Line 25: } >'$(srcdir)/package.m4'
> What do we need package. […]
No idea tbh, copied it from tests/Makefile.am. Let me know if you know better about this stuff.
https://gerrit.osmocom.org/c/libosmocore/+/40295/comment/10887003_0c96aef4?… :
PS4, Line 46: [ -e /proc/cpuinfo ] && cat /proc/cpuinfo
> This is already done in tests/Makefile.am, so I would remove it here.
Acknowledged
https://gerrit.osmocom.org/c/libosmocore/+/40295/comment/41c61360_43496d27?… :
PS4, Line 47: $(SHELL) '$(TESTSUITE)' $(TESTSUITEFLAGS) ENABLE_URING=$(ENABLE_URING) ENABLE_URING_TESTS=$(ENABLE_URING_TESTS)
> ```suggestion […]
Acknowledged
https://gerrit.osmocom.org/c/libosmocore/+/40295/comment/c125038f_544e472b?… :
PS4, Line 51: $(TESTSUITEFLAGS) ENABLE_URING=$(ENABLE_URING) ENABLE_URING_TESTS=$(ENABLE_URING_TESTS)
> ```suggestion […]
Acknowledged
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40295?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ia172d4c280f4bae1b411ad805cca02355ca7fcc9
Gerrit-Change-Number: 40295
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 15 May 2025 14:27:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Attention is currently required from: fixeria, pespin.
osmith has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmocore/+/40295?usp=email )
Change subject: Introduce tests/osmo-config-merge testsuite
......................................................................
Patch Set 4:
(6 comments)
File tests/osmo-config-merge/Makefile.am:
https://gerrit.osmocom.org/c/libosmocore/+/40295/comment/6944e778_68ec345b?… :
PS4, Line 25: } >'$(srcdir)/package.m4'
What do we need package.m4 for?
https://gerrit.osmocom.org/c/libosmocore/+/40295/comment/09c8c254_20aeb22f?… :
PS4, Line 46: [ -e /proc/cpuinfo ] && cat /proc/cpuinfo
This is already done in tests/Makefile.am, so I would remove it here.
https://gerrit.osmocom.org/c/libosmocore/+/40295/comment/c5a9850a_f208585d?… :
PS4, Line 47: $(SHELL) '$(TESTSUITE)' $(TESTSUITEFLAGS) ENABLE_URING=$(ENABLE_URING) ENABLE_URING_TESTS=$(ENABLE_URING_TESTS)
```suggestion
$(SHELL) '$(TESTSUITE)' $(TESTSUITEFLAGS)
```
https://gerrit.osmocom.org/c/libosmocore/+/40295/comment/d1a71072_698b27d1?… :
PS4, Line 51: $(TESTSUITEFLAGS) ENABLE_URING=$(ENABLE_URING) ENABLE_URING_TESTS=$(ENABLE_URING_TESTS)
```suggestion
$(TESTSUITEFLAGS)
```
File tests/osmo-config-merge/atlocal.in:
PS4:
do we need this empty file?
File tests/osmo-config-merge/testA.ok:
https://gerrit.osmocom.org/c/libosmocore/+/40295/comment/f755bf44_dfefc29a?… :
PS4, Line 74: power-ramp step-size 8000 mdB
: power-ramp step-interval 1
: phy 0 instance 0
: power-ramp step-size 2000 mdB
is this correct? I would have expected that the new power-ramp step-size overwrites the previous one
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/40295?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ia172d4c280f4bae1b411ad805cca02355ca7fcc9
Gerrit-Change-Number: 40295
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 15 May 2025 14:25:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: lynxis lazus.
pespin has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/37900?usp=email )
Change subject: s2b: answer echo requests
......................................................................
Patch Set 1: Code-Review-1
(1 comment)
File src/epdg_gtpc_s2b.erl:
https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/37900/comment/0013e146_e225… :
PS1, Line 520: IEs = [#v2_recovery{restart_counter = RCnt}],
This looks wrong. The RestartCounter we send has nothing to do with the one we receive.
Ideally we should read it at startup (or set to 0 if unable to read it), and increment and write it back after startup finishes.
For now I guess it's good enough to send a "0" or to send a timestamp?
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/37900?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: erlang/osmo-epdg
Gerrit-Branch: master
Gerrit-Change-Id: I3c470bd89b9811ba269b53eb400ffc21db4fba3a
Gerrit-Change-Number: 37900
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Thu, 15 May 2025 13:42:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes