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 &lt;vyanitskiy(a)sysmocom.de&gt; Gerrit-MessageType: newchange