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)
-
Hoernchen