Attention is currently required from: Hoernchen, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/30416 )
Change subject: ms-trx support ......................................................................
Patch Set 3: Code-Review-1
(16 comments)
File Makefile.am:
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/b149085f_86bef51a PS3, Line 37: osmocom-bb/src/host/trxcon \
osmocom-bb/src/host/trxcon is added twice? here and above
Ack. Most likely he forgot to remove this line.
File Transceiver52M/Makefile.am:
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/d979a6e9_746c2026 PS3, Line 26: AM_CPPFLAGS '-I' stuff should be in CPPFLAGS only, it makes no sense to have it in CFLAGS/CXXFLAGS.
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/c6880f67_88b3bfc7 PS3, Line 77: LIBOSMOCODING_LIBS Having LIBOSMOCODING_LIBS in COMMON_LDADD makes all osmo-trx-* binaries depend on libosmocoding, while it's only needed for osmo-trx-ms-* and osmo-prbs-tool.
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/83a2d0fc_dd0a1d18 PS3, Line 90: noinst_HEADERS This should be done conditionally too, but not critical I guess.
File Transceiver52M/ms/itrq.h:
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/16aab7e0_8f217ba5 PS3, Line 34: /* weird comment formatting, please make it consistent with the copyright header
File Transceiver52M/ms/l1ctl_server_cb.cpp:
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/e0c228dc_35756759 PS3, Line 65: osmocom_l2 TODO: make path configurable
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/2921f8c4_7e180c4a PS3, Line 74: return so the app keeps running if l1ctl_server_alloc() fails?
File Transceiver52M/ms/logging.cpp:
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/d82e3853_8e6bb5aa PS3, Line 54: loglevel = LOGL_NOTICE spacing issues
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/8293fbce_b4ab0313 PS3, Line 57: DTRXC not used by libtrxcon nor needed
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/98faf6d8_0b0b4d10 PS3, Line 64: DTRXD not used by libtrxcon nor needed
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/9ac236f1_1832c8bf PS3, Line 75: .loglevel = LOGL_NOTICE, spacing issues
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/577062e9_90eb9294 PS3, Line 76: enabled = 0, why disabled?
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/40fa09ba_dd2d7c09 PS3, Line 82: .loglevel = LOGL_NOTICE, spacing issues
File configure.ac:
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/668f1798_05ce8f1c PS3, Line 330: test -d osmocom-bb A proper approach would be making this configurable: --enable-ms or --with-ms.
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/552f7b86_a9ccfcd7 PS3, Line 333: AC_MSG_NOTICE(["Enabling ms-trx..."]) spacing issues
https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/fd88649c_69bebf3b PS3, Line 364: doc/manuals/Makefile \
This looks like a separate patch.
Ack, it's one of my fixups: https://cgit.osmocom.org/osmo-trx/commit/?h=fixeria/ms&id=8ed86879c4ea9a...