diff mbox

[2/3] amr: Instead of putting ms/bts into the same struct.. use it twice

Message ID 1443090576-63058-2-git-send-email-holger@freyther.de
State Accepted
Headers show

Commit Message

Holger Freyther Sept. 24, 2015, 10:29 a.m. UTC 
From: Holger Hans Peter Freyther <holger@moiji-mobile.com>

This way a lot of if/else can just be killed by the caller deciding
which of the two instances to use.

I have copied both branches to new files, replace bts for ms in one
of them and ran diff on it. There is no difference.
---
 openbsc/include/openbsc/gsm_04_08.h       |  3 ++-
 openbsc/include/openbsc/gsm_data_shared.h | 10 ++++----
 openbsc/src/libbsc/bsc_api.c              |  4 +--
 openbsc/src/libbsc/bsc_vty.c              | 16 ++++++------
 openbsc/src/libbsc/gsm_04_08_utils.c      | 42 +++++++++++--------------------
 5 files changed, 31 insertions(+), 44 deletions(-)

Comments

Holger Freyther Sept. 24, 2015, 2:36 p.m. UTC | #1 
> On 24 Sep 2015, at 12:29, Holger Hans Peter Freyther <holger@freyther.de> wrote:
> 
> From: Holger Hans Peter Freyther <holger@moiji-mobile.com>
> 
> This way a lot of if/else can just be killed by the caller deciding
> which of the two instances to use.


We should be able to  shrink the amount of VTY commands down to one by making
all the other modes optional (by adding ‘(‘ ‘)’) around these options. This would reduce
the amount of copy and paste for hysteris and threshold too.

Any volunteers?

holger
diff mbox

Patch

diff --git a/openbsc/include/openbsc/gsm_04_08.h b/openbsc/include/openbsc/gsm_04_08.h
index 02b2e3b..02d67f7 100644
--- a/openbsc/include/openbsc/gsm_04_08.h
+++ b/openbsc/include/openbsc/gsm_04_08.h
@@ -14,6 +14,7 @@  struct gsm_network;
 struct gsm_trans;
 struct gsm_subscriber_connection;
 struct amr_multirate_conf;
+struct amr_mode;
 
 #define GSM48_ALLOC_SIZE	2048
 #define GSM48_ALLOC_HEADROOM	256
