fixeria has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-bts/+/28085 )
Change subject: measurement: move SACCH detection to process_l1sap_meas_data()
......................................................................
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
---
M src/common/l1sap.c
M src/common/measurement.c
M tests/meas/meas_test.c
3 files changed, 16 insertions(+), 44 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/85/28085/1
diff --git a/src/common/l1sap.c b/src/common/l1sap.c
index 8e71cd4..2c5a141 100644
--- a/src/common/l1sap.c
+++ b/src/common/l1sap.c
@@ -697,6 +697,8 @@
}
}
+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,
@@ -722,6 +724,9 @@
.ci_cb = info_meas_ind->c_i_cb,
.is_sub = info_meas_ind->is_sub,
};
+ /* treat SACCH frames (match by TDMA FN) as SUB frames */
+ if (!ulm.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 +741,7 @@
.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;
@@ -750,6 +756,9 @@
.ci_cb = ph_data_ind->lqual_cb,
.is_sub = ph_data_ind->is_sub,
};
+ /* treat SACCH frames (match by RSL link ID) as SUB frames */
+ if (!ulm.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 d1f9960..aa1f5ae 100644
--- a/src/common/measurement.c
+++ b/src/common/measurement.c
@@ -56,20 +56,18 @@
/* 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 @@
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 @@
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 79ec358..a3c514b 100644
--- a/tests/meas/meas_test.c
+++ b/tests/meas/meas_test.c
@@ -373,28 +373,6 @@
}
}
-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 @@
* 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");
}
}
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/28085
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I507e96ee34ac0f8b7a2a6f16a8c7f92bc467df60
Gerrit-Change-Number: 28085
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <[email protected]>
Gerrit-MessageType: newchange
