Attention is currently required from: Hoernchen, laforge. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/33945 ) Change subject: ms/va: add test tool and data ...................................................................... Patch Set 10: (8 comments) File utils/Makefile.am: https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/b2d0c968_07e9ac43 PS10, Line 2: .. You wouldn't need these relative paths if you used `$(top_srcdir)`. https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/3ba2bcaf_d9ddc398 PS10, Line 2: AM_CPPFLAGS = -Wall $(STD_DEFINES_AND_INCLUDES) -I${srcdir}/../Transceiver52M/arch/common -I${srcdir}/../Transceiver52M/device/common -I${srcdir}/../Transceiver52M Also, for the sake of readability, one item per line please. https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/445883d2_a105d89c PS10, Line 3: lpthread This is actually not a compiler flag, so `LDADD` is a better fit. If it's needed only for the new binary you're adding, then please add it to `burst_gen_LDADD`. https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/61832549_38a9fda2 PS10, Line 4: LIBOSMOCODING_CFLAGS Is libosmocoding also needed by your new tool? If not, move it to `osmo_prbs_tool_CFLAGS`. https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/18e73255_a9601ef6 PS10, Line 13: burst-gen Add `osmo-` prefix for consistency with `osmo-prbs-tool`? https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/23599ea2_e9d571a7 PS10, Line 15: burst_gen_SOURCES = va-test/burst-gen.cpp ${srcdir}/../Transceiver52M/grgsm_vitac/grgsm_vitac.cpp ${srcdir}/../Transceiver52M/grgsm_vitac/viterbi_detector.cc Same here, one file per line please. https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/7df14252_ffcc695a PS10, Line 17: ../Transceiver52M/libtransceiver_common.la \ tabs vs spaces File utils/va-test/burst-gen.cpp: https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/fadbe803_98ad60c7 PS10, Line 524: int main() doesn't compiler warn about missing `return (int)`? -- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/33945 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: Ie38ed4996134e84ad0481872a743299bb442b3db Gerrit-Change-Number: 33945 Gerrit-PatchSet: 10 Gerrit-Owner: Hoernchen <ewild@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin@sysmocom.de> Gerrit-CC: fixeria <vyanitskiy@sysmocom.de> Gerrit-CC: laforge <laforge@osmocom.org> Gerrit-Attention: Hoernchen <ewild@sysmocom.de> Gerrit-Attention: laforge <laforge@osmocom.org> Gerrit-Comment-Date: Thu, 27 Jul 2023 21:16:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment