Attention is currently required from: laforge, osmith.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/35124?usp=email )
Change subject: Add GSMTAP encapsulation of RLP frames in CSD NT mode ......................................................................
Patch Set 6: Code-Review-1
(5 comments)
File src/common/l1sap.c:
https://gerrit.osmocom.org/c/osmo-bts/+/35124/comment/596bb9af_6b338676 PS6, Line 1854: if (!inst || !trx->bts->gsmtap.rlp) We also need to check `if (lchan->csd_mode == LCHAN_CSD_M_NT)`, otherwise we would be emitting garbage RLP frames during transparent calls. I saw this check in an early revision of this patch, but somehow it's gone?
https://gerrit.osmocom.org/c/osmo-bts/+/35124/comment/2382735b_2be5884e PS6, Line 1866: 2x Actually it's `4x36bit` for TCH/H2.4. Similarly to TCH/H4.8 carrying more data (per-block) than TCH/F4.8.
* TCH/F2.4: `2 * 36` bits every 20 ms, so `2 * 36 * (1000 / 20) = 3600` => 3.6 kbps * TCH/H2.4: `4 * 36` bits every 40 ms, so `4 * 36 * (1000 / 40) = 3600` => 3.6 kbps
https://gerrit.osmocom.org/c/osmo-bts/+/35124/comment/e05035e2_2a196cef PS6, Line 1867: 2x 290 bit IIRC, 290 is actually `8 * 36 + 2`.
https://gerrit.osmocom.org/c/osmo-bts/+/35124/comment/ae1a3861_11e2db86 PS6, Line 1867: TCH/H 14.4 TCH/F, not half-rate.
https://gerrit.osmocom.org/c/osmo-bts/+/35124/comment/a31d677b_cf7187f9 PS6, Line 1879: } else {
FYI if you think the UNNECESSARY_ELSE check is not something we need to follow for the Osmocom codin […]
I think it's fine to have it enabled in general. pylint is also enforcing the same rule for Python by default. The benefit of following this rule is reduced nesting. But I wish there was a way to say 'wontfix' for specific comments, rather than having to remove Jenkins and do CR+1 manually, or having to disable checks in the linter.