Attention is currently required from: laforge, osmith.

Patch set 6:Code-Review -1

View Change

5 comments:

• File src/common/l1sap.c:

  • Patch Set #6, 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?

  • Patch Set #6, 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

  • Patch Set #6, Line 1867: 2x 290 bit

    IIRC, 290 is actually `8 * 36 + 2`.

  • Patch Set #6, Line 1867: TCH/H 14.4

    TCH/F, not half-rate.

  • Patch Set #6, 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.

To view, visit change 35124. To unsubscribe, or for help writing mail filters, visit settings.