[MERGED] osmo-bts[master]: all models: fix vty write: bts_model_config_write_phy

[Ivan Kluchnikov]

Ivan Kluchnikov has submitted this change and it was merged.

Change subject: all models: fix vty write: bts_model_config_write_phy
......................................................................


all models: fix vty write: bts_model_config_write_phy

The vty write for phy/inst is broken, leading to a written-out config being
unparsable; fix all of these:

- in common/vty.c, actually call bts_model_config_write_phy_inst().

- in sysmo and lc15 write the phy instance elements in
  bts_model_config_write_phy_inst() and not in bts_model_config_write_phy(),
  which lead to writing the members above their parent 'instance'.

- sysmo, lc15 and oct omit the bts_model_config_write_phy_inst()
  implementation. This did not cause a compilation problem because it
  was in fact never called.

- sysmo writes 'clock-source None' when clk_src is zero, leading to unparsable
  config (related: OS#1944). Instead omit the 'clock-source' when zero.

- osmo-bts-trx seems to be the only part that lacks nothing, yet it
  also didn't work properly because bts_model_config_write_phy_inst()
  was never called.
  
This problem existed since commit d784e50747b8cf0ce505489e1451f75be5ccbd4b
"Introduce new phy_link and phy_instance abstraction"

Change-Id: Icc54fa70045c8fa58e78cf9f788c21a437edfbd4
---
M src/common/vty.c
M src/osmo-bts-litecell15/lc15bts_vty.c
M src/osmo-bts-octphy/octphy_vty.c
M src/osmo-bts-sysmo/sysmobts_vty.c
4 files changed, 19 insertions(+), 21 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/common/vty.c b/src/common/vty.c
index b48afa4..646ac87 100644
--- a/src/common/vty.c
+++ b/src/common/vty.c
@@ -358,6 +358,7 @@
 		if (!pinst)
 			break;
 		vty_out(vty, " instance %u%s", pinst->num, VTY_NEWLINE);
+		bts_model_config_write_phy_inst(vty, pinst);
 	}
 }
 
diff --git a/src/osmo-bts-litecell15/lc15bts_vty.c b/src/osmo-bts-litecell15/lc15bts_vty.c
index 0ffe663..46f73ab 100644
--- a/src/osmo-bts-litecell15/lc15bts_vty.c
+++ b/src/osmo-bts-litecell15/lc15bts_vty.c
@@ -329,7 +329,11 @@
 	vty_out(vty, "  nominal-tx-power %d%s", trx->nominal_power,VTY_NEWLINE);
 }
 
-static void write_phy_inst(struct vty *vty, struct phy_instance *pinst)
+void bts_model_config_write_phy(struct vty *vty, struct phy_link *plink)
+{
+}
+
+void bts_model_config_write_phy_inst(struct vty *vty, struct phy_instance *pinst)
 {
 	int i;
 
@@ -344,14 +348,6 @@
 	if (pinst->u.lc15.calib_path)
 		vty_out(vty, "  trx-calibration-path %s%s",
 			pinst->u.lc15.calib_path, VTY_NEWLINE);
-}
-
-void bts_model_config_write_phy(struct vty *vty, struct phy_link *plink)
-{
-	struct phy_instance *pinst;
-
-	llist_for_each_entry(pinst, &plink->instances, list)
-		write_phy_inst(vty, pinst);
 }
 
 int bts_model_vty_init(struct gsm_bts *bts)
diff --git a/src/osmo-bts-octphy/octphy_vty.c b/src/osmo-bts-octphy/octphy_vty.c
index c2d1fbb..abfac1e 100644
--- a/src/osmo-bts-octphy/octphy_vty.c
+++ b/src/osmo-bts-octphy/octphy_vty.c
@@ -219,6 +219,10 @@
 		VTY_NEWLINE);
 }
 
+void bts_model_config_write_phy_inst(struct vty *vty, struct phy_instance *pinst)
+{
+}
+
 void bts_model_config_write_bts(struct vty *vty, struct gsm_bts *bts)
 {
 }
diff --git a/src/osmo-bts-sysmo/sysmobts_vty.c b/src/osmo-bts-sysmo/sysmobts_vty.c
index e67d8be..330c9ce 100644
--- a/src/osmo-bts-sysmo/sysmobts_vty.c
+++ b/src/osmo-bts-sysmo/sysmobts_vty.c
@@ -443,7 +443,11 @@
 			VTY_NEWLINE);
 }
 
-static void write_phy_inst(struct vty *vty, struct phy_instance *pinst)
+void bts_model_config_write_phy(struct vty *vty, struct phy_link *plink)
+{
+}
+
+void bts_model_config_write_phy_inst(struct vty *vty, struct phy_instance *pinst)
 {
 	int i;
 
@@ -464,17 +468,10 @@
 	if (pinst->u.sysmobts.calib_path)
 		vty_out(vty, "  trx-calibration-path %s%s",
 			pinst->u.sysmobts.calib_path, VTY_NEWLINE);
-	vty_out(vty, "  clock-source %s%s",
-		get_value_string(femtobts_clksrc_names,
-				 pinst->u.sysmobts.clk_src), VTY_NEWLINE);
-}
-
-void bts_model_config_write_phy(struct vty *vty, struct phy_link *plink)
-{
-	struct phy_instance *pinst;
-
-	llist_for_each_entry(pinst, &plink->instances, list)
-		write_phy_inst(vty, pinst);
+	if (pinst->u.sysmobts.clk_src)
+		vty_out(vty, "  clock-source %s%s",
+			get_value_string(femtobts_clksrc_names,
+					 pinst->u.sysmobts.clk_src), VTY_NEWLINE);
 }
 
 int bts_model_vty_init(struct gsm_bts *bts)

-- 
To view, visit https://gerrit.osmocom.org/1761
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Icc54fa70045c8fa58e78cf9f788c21a437edfbd4
Gerrit-PatchSet: 4
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Ivan Kluchnikov <kluchnikovi at gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>