measurement: remove unecessary is_amr_sid_update parameter

The function ts45008_83_is_sub rougly decides if a frame is a SUB frame
or not. This works by checking the frame number against against lookup
tables. This works fine for codecs where the occurrence of SUB frames is
fixed. However for AMR this is not the case as the DTX periods are
dynamic. Here it is the responsibility of the lower layers (phy,
frame decoding) to tag SUB frames early since making the decision later
based on the frame number is not possible.

The parameter is_amr_sid_update was probably added as a placeholder. It
is set to falls by the callers of the function. Lets remove this
parameter as a late decision if an AMR frame is a SUB frame will never
work.

Change-Id: I125d5ff592218a9e98130a6a7b6bbc6378ce4132
Related: OS#2978

3 files changed, 4 insertions, 8 deletions

diff --git a/include/osmo-bts/measurement.h b/include/osmo-bts/measurement.h
index 4f04ffa2..8e91c334 100644
--- a/include/osmo-bts/measurement.h
+++ b/include/osmo-bts/measurement.h
@@ -12,7 +12,7 @@ int lchan_meas_process_measurement(struct gsm_lchan *lchan, struct bts_ul_meas *
 
 void lchan_meas_reset(struct gsm_lchan *lchan);
 
-bool ts45008_83_is_sub(struct gsm_lchan *lchan, uint32_t fn, bool is_amr_sid_update);
+bool ts45008_83_is_sub(struct gsm_lchan *lchan, uint32_t fn);
 
 int is_meas_complete(struct gsm_lchan *lchan, uint32_t fn);
 
diff --git a/src/common/measurement.c b/src/common/measurement.c
index b883d51f..c5cbbb04 100644
--- a/src/common/measurement.c
+++ b/src/common/measurement.c
@@ -45,7 +45,7 @@ static bool array_contains(const uint8_t *arr, unsigned int len, uint8_t val) {
 
 /* Decide if a given frame number is part of the "-SUB" measurements (true) or not (false)
  * (this function is only used internally, it is public to call it from unit-tests) */
-bool ts45008_83_is_sub(struct gsm_lchan *lchan, uint32_t fn, bool is_amr_sid_update)
+bool ts45008_83_is_sub(struct gsm_lchan *lchan, uint32_t fn)
 {
 	uint32_t fn104 = fn % 104;
 
@@ -66,8 +66,6 @@ bool ts45008_83_is_sub(struct gsm_lchan *lchan, uint32_t fn, bool is_amr_sid_upd
 		case GSM48_CMODE_SPEECH_AMR:
 			if (trx_sched_is_sacch_fn(lchan->ts, fn, true))
 				return true;
-			if (is_amr_sid_update)
-				return true;
 			break;
 		default:
 			LOGPFN(DMEAS, LOGL_ERROR, fn, "%s: Unsupported lchan->tch_mode %u\n",
@@ -96,8 +94,6 @@ bool ts45008_83_is_sub(struct gsm_lchan *lchan, uint32_t fn, bool is_amr_sid_upd
 		case GSM48_CMODE_SPEECH_AMR:
 			if (trx_sched_is_sacch_fn(lchan->ts, fn, true))
 				return true;
-			if (is_amr_sid_update)
-				return true;
 			break;
 		case GSM48_CMODE_SIGN:
 			/* No DTX allowed; SUB=FULL, therefore measurements at all frame numbers are
@@ -343,7 +339,7 @@ int lchan_new_ul_meas(struct gsm_lchan *lchan, struct bts_ul_meas *ulm, uint32_t
 	/* We expect the lower layers to mark AMR SID_UPDATE frames already as such.
 	 * In this function, we only deal with the common logic as per the TS 45.008 tables */
 	if (!ulm->is_sub)
-		ulm->is_sub = ts45008_83_is_sub(lchan, fn, false);
+		ulm->is_sub = ts45008_83_is_sub(lchan, fn);
 
 	DEBUGPFN(DMEAS, fn, "%s adding measurement (is_sub=%u), num_ul_meas=%d, fn_mod=%u\n",
 		 gsm_lchan_name(lchan), ulm->is_sub, lchan->meas.num_ul_meas, fn_mod);
diff --git a/tests/meas/meas_test.c b/tests/meas/meas_test.c
index c8f06b64..f414b60b 100644
--- a/tests/meas/meas_test.c
+++ b/tests/meas/meas_test.c
@@ -469,7 +469,7 @@ static void test_ts45008_83_is_sub_single(uint8_t ts, uint8_t ss, bool fr)
 	/* Walk trough the first 100 intervals and check for unexpected
 	 * results (false positive and false negative) */
 	for (i = 0; i < 104 * 100; i++) {
-		rc = ts45008_83_is_sub(lchan, i, false);
+		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(i, ss)) {