Attention is currently required from: Hoernchen, pespin.

fixeria 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 6:

(6 comments)

File Transceiver52M/grgsm_vitac/grgsm_vitac.h:

https://gerrit.osmocom.org/c/osmo-trx/+/33944/comment/402d44d8_32e18d97 PS6, Line 61: MULTI_VER_TARGET_ATTR_CLANGONLY Why removing this? If on purpose, please explain in the commit message.

https://gerrit.osmocom.org/c/osmo-trx/+/33944/comment/bfde6848_a3b770d8 PS6, Line 62: detect_burst_access_burst `detect_burst_ab()` for consistency with `detect_burst_nb()` then?

File Transceiver52M/grgsm_vitac/grgsm_vitac.cpp:

https://gerrit.osmocom.org/c/osmo-trx/+/33944/comment/5e2af26d_b3af6876 PS6, Line 77: MULTI_VER_TARGET_ATTR NO_UBSAN Why removing this? If on purpose, please explain in the commit message.

https://gerrit.osmocom.org/c/osmo-trx/+/33944/comment/03a0ecb3_d2d28bf6 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_ACCESS_BITS` is perhaps from the context of access bursts? Looks like the comment is copy-pasted? If it's for access bursts, maybe call it `d_acc_training_seq`?

https://gerrit.osmocom.org/c/osmo-trx/+/33944/comment/9c0c697b_0e8ea44b 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. I am find with that in general, but it makes the patch harder to read, so ideally should be done separately.

https://gerrit.osmocom.org/c/osmo-trx/+/33944/comment/0ffd49f1_528eceb7 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.