Change in osmo-bsc[master]: introduce gsm48_lchan_and_pchan2chan_desc()

neels gerrit-no-reply at lists.osmocom.org
Mon Aug 16 00:12:14 UTC 2021


neels has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/25163 )

Change subject: introduce gsm48_lchan_and_pchan2chan_desc()
......................................................................

introduce gsm48_lchan_and_pchan2chan_desc()

The function gsm48_lchan2chan_desc_as_configured() dups
gsm48_lchan2chan_desc() with merely a different pchan type
(ts->pchan_from_config instead of ts->pchan_is).

In an upcoming patch, I would like to do the same, just with yet another
pchan value (derived from lchan->type, because that reflects the channel
type even before a dynamic timeslot switched its pchan type).

So replace gsm48_lchan2chan_desc_as_configured() by
gsm48_lchan_and_pchan2chan_desc() with explicit pchan arg;
also call this from gsm48_lchan2chan_desc(), reducing code dup.

gsm48_lchan2chan_desc_as_configured() had more concise error logging.
Absorb that into the new gsm48_lchan_and_pchan2chan_desc().

Add gsm_lchan_and_pchan2chan_nr(), like gsm_lchan2chan_nr() just with
explicit pchan arg, to be able to pass the pchan down from the new
functions mentioned above.

Related: SYS#5559
Change-Id: I67f178c8160cdda1f2ab5513ac4f65c027d4012f
---
M include/osmocom/bsc/gsm_data.h
M src/osmo-bsc/gsm_data.c
M src/osmo-bsc/system_information.c
3 files changed, 29 insertions(+), 30 deletions(-)

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



diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index 5a20e72..31711c7 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -1075,12 +1075,15 @@
 int gsm_pchan2chan_nr(enum gsm_phys_chan_config pchan,
 		      uint8_t ts_nr, uint8_t lchan_nr, bool vamos_is_secondary);
 int gsm_lchan2chan_nr(const struct gsm_lchan *lchan, bool allow_osmo_cbits);
+int gsm_lchan_and_pchan2chan_nr(const struct gsm_lchan *lchan, enum gsm_phys_chan_config pchan, bool allow_osmo_cbits);
 
 int gsm48_lchan2chan_desc(struct gsm48_chan_desc *cd,
 			  const struct gsm_lchan *lchan,
 			  uint8_t tsc, bool allow_osmo_cbits);
-int gsm48_lchan2chan_desc_as_configured(struct gsm48_chan_desc *cd, const struct gsm_lchan *lchan,
-					uint8_t tsc);
+int gsm48_lchan_and_pchan2chan_desc(struct gsm48_chan_desc *cd,
+				    const struct gsm_lchan *lchan,
+				    enum gsm_phys_chan_config pchan,
+				    uint8_t tsc, bool allow_osmo_cbits);
 
 uint8_t gsm_ts_tsc(const struct gsm_bts_trx_ts *ts);
 
diff --git a/src/osmo-bsc/gsm_data.c b/src/osmo-bsc/gsm_data.c
index 4db70f0..e976a5b 100644
--- a/src/osmo-bsc/gsm_data.c
+++ b/src/osmo-bsc/gsm_data.c
@@ -562,7 +562,7 @@
 /* For RSL, to talk to osmo-bts, we introduce Osmocom specific channel number cbits to indicate VAMOS secondary lchans.
  * However, in RR, which is sent to the MS, these special cbits must not be sent, but their "normal" equivalent; for RR
  * messages, pass allow_osmo_cbits = false. */
-int gsm_lchan2chan_nr(const struct gsm_lchan *lchan, bool allow_osmo_cbits)
+int gsm_lchan_and_pchan2chan_nr(const struct gsm_lchan *lchan, enum gsm_phys_chan_config pchan, bool allow_osmo_cbits)
 {
 	int rc;
 	uint8_t lchan_nr = lchan->nr;
@@ -582,7 +582,7 @@
 	 * a primary ts->lchan[0] and a VAMOS ts->lchan[1]. Still, the VAMOS lchan should send chan_nr = 0. */
 	if (lchan->vamos.is_secondary)
 		lchan_nr -= lchan->ts->max_primary_lchans;
-	rc = gsm_pchan2chan_nr(lchan->ts->pchan_is, lchan->ts->nr, lchan_nr,
+	rc = gsm_pchan2chan_nr(pchan, lchan->ts->nr, lchan_nr,
 			       allow_osmo_cbits ? lchan->vamos.is_secondary : false);
 	/* Log an error so that we don't need to add logging to each caller of this function */
 	if (rc < 0)
@@ -593,6 +593,11 @@
 	return rc;
 }
 
+int gsm_lchan2chan_nr(const struct gsm_lchan *lchan, bool allow_osmo_cbits)
+{
+	return gsm_lchan_and_pchan2chan_nr(lchan, lchan->ts->pchan_is, allow_osmo_cbits);
+}
+
 static const uint8_t subslots_per_pchan[] = {
 	[GSM_PCHAN_NONE] = 0,
 	[GSM_PCHAN_CCCH] = 0,
@@ -712,14 +717,18 @@
 	}
 }
 
