fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/27552 )

Change subject: osmo-bts-trx: rx_tchh_fn(): get rid of chan_state->meas_avg_facch ......................................................................

osmo-bts-trx: rx_tchh_fn(): get rid of chan_state->meas_avg_facch

FACCH/H takes out two speech frames, so we send two BFIs once we have received it in rx_tchh_fn(). The upper layers responsible for handling of the Uplink measurements expect a fixed amount of measurement samples, so currently we do:

* send a FACCH frame with the associated measurememnts (S6N6), * send the 1st BFI with invalidated measurements (RSSI=0), * send the 2nd BFI with the stored measurememnts of FACCH.

This is achieved by preserving a copy of the FACCH measurememnts in chan_state->meas_avg_facch and then using it two bursts later.

The same goal can be achieved a lot easier by sending the associated measurements with both BFIs as if no FACCH was received:

* send a FACCH frame with invalidated measurememnts (RSSI=0), * send the 1st BFI with the associated measurememnts (S6N4), * send the 2nd BFI with the associated measurememnts (S6N4).

This eliminates the need to store anything outside of the existing measurement history and simplifies the code a lot. Also, this eliminates the need for using a dedicated averaging mode S6N6.

Varified by running BTS_Tests.TC_meas_res_speech_tchh_facch.

Change-Id: I7902b4709bc3f418174e8373f52e87bb31cdc826 Related: I1ad9fa3815feb2b4da608ab7df716a87ba1f2f91 --- M include/osmo-bts/scheduler.h M src/osmo-bts-trx/sched_lchan_tchh.c 2 files changed, 7 insertions(+), 32 deletions(-)

Approvals: laforge: Looks good to me, but someone else must approve fixeria: Looks good to me, approved dexter: Looks good to me, but someone else must approve osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified

diff --git a/include/osmo-bts/scheduler.h b/include/osmo-bts/scheduler.h index 2806117..f671874 100644 --- a/include/osmo-bts/scheduler.h +++ b/include/osmo-bts/scheduler.h @@ -125,7 +125,6 @@ /* TCH/H */ uint8_t dl_ongoing_facch; /* FACCH/H on downlink */ uint8_t ul_ongoing_facch; /* FACCH/H on uplink */ - struct l1sched_meas_set meas_avg_facch; /* measurement results for last FACCH */

uint8_t dl_facch_bursts; /* number of remaining DL FACCH bursts */

diff --git a/src/osmo-bts-trx/sched_lchan_tchh.c b/src/osmo-bts-trx/sched_lchan_tchh.c index 4553132..e97adbd 100644 --- a/src/osmo-bts-trx/sched_lchan_tchh.c +++ b/src/osmo-bts-trx/sched_lchan_tchh.c @@ -71,7 +71,6 @@ uint16_t ber10k = 0; uint8_t is_sub = 0; uint8_t ft; - bool mask_stolen_tch_block = false; bool fn_is_cmi;

/* If handover RACH detection is turned on, treat this burst as an Access Burst. @@ -127,11 +126,7 @@ memcpy(*bursts_p + 232, *bursts_p + 464, 232); /* we have already sent the first BFI when a FACCH/H frame * was decoded (see below), now send the second one. */ - memset(&meas_avg, 0, sizeof(meas_avg)); - /* In order to provide an even stream of measurement reports - * we ask the code below to mask the missing TCH/H block - * measurement report with the FACCH measurement results. */ - mask_stolen_tch_block = true; + trx_sched_meas_avg(chan_state, &meas_avg, meas_avg_mode); goto bfi; }

@@ -248,8 +243,6 @@ ber10k = compute_ber10k(n_bits_total, n_errors);

/* average measurements of the last N (depends on mode) bursts */ - if (rc == GSM_MACBLOCK_LEN) - meas_avg_mode = SCHED_MEAS_AVG_M_S6N6; trx_sched_meas_avg(chan_state, &meas_avg, meas_avg_mode);

/* Check if the frame is bad */ @@ -277,22 +270,15 @@ fn_begin = gsm0502_fn_remap(bi->fn, FN_REMAP_FACCH_H0); else fn_begin = gsm0502_fn_remap(bi->fn, FN_REMAP_FACCH_H1); + /* In order to provide an even stream of measurement reports, here we + * intentionally invalidate RSSI, so that this report gets dropped in + * process_l1sap_meas_data(). The averaged results will still be sent + * with the first BFI (see below). */ _sched_compose_ph_data_ind(l1ts, fn_begin, bi->chan, tch_data + amr, GSM_MACBLOCK_LEN, - meas_avg.rssi, meas_avg.toa256, - meas_avg.ci_cb, ber10k, + 0, /* intentionally invalidate RSSI */ + meas_avg.toa256, meas_avg.ci_cb, ber10k, PRES_INFO_UNKNOWN); - - /* Keep a copy of the measurement results of the last FACCH - * transmission in order to be able to create a replacement - * measurement result for the one missing TCH block - * measurement */ - memcpy(&chan_state->meas_avg_facch, &meas_avg, sizeof(meas_avg)); - - /* Invalidate the current measurement result to prevent the - * code below from handing up the current measurement a second - * time. */ - memset(&meas_avg, 0, sizeof(meas_avg)); ber10k = 0; bfi: /* A FACCH/H frame replaces two speech frames, so we need to send two BFIs. @@ -363,16 +349,6 @@ else fn_begin = gsm0502_fn_remap(fn_tch_end, FN_REMAP_TCH_H1);

- /* A FACCH/H transmission takes out two TCH/H voice blocks and the - * related measurement results. The first measurement result is handed - * up directly with the FACCH (see above), the second one needs to be - * compensated by filling the gap with the measurement result we got - * from the FACCH transmission. */ - if (mask_stolen_tch_block) { - memcpy(&meas_avg, &chan_state->meas_avg_facch, sizeof(meas_avg)); - memset(&chan_state->meas_avg_facch, 0, sizeof(meas_avg)); - } - return _sched_compose_tch_ind(l1ts, fn_begin, bi->chan, tch_data, rc, /* FIXME: what should we use for BFI here? */ bfi_flag ? bi->toa256 : meas_avg.toa256, ber10k,