measurement: move SACCH detection to process_l1sap_meas_data()

SACCH detection can be simplified by checking the RSL Link ID in
process_l1sap_meas_data().  This eliminates the need to lookup
the multiframe position by calling trx_sched_is_sacch_fn(), which
definitely takes more CPU time than just L1SAP_IS_LINK_SACCH().

Calling trx_sched_is_sacch_fn() is still required for BTS models
reporting the measurements via PRIM_MPH_INFO (legacy way),
separately from the related Uplink blocks.

This patch can be summarized as follows:

* detect SACCH and set .is_sub=1 in process_l1sap_meas_data();
** for PRIM_MPH_INFO use trx_sched_is_sacch_fn();
** for PRIM_PH_DATA use L1SAP_IS_LINK_SACCH();
* do not call trx_sched_is_sacch_fn() from ts45008_83_is_sub();
* modify the unit test - test_ts45008_83_is_sub_single();
** remove test_ts45008_83_is_sub_is_sacch().

Change-Id: I507e96ee34ac0f8b7a2a6f16a8c7f92bc467df60
Related: SYS#5853

3 files changed, 16 insertions, 46 deletions

diff --git a/src/common/l1sap.c b/src/common/l1sap.c
index 8e71cd4f..b74fd5a8 100644
--- a/src/common/l1sap.c
+++ b/src/common/l1sap.c
@@ -697,6 +697,8 @@ static inline void set_ms_to_data(struct gsm_lchan *lchan, int16_t data, bool se
 	}
 }
 
