diff options
authorPhilipp Maier <pmaier@sysmocom.de>2020-04-22 12:41:39 +0200
committerlaforge <laforge@osmocom.org>2020-04-25 14:03:06 +0000
commit78a1d3a31a568731f3c253743f0426f51b2df200 (patch)
parent58b9f1189843a91f53bce76b3afa9483f89440a0 (diff)
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
if (trx_sched_is_sacch_fn(lchan->ts, fn, true))
return true;
- if (is_amr_sid_update)
- return true;
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
if (trx_sched_is_sacch_fn(lchan->ts, fn, true))
return true;
- if (is_amr_sid_update)
- return true;
/* 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)) {