-int gsm48_lchan2chan_desc(struct gsm48_chan_desc *cd,
-			  const struct gsm_lchan *lchan,
-			  uint8_t tsc, bool allow_osmo_cbits)
+int gsm48_lchan_and_pchan2chan_desc(struct gsm48_chan_desc *cd,
+				    const struct gsm_lchan *lchan,
+				    enum gsm_phys_chan_config pchan,
+				    uint8_t tsc, bool allow_osmo_cbits)
 {
-	int chan_nr = gsm_lchan2chan_nr(lchan, allow_osmo_cbits);
+	int chan_nr = gsm_lchan_and_pchan2chan_nr(lchan, pchan, allow_osmo_cbits);
 	if (chan_nr < 0) {
 		/* Log an error so that we don't need to add logging to each caller of this function */
-		LOG_LCHAN(lchan, LOGL_ERROR, "Error encoding Channel Number\n");
+		LOG_LCHAN(lchan, LOGL_ERROR,
+			  "Error encoding Channel Number: pchan %s ts %u ss %u%s (rc = %d)\n",
+			  gsm_pchan_name(pchan), lchan->ts->nr, lchan->nr,
+			  lchan->vamos.is_secondary ? " (VAMOS shadow)" : "", chan_nr);
 		return chan_nr;
 	}
 	cd->chan_nr = chan_nr;
@@ -727,26 +736,11 @@
 	return 0;
 }
 
-/* like gsm48_lchan2chan_desc() above, but use ts->pchan_from_config to
- * return a channel description based on what is configured, rather than
- * what the current state of the pchan type is */
-int gsm48_lchan2chan_desc_as_configured(struct gsm48_chan_desc *cd,
-					const struct gsm_lchan *lchan,
-					uint8_t tsc)
+int gsm48_lchan2chan_desc(struct gsm48_chan_desc *cd,
+			  const struct gsm_lchan *lchan,
+			  uint8_t tsc, bool allow_osmo_cbits)
 {
-	int chan_nr = gsm_pchan2chan_nr(lchan->ts->pchan_from_config, lchan->ts->nr, lchan->nr,
-					lchan->vamos.is_secondary);
-	if (chan_nr < 0) {
-		/* Log an error so that we don't need to add logging to each caller of this function */
-		LOG_LCHAN(lchan, LOGL_ERROR,
-			  "Error encoding Channel Number: pchan %s ts %u ss %u%s (rc = %d)\n",
-			  gsm_pchan_name(lchan->ts->pchan_from_config), lchan->ts->nr, lchan->nr,
-			  lchan->vamos.is_secondary ? " (VAMOS shadow)" : "", chan_nr);
-		return chan_nr;
-	}
-	cd->chan_nr = chan_nr;
-	_chan_desc_fill_tail(cd, lchan, tsc);
-	return 0;
+	return gsm48_lchan_and_pchan2chan_desc(cd, lchan, lchan->ts->pchan_is, tsc, allow_osmo_cbits);
 }
 
 uint8_t gsm_ts_tsc(const struct gsm_bts_trx_ts *ts)
diff --git a/src/osmo-bsc/system_information.c b/src/osmo-bsc/system_information.c
index e8b3b7d..974af3a 100644
--- a/src/osmo-bsc/system_information.c
+++ b/src/osmo-bsc/system_information.c
@@ -1017,8 +1017,10 @@
 		const struct gsm_bts_trx_ts *ts = cbch_lchan->ts;
 		struct gsm48_chan_desc cd;
 
-		/* 10.5.2.5 (TV) CBCH Channel Description IE */
-		if (gsm48_lchan2chan_desc_as_configured(&cd, cbch_lchan, gsm_ts_tsc(cbch_lchan->ts)))
+		/* 10.5.2.5 (TV) CBCH Channel Description IE.
+		 * CBCH is never in VAMOS mode, so just pass allow_osmo_cbits == false. */
+		if (gsm48_lchan_and_pchan2chan_desc(&cd, cbch_lchan, cbch_lchan->ts->pchan_from_config,
+						    gsm_ts_tsc(cbch_lchan->ts), false))
 			return -EINVAL;
 		tail = tv_fixed_put(tail, GSM48_IE_CBCH_CHAN_DESC,
 				    sizeof(cd), (uint8_t *) &cd);

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

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I67f178c8160cdda1f2ab5513ac4f65c027d4012f
Gerrit-Change-Number: 25163
Gerrit-PatchSet: 2
Gerrit-Owner: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: osmith <osmith at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210816/87c77d59/attachment.htm>


More information about the gerrit-log mailing list