Change in osmo-bts[master]: measurement: pass *mr to lchan_bs_pwr_ctrl()

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

fixeria gerrit-no-reply at lists.osmocom.org
Thu Nov 4 13:47:17 UTC 2021


fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/26049 )

Change subject: measurement: pass *mr to lchan_bs_pwr_ctrl()
......................................................................

measurement: pass *mr to lchan_bs_pwr_ctrl()

As a side effect, we have to sacrifice a unit test (TC_inval_dummy)
because it becomes impossible to pass a dummy or invalid SACCH block
to lchan_bs_pwr_ctrl().

Change-Id: I937117cf26fb718d57920382f6972390ad498c51
Related: SYS#4918
---
M include/osmo-bts/power_control.h
M src/common/measurement.c
M src/common/power_control.c
M tests/power/bs_power_loop_test.c
M tests/power/bs_power_loop_test.err
M tests/power/bs_power_loop_test.ok
6 files changed, 19 insertions(+), 112 deletions(-)

Approvals:
  Jenkins Builder: Verified
  pespin: Looks good to me, approved
  laforge: Looks good to me, but someone else must approve



diff --git a/include/osmo-bts/power_control.h b/include/osmo-bts/power_control.h
index e8b035d..0764ba7 100644
--- a/include/osmo-bts/power_control.h
+++ b/include/osmo-bts/power_control.h
@@ -87,4 +87,4 @@
 		      const int16_t ul_lqual_cb);
 
 int lchan_bs_pwr_ctrl(struct gsm_lchan *lchan,
-		      const struct gsm48_hdr *gh);
+		      const struct gsm48_meas_res *mr);
diff --git a/src/common/measurement.c b/src/common/measurement.c
index c5d60b2..8f33eaa 100644
--- a/src/common/measurement.c
+++ b/src/common/measurement.c
@@ -954,8 +954,8 @@
 	}
 	lchan_ms_ta_ctrl(lchan, ms_ta, lchan->meas.ms_toa256);
 	lchan_ms_pwr_ctrl(lchan, ms_pwr, ul_rssi, ul_ci_cb);
-	if (gh)
-		lchan_bs_pwr_ctrl(lchan, gh);
+	if (mr && mr->meas_valid == 0) /* 0 = valid */
+		lchan_bs_pwr_ctrl(lchan, mr);
 
 	repeated_dl_facch_active_decision(lchan, mr);
 