@@ -90,6 +91,6 @@  void gsm48_lchan2chan_desc(struct gsm48_chan_desc *cd,
 void release_security_operation(struct gsm_subscriber_connection *conn);
 void allocate_security_operation(struct gsm_subscriber_connection *conn);
 
-int gsm48_multirate_config(uint8_t *lv, struct amr_multirate_conf *mr, int ms);
+int gsm48_multirate_config(uint8_t *lv, struct amr_multirate_conf *mr, struct amr_mode *modes);
 
 #endif
diff --git a/openbsc/include/openbsc/gsm_data_shared.h b/openbsc/include/openbsc/gsm_data_shared.h
index 5ff6c20..be3333c 100644
--- a/openbsc/include/openbsc/gsm_data_shared.h
+++ b/openbsc/include/openbsc/gsm_data_shared.h
@@ -150,14 +150,14 @@  struct bts_codec_conf {
 
 struct amr_mode {
 	uint8_t mode;
-	uint8_t threshold_ms;
-	uint8_t hysteresis_ms;
-	uint8_t threshold_bts;
-	uint8_t hysteresis_bts;
+	uint8_t threshold;
+	uint8_t hysteresis;
 };
+
 struct amr_multirate_conf {
 	uint8_t gsm48_ie[2];
-	struct amr_mode mode[4];
+	struct amr_mode ms_mode[4];
+	struct amr_mode bts_mode[4];
 	uint8_t num_modes;
 };
 /* /BTS ONLY */
diff --git a/openbsc/src/libbsc/bsc_api.c b/openbsc/src/libbsc/bsc_api.c
index 78e7c87..e157eb9 100644
--- a/openbsc/src/libbsc/bsc_api.c
+++ b/openbsc/src/libbsc/bsc_api.c
@@ -178,8 +178,8 @@  static void handle_mr_config(struct gsm_subscriber_connection *conn,
 		mr_conf->icmi = 1;
 		mr_conf->m5_90 = 1;
 	}
-	gsm48_multirate_config(lchan->mr_ms_lv, mr, 1);
-	gsm48_multirate_config(lchan->mr_bts_lv, mr, 0);
+	gsm48_multirate_config(lchan->mr_ms_lv, mr, mr->ms_mode);
+	gsm48_multirate_config(lchan->mr_bts_lv, mr, mr->bts_mode);
 }
 
 /*
diff --git a/openbsc/src/libbsc/bsc_vty.c b/openbsc/src/libbsc/bsc_vty.c
index 9b0f020..d940624 100644
--- a/openbsc/src/libbsc/bsc_vty.c
+++ b/openbsc/src/libbsc/bsc_vty.c
@@ -505,22 +505,22 @@  static void config_write_bts_amr(struct vty *vty, struct gsm_bts *bts,
 	if (num > 1) {
 		vty_out(vty, "  %s threshold ms", prefix);
 		for (i = 0; i < num - 1; i++) {
-			vty_out(vty, " %d", mr->mode[i].threshold_ms);
+			vty_out(vty, " %d", mr->ms_mode[i].threshold);
 		}
 		vty_out(vty, "%s", VTY_NEWLINE);
 		vty_out(vty, "  %s hysteresis ms", prefix);
 		for (i = 0; i < num - 1; i++) {
-			vty_out(vty, " %d", mr->mode[i].hysteresis_ms);
+			vty_out(vty, " %d", mr->ms_mode[i].hysteresis);
 		}
 		vty_out(vty, "%s", VTY_NEWLINE);
 		vty_out(vty, "  %s threshold bts", prefix);
 		for (i = 0; i < num - 1; i++) {
-			vty_out(vty, " %d", mr->mode[i].threshold_bts);
+			vty_out(vty, " %d", mr->bts_mode[i].threshold);
 		}
 		vty_out(vty, "%s", VTY_NEWLINE);
 		vty_out(vty, "  %s hysteresis bts", prefix);
 		for (i = 0; i < num - 1; i++) {
-			vty_out(vty, " %d", mr->mode[i].hysteresis_bts);
+			vty_out(vty, " %d", mr->bts_mode[i].hysteresis);
 		}
 		vty_out(vty, "%s", VTY_NEWLINE);
 	}
@@ -2957,10 +2957,10 @@  static void get_amr_th_from_arg(struct vty *vty, int argc, const char *argv[], i
 
 	if (argv[0][0]=='m') {
 		for (i = 0; i < argc - 1; i++)
-			mr->mode[i].threshold_ms = atoi(argv[i + 1]);
+			mr->ms_mode[i].threshold = atoi(argv[i + 1]);
 	} else {
 		for (i = 0; i < argc - 1; i++)
-			mr->mode[i].threshold_bts = atoi(argv[i + 1]);
+			mr->bts_mode[i].threshold = atoi(argv[i + 1]);
 	}
 }
 
@@ -2972,10 +2972,10 @@  static void get_amr_hy_from_arg(struct vty *vty, int argc, const char *argv[], i
 
 	if (argv[0][0]=='m') {
 		for (i = 0; i < argc - 1; i++)
-			mr->mode[i].hysteresis_ms = atoi(argv[i + 1]);
+			mr->ms_mode[i].hysteresis = atoi(argv[i + 1]);
 	} else {
 		for (i = 0; i < argc - 1; i++)
-			mr->mode[i].hysteresis_bts = atoi(argv[i + 1]);
+			mr->bts_mode[i].hysteresis = atoi(argv[i + 1]);
 	}
 }
 
diff --git a/openbsc/src/libbsc/gsm_04_08_utils.c b/openbsc/src/libbsc/gsm_04_08_utils.c
index 24a1cfd..4901912 100644
--- a/openbsc/src/libbsc/gsm_04_08_utils.c
+++ b/openbsc/src/libbsc/gsm_04_08_utils.c
@@ -364,7 +364,7 @@  void gsm48_lchan2chan_desc(struct gsm48_chan_desc *cd,
 	}
 }
 
-int gsm48_multirate_config(uint8_t *lv, struct amr_multirate_conf *mr, int ms)
+int gsm48_multirate_config(uint8_t *lv, struct amr_multirate_conf *mr, struct amr_mode *modes)
 {
 	int num = 0, i;
 
@@ -387,33 +387,19 @@  int gsm48_multirate_config(uint8_t *lv, struct amr_multirate_conf *mr, int ms)
 	memcpy(lv + 1, mr->gsm48_ie, 2);
 	if (num == 1)
 		return 0;
-	if (ms) {
-		lv[3] = mr->mode[0].threshold_ms & 0x3f;
-		lv[4] = mr->mode[0].hysteresis_ms << 4;
-		if (num == 2)
-			return 0;
-		lv[4] |= (mr->mode[1].threshold_ms & 0x3f) >> 2;
-		lv[5] = mr->mode[1].threshold_ms << 6;
-		lv[5] |= (mr->mode[1].hysteresis_ms & 0x0f) << 2;
-		if (num == 3)
-			return 0;
-		lv[5] |= (mr->mode[2].threshold_ms & 0x3f) >> 4;
-		lv[6] = mr->mode[2].threshold_ms << 4;
-		lv[6] |= mr->mode[2].hysteresis_ms & 0x0f;
-	} else {
-		lv[3] = mr->mode[0].threshold_bts & 0x3f;
-		lv[4] = mr->mode[0].hysteresis_bts << 4;
-		if (num == 2)
-			return 0;
-		lv[4] |= (mr->mode[1].threshold_bts & 0x3f) >> 2;
-		lv[5] = mr->mode[1].threshold_bts << 6;
-		lv[5] |= (mr->mode[1].hysteresis_bts & 0x0f) << 2;
-		if (num == 3)
-			return 0;
-		lv[5] |= (mr->mode[2].threshold_bts & 0x3f) >> 4;
-		lv[6] = mr->mode[2].threshold_bts << 4;
-		lv[6] |= mr->mode[2].hysteresis_bts & 0x0f;
-	}
+
+	lv[3] = modes[0].threshold & 0x3f;
+	lv[4] = modes[0].hysteresis << 4;
+	if (num == 2)
+		return 0;
+	lv[4] |= (modes[1].threshold & 0x3f) >> 2;
+	lv[5] = modes[1].threshold << 6;
+	lv[5] |= (modes[1].hysteresis & 0x0f) << 2;
+	if (num == 3)
+		return 0;
+	lv[5] |= (modes[2].threshold & 0x3f) >> 4;
+	lv[6] = modes[2].threshold << 4;
+	lv[6] |= modes[2].hysteresis & 0x0f;
 
 	return 0;
 }