+bool trx_sched_is_sacch_fn(const struct gsm_bts_trx_ts *ts, uint32_t fn, bool uplink);
+
 /* measurement information received from bts model */
 static void process_l1sap_meas_data(struct gsm_lchan *lchan,
 				    const struct osmo_phsap_prim *l1sap,
@@ -720,8 +722,10 @@ static void process_l1sap_meas_data(struct gsm_lchan *lchan,
 			.inv_rssi = info_meas_ind->inv_rssi,
 			.ber10k = info_meas_ind->ber10k,
 			.ci_cb = info_meas_ind->c_i_cb,
-			.is_sub = info_meas_ind->is_sub,
 		};
+		/* additionally treat SACCH frames (match by TDMA FN) as SUB frames */
+		if (info_meas_ind->is_sub || trx_sched_is_sacch_fn(lchan->ts, fn, true))
+			ulm.is_sub = 1;
 		break;
 	case PRIM_TCH:
 		ph_tch_ind = &l1sap->u.tch;
@@ -736,6 +740,7 @@ static void process_l1sap_meas_data(struct gsm_lchan *lchan,
 			.ci_cb = ph_tch_ind->lqual_cb,
 			.is_sub = ph_tch_ind->is_sub,
 		};
+		/* PRIM_TCH always carries DCCH, not SACCH */
 		break;
 	case PRIM_PH_DATA:
 		ph_data_ind = &l1sap->u.data;
@@ -748,8 +753,10 @@ static void process_l1sap_meas_data(struct gsm_lchan *lchan,
 			.inv_rssi = abs(ph_data_ind->rssi),
 			.ber10k = ph_data_ind->ber10k,
 			.ci_cb = ph_data_ind->lqual_cb,
-			.is_sub = ph_data_ind->is_sub,
 		};
+		/* additionally treat SACCH frames (match by RSL link ID) as SUB frames */
+		if (ph_data_ind->is_sub || L1SAP_IS_LINK_SACCH(ph_data_ind->link_id))
+			ulm.is_sub = 1;
 		break;
 	default:
 		OSMO_ASSERT(false);
diff --git a/src/common/measurement.c b/src/common/measurement.c
index d1f99609..aa1f5ae0 100644
--- a/src/common/measurement.c
+++ b/src/common/measurement.c
@@ -56,20 +56,18 @@ bool ts45008_83_is_sub(struct gsm_lchan *lchan, uint32_t fn)
 	/* See TS 45.008 Sections 8.3 and 8.4 for a detailed descriptions of the rules
 	 * implemented here. We only implement the logic for Voice, not CSD */
 
+	/* AMR is special, SID frames may be scheduled dynamically at any time */
+	if (lchan->tch_mode == GSM48_CMODE_SPEECH_AMR)
+		return false;
+
 	switch (lchan->type) {
 	case GSM_LCHAN_TCH_F:
 		switch (lchan->tch_mode) {
 		case GSM48_CMODE_SPEECH_V1:
 		case GSM48_CMODE_SPEECH_EFR:
-			if (trx_sched_is_sacch_fn(lchan->ts, fn, true))
-				return true;
 			if (ARRAY_CONTAINS(ts45008_83_tch_f, fn104))
 				return true;
 			break;
-		case GSM48_CMODE_SPEECH_AMR:
-			if (trx_sched_is_sacch_fn(lchan->ts, fn, true))
-				return true;
-			break;
 		case GSM48_CMODE_SIGN:
 			/* No DTX allowed; SUB=FULL, therefore measurements at all frame numbers are
 			 * SUB */
@@ -83,8 +81,6 @@ bool ts45008_83_is_sub(struct gsm_lchan *lchan, uint32_t fn)
 	case GSM_LCHAN_TCH_H:
 		switch (lchan->tch_mode) {
 		case GSM48_CMODE_SPEECH_V1:
-			if (trx_sched_is_sacch_fn(lchan->ts, fn, true))
-				return true;
 			switch (lchan->nr) {
 			case 0:
 				if (ARRAY_CONTAINS(ts45008_83_tch_hs0, fn104))
@@ -98,10 +94,6 @@ bool ts45008_83_is_sub(struct gsm_lchan *lchan, uint32_t fn)
 				OSMO_ASSERT(0);
 			}
 			break;
-		case GSM48_CMODE_SPEECH_AMR:
-			if (trx_sched_is_sacch_fn(lchan->ts, fn, true))
-				return true;
-			break;
 		case GSM48_CMODE_SIGN:
 			/* No DTX allowed; SUB=FULL, therefore measurements at all frame numbers are
 			 * SUB */
diff --git a/tests/meas/meas_test.c b/tests/meas/meas_test.c
index 79ec3584..a3c514b8 100644
--- a/tests/meas/meas_test.c
+++ b/tests/meas/meas_test.c
@@ -373,28 +373,6 @@ void test_lchan_meas_process_measurement(bool no_sacch, bool dropouts)
 	}
 }
 
-static bool test_ts45008_83_is_sub_is_sacch(uint32_t fn)
-{
-	if (fn % 104 == 12)
-		return true;
-	if (fn % 104 == 25)
-		return true;
-	if (fn % 104 == 38)
-		return true;
-	if (fn % 104 == 51)
-		return true;
-	if (fn % 104 == 64)
-		return true;
-	if (fn % 104 == 77)
-		return true;
-	if (fn % 104 == 90)
-		return true;
-	if (fn % 104 == 103)
-		return true;
-
-	return false;
-}
-
 static bool test_ts45008_83_is_sub_is_sub(const struct gsm_lchan *lchan, uint32_t fn)
 {
 	fn = fn % 104;
@@ -473,16 +451,9 @@ static void test_ts45008_83_is_sub_single(uint8_t ts, uint8_t ss, bool fr)
 	 * results (false positive and false negative) */
 	for (i = 0; i < 104 * 100; i++) {
 		rc = ts45008_83_is_sub(lchan, i);
-		if (rc) {
-			if (!test_ts45008_83_is_sub_is_sacch(i)
-			    && !test_ts45008_83_is_sub_is_sub(lchan, i)) {
-				printf("  ==> Unexpected SUB frame at fn=%u\n", i);
-			}
-		} else {
-			if (test_ts45008_83_is_sub_is_sacch(i)
-			    && test_ts45008_83_is_sub_is_sub(lchan, i)) {
-				printf("  ==> Unexpected non-SUB frame at fn=%u\n", i);
-			}
+		if (rc != test_ts45008_83_is_sub_is_sub(lchan, i)) {
+			printf("  ==> ts45008_83_is_sub(fn=%u) yields %s, expected %s\n",
+			       i, rc ? "true" : "false", !rc ? "true" : "false");
 		}
 	}
 }