diff --git a/src/common/power_control.c b/src/common/power_control.c
index a81000e..42d9d07 100644
--- a/src/common/power_control.c
+++ b/src/common/power_control.c
@@ -311,46 +311,26 @@
 
 /*! compute the new Downlink attenuation value for the given logical channel.
  *  \param lchan logical channel for which to compute (and in which to store) new power value.
- *  \param[in] gh pointer to the beginning of (presumably) a Measurement Report.
+ *  \param[in] mr pointer to a *valid* Measurement Report.
  */
 int lchan_bs_pwr_ctrl(struct gsm_lchan *lchan,
-		      const struct gsm48_hdr *gh)
+		      const struct gsm48_meas_res *mr)
 {
 	struct lchan_power_ctrl_state *state = &lchan->bs_power_ctrl;
 	const struct gsm_power_ctrl_params *params = state->dpc_params;
-	uint8_t rxqual_full, rxqual_sub;
-	uint8_t rxlev_full, rxlev_sub;
 	uint8_t rxqual, rxqual_avg, rxlev, rxlev_avg;
 	int new_att;
 
 	/* Check if dynamic BS Power Control is enabled */
 	if (params == NULL)
 		return 0;
-	/* Check if this is a Measurement Report */
-	if (gh->proto_discr != GSM48_PDISC_RR)
-		return 0;
-	if (gh->msg_type != GSM48_MT_RR_MEAS_REP)
-		return 0;
-
-	/* Check if the measurement results are valid */
-	if ((gh->data[1] & 0x40) == 0x40) {
-		LOGPLCHAN(lchan, DLOOP, LOGL_DEBUG,
-			  "The measurement results are not valid\n");
-		return 0;
-	}
-
-	/* See 3GPP TS 44.018, section 10.5.2.20 */
-	rxqual_full = (gh->data[2] >> 4) & 0x7;
-	rxqual_sub = (gh->data[2] >> 1) & 0x7;
-
-	rxlev_full = gh->data[0] & 0x3f;
-	rxlev_sub = gh->data[1] & 0x3f;
 
 	LOGPLCHAN(lchan, DLOOP, LOGL_DEBUG, "Rx DL Measurement Report: "
 		  "RXLEV-FULL(%02u), RXQUAL-FULL(%u), "
 		  "RXLEV-SUB(%02u), RXQUAL-SUB(%u), "
 		  "DTx is %s => using %s\n",
-		  rxlev_full, rxqual_full, rxlev_sub, rxqual_sub,
+		  mr->rxlev_full, mr->rxqual_full,
+		  mr->rxlev_sub, mr->rxqual_sub,
 		  lchan->tch.dtx.dl_active ? "enabled" : "disabled",
 		  lchan->tch.dtx.dl_active ? "SUB" : "FULL");
 
@@ -360,11 +340,11 @@
 
 	/* If DTx is active on Downlink, use the '-SUB' */
 	if (lchan->tch.dtx.dl_active) {
-		rxqual = rxqual_sub;
-		rxlev = rxlev_sub;
+		rxqual = mr->rxqual_sub;
+		rxlev = mr->rxlev_sub;
 	} else { /* ... otherwise use the '-FULL' */
-		rxqual = rxqual_full;
-		rxlev = rxlev_full;
+		rxqual = mr->rxqual_full;
+		rxlev = mr->rxlev_full;
 	}
 
 	rxlev_avg = do_avg_algo(&params->rxlev_meas, &state->rxlev_meas_proc, rxlev);
diff --git a/tests/power/bs_power_loop_test.c b/tests/power/bs_power_loop_test.c
index 6b67ba2..06fe3ed 100644
--- a/tests/power/bs_power_loop_test.c
+++ b/tests/power/bs_power_loop_test.c
@@ -47,14 +47,8 @@
 	{ DL_MEAS_FULL(rxqual, rxlev), \
 	  DL_MEAS_SUB(rxqual, rxlev) }
 
-#define DL_MEAS_FULL_SUB_INV(rxqual, rxlev) \
-	{ DL_MEAS_FULL(rxqual, rxlev), \
-	  DL_MEAS_SUB(rxqual, rxlev), \
-	  .invalid = true }
-
 enum power_test_step_type {
 	PWR_TEST_ST_IND_MEAS = 0,
-	PWR_TEST_ST_IND_DUMMY,
 	PWR_TEST_ST_SET_STATE,
 	PWR_TEST_ST_SET_CTRL_INTERVAL,
 	PWR_TEST_ST_SET_STEP_SIZE,
@@ -78,7 +72,6 @@
 			uint8_t rxqual_sub;
 			uint8_t rxlev_full;
 			uint8_t rxlev_sub;
-			bool invalid;
 		} meas;
 		/* Increase / reduce step size */
 		struct {
@@ -123,28 +116,21 @@
 	printf("\nStarting test case '%s'\n", name);
 }
 
-static void enc_meas_rep(struct gsm48_hdr *gh,
+static void enc_meas_rep(struct gsm48_meas_res *mr,
 			 const unsigned int n,
 			 const struct power_test_step *step)
 {
-	struct gsm48_meas_res *mr = (struct gsm48_meas_res *) gh->data;
-
-	gh->proto_discr = GSM48_PDISC_RR;
-	gh->msg_type = GSM48_MT_RR_MEAS_REP;
-
 	*mr = (struct gsm48_meas_res) {
 		.rxlev_full = step->meas.rxlev_full,
 		.rxlev_sub = step->meas.rxlev_sub,
 		.rxqual_full = step->meas.rxqual_full,
 		.rxqual_sub = step->meas.rxqual_sub,
-		/* NOTE: inversed logic (1 means invalid) */
-		.meas_valid = step->meas.invalid,
 	};
 
-	printf("#%02u %s() -> Measurement Results (%svalid): "
+	printf("#%02u %s() -> Measurement Results (valid): "
 	       "RXLEV-FULL(%02u), RXQUAL-FULL(%u), "
 	       "RXLEV-SUB(%02u), RXQUAL-SUB(%u)\n",
-	       n, __func__, step->meas.invalid ? "in" : "",
+	       n, __func__,
 	       mr->rxlev_full, mr->rxqual_full,
 	       mr->rxlev_sub, mr->rxqual_sub);
 }
@@ -153,11 +139,8 @@
 			   const unsigned int n,
 			   const struct power_test_step *step)
 {
-	struct gsm48_hdr *gh;
+	struct gsm48_meas_res mr;
 	uint8_t old, new;
-	uint8_t buf[18];
-
-	gh = (struct gsm48_hdr *) buf;
 
 	switch (step->type) {
 	case PWR_TEST_ST_SET_STATE:
@@ -192,20 +175,16 @@
 		printf("#%02u %s() <- Enable DTXd\n", n, __func__);
 		lchan->tch.dtx.dl_active = true;
 		return 0; /* we're done */
-	case PWR_TEST_ST_IND_DUMMY:
-		printf("#%02u %s() <- Dummy block\n", n, __func__);
-		memset(buf, 0x2b, sizeof(buf));
-		break;
 	case PWR_TEST_ST_IND_MEAS:
-		enc_meas_rep(gh, n, step);
+		enc_meas_rep(&mr, n, step);
 		break;
 	}
 
-	printf("#%02u lchan_bs_pwr_ctrl() <- UL SACCH: %s\n",
-	       n, osmo_hexdump(buf, sizeof(buf)));
+	printf("#%02u lchan_bs_pwr_ctrl() <- UL SACCH: 06 15 %s\n",
+	       n, osmo_hexdump((void *)&mr, sizeof(mr)));
 
 	old = lchan->bs_power_ctrl.current;
-	lchan_bs_pwr_ctrl(lchan, gh);
+	lchan_bs_pwr_ctrl(lchan, &mr);
 	new = lchan->bs_power_ctrl.current;
 
 	printf("#%02u lchan_bs_pwr_ctrl() -> BS power reduction: "
@@ -406,24 +385,6 @@
 	{ .meas = DL_MEAS_FULL_SUB(7, PWR_TEST_RXLEV_TARGET) }, /* max */
 };
 
-/* Verify that invalid and dummy SACCH blocks are ignored. */
-static const struct power_test_step TC_inval_dummy[] = {
-	/* Initial state: 16 dB, up to 20 dB */
-	{ .type = PWR_TEST_ST_SET_STATE,
-	  .state = { .current = 16, .max = 2 * 10 } },
-
-	/* MS sends invalid measurement results which must be ignored */
-	{ .meas = DL_MEAS_FULL_SUB_INV(7, 63),			.exp_txred = 16 },
-	{ .meas = DL_MEAS_FULL_SUB_INV(0, 0),			.exp_txred = 16 },
-
-	/* Let's say SMS (SAPI=3) blocks substitute some of the reports */
-	{ .meas = DL_MEAS_FULL_SUB(0, PWR_TEST_RXLEV_TARGET),	.exp_txred = 16 },
-	{ .type = PWR_TEST_ST_IND_DUMMY, /* not a report */	.exp_txred = 16 },
-	{ .meas = DL_MEAS_FULL_SUB(0, PWR_TEST_RXLEV_TARGET),	.exp_txred = 16 },
-	{ .type = PWR_TEST_ST_IND_DUMMY, /* not a report */	.exp_txred = 16 },
-	{ .meas = DL_MEAS_FULL_SUB(0, PWR_TEST_RXLEV_TARGET),	.exp_txred = 16 },
-};
-
 /* Verify handling of optional power control interval (P_Con_INTERVAL). */
 static const struct power_test_step TC_ctrl_interval[] = {
 	/* Initial state: 0 dB, up to 20 dB */
@@ -545,7 +506,6 @@
 
 	exec_test(TC_dtxd_mode);
 	exec_test(TC_rxqual_ber);
-	exec_test(TC_inval_dummy);
 	exec_test(TC_ctrl_interval);
 
 	exec_test(TC_rxlev_hyst);
diff --git a/tests/power/bs_power_loop_test.err b/tests/power/bs_power_loop_test.err
index dc4f411..24107ba 100644
--- a/tests/power/bs_power_loop_test.err
+++ b/tests/power/bs_power_loop_test.err
@@ -130,14 +130,6 @@
 (bts=0,trx=0,ts=0,ss=0) Keeping DL attenuation at 0 dB: max 20 dB, RSSI[curr -80, avg -80, thresh -80..-80] dBm, RxQual[curr 7, avg 7, thresh 3..0]
 (bts=0,trx=0,ts=0,ss=0) Rx DL Measurement Report: RXLEV-FULL(30), RXQUAL-FULL(7), RXLEV-SUB(30), RXQUAL-SUB(7), DTx is disabled => using FULL
 (bts=0,trx=0,ts=0,ss=0) Keeping DL attenuation at 0 dB: max 20 dB, RSSI[curr -80, avg -80, thresh -80..-80] dBm, RxQual[curr 7, avg 7, thresh 3..0]
-(bts=0,trx=0,ts=0,ss=0) The measurement results are not valid
-(bts=0,trx=0,ts=0,ss=0) The measurement results are not valid
-(bts=0,trx=0,ts=0,ss=0) Rx DL Measurement Report: RXLEV-FULL(30), RXQUAL-FULL(0), RXLEV-SUB(30), RXQUAL-SUB(0), DTx is disabled => using FULL
-(bts=0,trx=0,ts=0,ss=0) Keeping DL attenuation at 16 dB: max 20 dB, RSSI[curr -80, avg -80, thresh -80..-80] dBm, RxQual[curr 0, avg 0, thresh 3..0]
-(bts=0,trx=0,ts=0,ss=0) Rx DL Measurement Report: RXLEV-FULL(30), RXQUAL-FULL(0), RXLEV-SUB(30), RXQUAL-SUB(0), DTx is disabled => using FULL
-(bts=0,trx=0,ts=0,ss=0) Keeping DL attenuation at 16 dB: max 20 dB, RSSI[curr -80, avg -80, thresh -80..-80] dBm, RxQual[curr 0, avg 0, thresh 3..0]
-(bts=0,trx=0,ts=0,ss=0) Rx DL Measurement Report: RXLEV-FULL(30), RXQUAL-FULL(0), RXLEV-SUB(30), RXQUAL-SUB(0), DTx is disabled => using FULL
-(bts=0,trx=0,ts=0,ss=0) Keeping DL attenuation at 16 dB: max 20 dB, RSSI[curr -80, avg -80, thresh -80..-80] dBm, RxQual[curr 0, avg 0, thresh 3..0]
 (bts=0,trx=0,ts=0,ss=0) Rx DL Measurement Report: RXLEV-FULL(60), RXQUAL-FULL(0), RXLEV-SUB(60), RXQUAL-SUB(0), DTx is disabled => using FULL
 (bts=0,trx=0,ts=0,ss=0) Raising DL attenuation 0 dB => 2 dB:max 20 dB, RSSI[curr -50, avg -50, thresh -80..-80] dBm, RxQual[curr 0, avg 0, thresh 3..0]
 (bts=0,trx=0,ts=0,ss=0) Rx DL Measurement Report: RXLEV-FULL(60), RXQUAL-FULL(0), RXLEV-SUB(60), RXQUAL-SUB(0), DTx is disabled => using FULL
diff --git a/tests/power/bs_power_loop_test.ok b/tests/power/bs_power_loop_test.ok
index 2c12392..18a94ba 100644
--- a/tests/power/bs_power_loop_test.ok
+++ b/tests/power/bs_power_loop_test.ok
@@ -243,31 +243,6 @@
 #16 lchan_bs_pwr_ctrl() -> BS power reduction: 0 -> 0 (expected 0)
 Test case verdict: SUCCESS
 
-Starting test case 'TC_inval_dummy'
-#00 exec_power_step() <- State (re)set (current 16 dB, max 20 dB)
-#01 enc_meas_rep() -> Measurement Results (invalid): RXLEV-FULL(63), RXQUAL-FULL(7), RXLEV-SUB(63), RXQUAL-SUB(7)
-#01 lchan_bs_pwr_ctrl() <- UL SACCH: 06 15 3f 7f 7e 00 00 00 00 00 00 00 00 00 00 00 00 00 
-#01 lchan_bs_pwr_ctrl() -> BS power reduction: 16 -> 16 (expected 16)
-#02 enc_meas_rep() -> Measurement Results (invalid): RXLEV-FULL(00), RXQUAL-FULL(0), RXLEV-SUB(00), RXQUAL-SUB(0)
-#02 lchan_bs_pwr_ctrl() <- UL SACCH: 06 15 00 40 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
-#02 lchan_bs_pwr_ctrl() -> BS power reduction: 16 -> 16 (expected 16)
-#03 enc_meas_rep() -> Measurement Results (valid): RXLEV-FULL(30), RXQUAL-FULL(0), RXLEV-SUB(30), RXQUAL-SUB(0)
-#03 lchan_bs_pwr_ctrl() <- UL SACCH: 06 15 1e 1e 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
-#03 lchan_bs_pwr_ctrl() -> BS power reduction: 16 -> 16 (expected 16)
-#04 exec_power_step() <- Dummy block
-#04 lchan_bs_pwr_ctrl() <- UL SACCH: 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
-#04 lchan_bs_pwr_ctrl() -> BS power reduction: 16 -> 16 (expected 16)
-#05 enc_meas_rep() -> Measurement Results (valid): RXLEV-FULL(30), RXQUAL-FULL(0), RXLEV-SUB(30), RXQUAL-SUB(0)
-#05 lchan_bs_pwr_ctrl() <- UL SACCH: 06 15 1e 1e 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
-#05 lchan_bs_pwr_ctrl() -> BS power reduction: 16 -> 16 (expected 16)
-#06 exec_power_step() <- Dummy block
-#06 lchan_bs_pwr_ctrl() <- UL SACCH: 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 2b 
-#06 lchan_bs_pwr_ctrl() -> BS power reduction: 16 -> 16 (expected 16)
-#07 enc_meas_rep() -> Measurement Results (valid): RXLEV-FULL(30), RXQUAL-FULL(0), RXLEV-SUB(30), RXQUAL-SUB(0)
-#07 lchan_bs_pwr_ctrl() <- UL SACCH: 06 15 1e 1e 00 00 00 00 00 00 00 00 00 00 00 00 00 00 
-#07 lchan_bs_pwr_ctrl() -> BS power reduction: 16 -> 16 (expected 16)
-Test case verdict: SUCCESS
-
 Starting test case 'TC_ctrl_interval'
 #00 exec_power_step() <- State (re)set (current 0 dB, max 20 dB)
 #01 exec_power_step() <- (Re)set power control interval: 0 -> 0

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/26049
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I937117cf26fb718d57920382f6972390ad498c51
Gerrit-Change-Number: 26049
Gerrit-PatchSet: 5
Gerrit-Owner: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20211104/8c7f51b9/attachment.htm>


More information about the gerrit-log mailing list