Attention is currently required from: falconia.
fixeria has posted comments on this change by falconia. ( https://gerrit.osmocom.org/c/osmo-bts/+/38553?usp=email )
Change subject: CSD: implement half-rate modes correctly ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
File src/common/l1sap.c:
https://gerrit.osmocom.org/c/osmo-bts/+/38553/comment/2418faa1_c0953808?usp=... : PS2, Line 2098: switch (lchan->tch_mode) {
the only valid modes are 6.0k and 3.6k raw rates, and my code does OSMO_ASSERT on all others. Changing the code to use csd_v110_lchan_desc[] would entail allowing invalid modes [...]
The point of using `csd_v110_lchan_desc[]` is to avoid using magic numbers, which as we have already seen [in the case of 26] raising questions like "where it's coming from?". You can keep the switch statement if really want to be 100% sure that we were given the right lchan kind:
``` const struct csd_v110_lchan_desc *desc;
switch (lchan->tch_mode) { case GSM48_CMODE_DATA_6k0: case GSM48_CMODE_DATA_3k6: desc = csd_v110_lchan_desc[lchan->tch_mode]; bits_per_20ms = desc->num_blocks * desc->num_bits; break; default: OSMO_ASSERT(0); } ```
The reason why I am suggesting to move `csd_v110_lchan_desc[]` to libosmocore.git is that we also want to use those definitions in osmocom-bb.git/trxcon (we do have a similar lookup table in there). This is of course not a blocker and can be done later separately from this patch.
-
fixeria