Attention is currently required from: laforge, fixeria. Hoernchen 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 11: (8 comments) File utils/Makefile.am: https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/872b1a46_5f73e678 PS10, Line 2: ..

You wouldn't need these relative paths if you used `$(top_srcdir)`. Done

https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/91330db1_753b756f 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. Done

https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/00b40a68_5b00a727 PS10, Line 3: lpthread

This is actually not a compiler flag, so `LDADD` is a better fit. […] Done

https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/e29cd63d_9fadaa10 PS10, Line 4: LIBOSMOCODING_CFLAGS

Is libosmocoding also needed by your new tool? If not, move it to `osmo_prbs_tool_CFLAGS`. Done

https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/efe03561_fb1b351c PS10, Line 13: burst-gen

Add `osmo-` prefix for consistency with `osmo-prbs-tool`? Done

https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/938e5130_13136168 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. Done

https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/ef80e976_4e38a8f3 PS10, Line 17: ../Transceiver52M/libtransceiver_common.la \

tabs vs spaces Done

File utils/va-test/burst-gen.cpp: https://gerrit.osmocom.org/c/osmo-trx/+/33945/comment/4a64fe8d_a67c31ca PS10, Line 524: int main()

doesn't compiler warn about missing `return (int)`? No, see https://en.cppreference.com/w/cpp/language/main_function 4)

-- 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: 11 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: laforge <laforge@osmocom.org> Gerrit-Attention: fixeria <vyanitskiy@sysmocom.de> Gerrit-Comment-Date: Fri, 28 Jul 2023 13:38:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy@sysmocom.de> Gerrit-MessageType: comment