Attention is currently required from: neels, dexter.

fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/31176 )

Change subject: support for Ericsson RBS E1 CCU
......................................................................


Patch Set 24:

(7 comments)

File configure.ac:

https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/4a9491fb_432ad9df
PS24, Line 90: PKG_CHECK_MODULES(LIBOSMOABIS, libosmoabis >= 1.4.0)
So the E1 CCU feature is configurable via `--enable-er-e1-ccu` and is disabled 
by default, but you're requiring `libosmo{abis,trau}` unconditionally. Maybe 
depend on these libraries only if the feature is enabled?


File contrib/jenkins.sh:

https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/9ef8c98e_73a7cf6b
PS24, Line 59:   PCU_CONFIG="$PCU_CONFIG --enable-werror --enable-sanitize"
You should add `--enable-er-e1-ccu` here, so that the conditional code is also 
built by Jenkins.


File contrib/osmo-pcu.spec.in:

https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/e5acb7d9_eea5f3d8
PS24, Line 36: 1.3.0
You require 1.4.0 in `configure.ac`, but 1.3.0 here?


https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/2e962ef1_e28a0ec4
PS24, Line 49: configure
Do we want to build RPM and DEB packages with `--enable-er-e1-ccu`?


File src/Makefile.am:

https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/c418f2b4_faf756d3
PS24, Line 218: EXTRA_DIST
Why do you put `.[ch]` files to `EXTRA_DIST`? This is unusual.


File src/ericsson-rbs/er_ccu_descr.h:

https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/e449870c_37b3ceaa
PS24, Line 3: #include <stdint.h>
missing `<stdbool.h>`


File src/ericsson-rbs/er_ccu_if.c:

https://gerrit.osmocom.org/c/osmo-pcu/+/31176/comment/f54e5d0d_af70b8b2
PS24, Line 412: tall_ccu_ctx = ctx;
Either obtain a child ctx here, or simply use `extern void *tall_pcu_ctx;`.



--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/31176
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I5c0a76667339ca984a12cbd2052f5d9e5b0f9c4d
Gerrit-Change-Number: 31176
Gerrit-PatchSet: 24
Gerrit-Owner: dexter <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-CC: fixeria <[email protected]>
Gerrit-CC: neels <[email protected]>
Gerrit-Attention: neels <[email protected]>
Gerrit-Attention: dexter <[email protected]>
Gerrit-Comment-Date: Tue, 07 Mar 2023 12:55:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

Reply via email to