Attention is currently required from: Hoernchen, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-trx/+/30416 )

Change subject: ms-trx support ......................................................................

Patch Set 3:

(21 comments)

File Transceiver52M/ms/ms_upper.h:

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/de8e74a6_3a259a08 PS3, Line 1: ws

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/0a302518_73cc4d5c PS3, Line 36: trx_if do you really need/use TRXC/TRXD here?

File Transceiver52M/ms/ms_upper.cpp:

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/32a010d7_8a17baca PS3, Line 61: #include <osmocom/core/gsmtap_util.h> you're not using GSMTAP API here...

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/226cc199_31255475 PS3, Line 64: // #include <osmocom/core/application.h> removeme

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/93fc1081_e6a4c799 PS3, Line 71: trx_if do you really need/use TRXC/TRXD here?

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/fef1b6b3_90d5b9f3 PS3, Line 74: #include <osmocom/bb/l1sched/l1sched.h> I don't see any l1sched_* API used in this file.

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/7aa85f32_4e0185b4 PS3, Line 177: return trash can you clarify this a bit further? thrash?

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/8b6afa5b_a3a7daf3 PS3, Line 184: RSSI = 10 why 10?

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/3aeccd0b_dd0bd268 PS3, Line 223: timingOffset = (int)round(0); TODO/FIXME?

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/164d3c60_0da5723e PS3, Line 238: trx_data_rx_handler removeme

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/3f5c1c4f_7ee4a937 PS3, Line 246: trxcon_phyif_handle_clock_ind removeme

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/d43344cf_b38b007c PS3, Line 304: case trxcon::TRXCON_PHYIF_CMDT_RESET Better use value-string for that. Ideally such a value string should be part of libtrxcon, so it's always in sync with the TRXCON_PHYIF_CMDT_* values.

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/2faadf5a_771fc6f7 PS3, Line 352: set_ta Shouldn't this be done on receipt of TRXCON_PHYIF_CMDT_RESET instead?

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/86b41add_7e99a70e PS3, Line 357: dbm = -80 please add a FIXME or TODO, so it's clear that the measurement logic is missing

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/3d17174c_41e5bdcd PS3, Line 364: gsm_arfcn2freq10 careful, this function may return 0xffff

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/76523f1d_1714766d PS3, Line 385: assert OSMO_ASSERT?

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/32cb86c8_4b13670b PS3, Line 425: trxcon it's not trxcon app anymore

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/46c73659_9bc25e02 PS3, Line 429: 3 pass 0 to avoid confusion, we're not using fn_advance here

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/17f43833_0f042a41 PS3, Line 430: gsmtap = 0; gsmtap is a pointer, so = NULL

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/f9efc989_1a4b0ef7 PS3, Line 431: 0x1234 What's this? Why not NULL?

https://gerrit.osmocom.org/c/osmo-trx/+/30416/comment/31d216f3_ce23d1ec PS3, Line 433: trxc I would avoid using 'trxc' here to avoid confusion with the TRXC protocol.