Attention is currently required from: fixeria, pespin. Hoernchen has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/33944 ) Change subject: ms: restructure the va code to add rach support ...................................................................... Patch Set 8: (6 comments) File Transceiver52M/grgsm_vitac/grgsm_vitac.h: https://gerrit.osmocom.org/c/osmo-trx/+/33944/comment/1d340463_adcb2a78 PS6, Line 61: MULTI_VER_TARGET_ATTR_CLANGONLY

Why removing this? If on purpose, please explain in the commit message. Done

https://gerrit.osmocom.org/c/osmo-trx/+/33944/comment/e1a195e2_e6f73de0 PS6, Line 62: detect_burst_access_burst

`detect_burst_ab()` for consistency with `detect_burst_nb()` then? Done

File Transceiver52M/grgsm_vitac/grgsm_vitac.cpp: https://gerrit.osmocom.org/c/osmo-trx/+/33944/comment/3789d5b7_47636c20 PS6, Line 77: MULTI_VER_TARGET_ATTR NO_UBSAN

Why removing this? If on purpose, please explain in the commit message. Done

https://gerrit.osmocom.org/c/osmo-trx/+/33944/comment/0e60bd3a_b2b20e16 PS6, Line 42: d_sync_training_seq[N_ACCESS_BITS]; ///<encoded training sequence of a SCH burst

I find this confusing: the comment mentions SCH, the symbol name contains `_sync_`, and the `N_ACCES […] Done

https://gerrit.osmocom.org/c/osmo-trx/+/33944/comment/d8d832f5_153e8238 PS6, Line 71: gr_complex startpoint = train_seq[i][0] == 0 ? gr_complex(1.0, 0.0) : gr_complex(-1.0, 0.0);

Unrelated spacing changes. […] This file was mostly unformatted before to keep it "in sync" with gr-gsm, but now, due to all those changes, trying to keep the original formatting is pointless, which is why it now got automatically reformatted with the osmo-trx (kernel) coding style, which has longer line limits.

https://gerrit.osmocom.org/c/osmo-trx/+/33944/comment/08319b31_b6f07db3 PS6, Line 139: gmsk_output[i] = j * gr_complex(encoded_symbol, 0.0) * gmsk_output[i - 1];

Again, I am fine with proper formatting, but it makes your patch harder to read. Done

-- To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/33944 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-trx Gerrit-Branch: master Gerrit-Change-Id: I4a5cedc8c9a3289c75ce7b914eac286e601ebed0 Gerrit-Change-Number: 33944 Gerrit-PatchSet: 8 Gerrit-Owner: Hoernchen <ewild@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge@osmocom.org> Gerrit-Reviewer: pespin <pespin@sysmocom.de> Gerrit-CC: fixeria <vyanitskiy@sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy@sysmocom.de> Gerrit-Attention: pespin <pespin@sysmocom.de> Gerrit-Comment-Date: Fri, 28 Jul 2023 14:23:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy@sysmocom.de> Gerrit-MessageType: comment