fix nanobts: timeslot FSM: use flags to remember OML,RSL status

Before this patch, the timeslot FSM receives OML and RSL ready events.
Afterwards, it relies on examining the RSL and OML status to match the received
events. This doesn't work for the ip.access nanobts, which fails to change the
CHANNEL OM's operational status even though it has sent an Opstart ACK.  We
receive OML CHANNEL Opstart ACK, but the mo's state left at OP_STATE=Disabled.
We apparently cannot rely on the gsm_abis_mo state as assumed before this
patch, since changing the state depends on each BTS vendor's OML
implementation.

Also, implementation wise, it is better to not include assumptions on RSL and
OML implementations in the timeslot FSM. Simply receive the OML and RSL ready
events and remember that they arrived in dedicated flags.

Remove the no longer needed oml_is_ts_ready() callback from struct
gsm_bts_model added in:

  commit 91aa68f762218906e45be4817c6ea54b480da5e1
  "dyn TS: init only when both RSL and the Channel OM are established"
  I99f29d2ba079f6f4b77f0af12d9784588d2f56b3

This keeps osmo-bts operational while fixing ip.access nanobts, where the
CHANNEL OM's state prevented the timeslot FSM from entering operation.

Change-Id: I4843d03b3237cdcca0ad2041ef6895ff253d8419

5 files changed, 12 insertions, 17 deletions

diff --git a/include/osmocom/bsc/gsm_data.h b/include/osmocom/bsc/gsm_data.h
index ce563ef17..750c02794 100644
--- a/include/osmocom/bsc/gsm_data.h
+++ b/include/osmocom/bsc/gsm_data.h
@@ -605,6 +605,11 @@ struct gsm_bts_trx_ts {
 	 * enters IN_USE state, i.e. after each TCH use we try to PDCH ACT once again. */
 	bool pdch_act_allowed;
 
+	/* Whether TS_EV_OML_READY was received */
+	bool is_oml_ready;
+	/* Whether TS_EV_RSL_READY was received */
+	bool is_rsl_ready;
+
 	struct gsm_abis_mo mo;
 	struct tlv_parsed nm_attr;
 	uint8_t nm_chan_comb;
@@ -739,7 +744,6 @@ struct gsm_bts_model {
 	int (*start)(struct gsm_network *net);
 	int (*oml_rcvmsg)(struct msgb *msg);
 	char * (*oml_status)(const struct gsm_bts *bts);
-	bool (*oml_is_ts_ready)(const struct gsm_bts_trx_ts *ts);
 
 	void (*e1line_bind_ops)(struct e1inp_line *line);
 
diff --git a/src/osmo-bsc/bts_ericsson_rbs2000.c b/src/osmo-bsc/bts_ericsson_rbs2000.c
index c2975f4c6..ed3db939a 100644
--- a/src/osmo-bsc/bts_ericsson_rbs2000.c
+++ b/src/osmo-bsc/bts_ericsson_rbs2000.c
@@ -175,17 +175,11 @@ static void bts_model_rbs2k_e1line_bind_ops(struct e1inp_line *line)
 	e1inp_line_bind_ops(line, &bts_isdn_e1inp_line_ops);
 }
 
-static bool bts_model_rbs2k_is_ts_ready(const struct gsm_bts_trx_ts *ts)
-{
-	return ts && ts->mo.nm_state.operational == NM_OPSTATE_ENABLED;
-}
-
 static struct gsm_bts_model model_rbs2k = {
 	.type = GSM_BTS_TYPE_RBS2000,
 	.name = "rbs2000",
 	.start = bts_model_rbs2k_start,
 	.oml_rcvmsg = &abis_om2k_rcvmsg,
-	.oml_is_ts_ready = bts_model_rbs2k_is_ts_ready,
 	.config_write_bts = &config_write_bts,
 	.e1line_bind_ops = &bts_model_rbs2k_e1line_bind_ops,
 };
diff --git a/src/osmo-bsc/bts_ipaccess_nanobts.c b/src/osmo-bsc/bts_ipaccess_nanobts.c
index 6b6e26550..80f7c9c6d 100644
--- a/src/osmo-bsc/bts_ipaccess_nanobts.c
+++ b/src/osmo-bsc/bts_ipaccess_nanobts.c
@@ -55,18 +55,12 @@ static char *get_oml_status(const struct gsm_bts *bts)
 	return "disconnected";
 }
 
-static bool oml_is_ts_ready(const struct gsm_bts_trx_ts *ts)
-{
-	return ts && ts->mo.nm_state.operational == NM_OPSTATE_ENABLED;
-}
-
 struct gsm_bts_model bts_model_nanobts = {
 	.type = GSM_BTS_TYPE_NANOBTS,
 	.name = "nanobts",
 	.start = bts_model_nanobts_start,
 	.oml_rcvmsg = &abis_nm_rcvmsg,
 	.oml_status = &get_oml_status,
-	.oml_is_ts_ready = oml_is_ts_ready,
 	.e1line_bind_ops = bts_model_nanobts_e1line_bind_ops, 
 	.nm_att_tlvdef = {
 		.def = {
diff --git a/src/osmo-bsc/timeslot_fsm.c b/src/osmo-bsc/timeslot_fsm.c
index 84d80f8f8..245ce76f7 100644
--- a/src/osmo-bsc/timeslot_fsm.c
+++ b/src/osmo-bsc/timeslot_fsm.c
@@ -220,13 +220,13 @@ static void ts_setup_lchans(struct gsm_bts_trx_ts *ts)
 static void ts_fsm_not_initialized(struct osmo_fsm_inst *fi, uint32_t event, void *data)
 {
 	struct gsm_bts_trx_ts *ts = ts_fi_ts(fi);
-	struct gsm_bts *bts = ts->trx->bts;
 	switch (event) {
 
 	case TS_EV_OML_READY:
 		ts->pdch_act_allowed = true;
+		ts->is_oml_ready = true;
 		ts_setup_lchans(ts);
-		if (!ts->trx->rsl_link) {
+		if (!ts->is_rsl_ready) {
 			LOG_TS(ts, LOGL_DEBUG, "No RSL link yet\n");
 			return;
 		}
@@ -235,8 +235,8 @@ static void ts_fsm_not_initialized(struct osmo_fsm_inst *fi, uint32_t event, voi
 
 	case TS_EV_RSL_READY:
 		ts->pdch_act_allowed = true;
-		if (bts->model->oml_is_ts_ready
-		    && !bts->model->oml_is_ts_ready(ts)) {
+		ts->is_rsl_ready = true;
+		if (!ts->is_oml_ready) {
 			LOG_TS(ts, LOGL_DEBUG, "OML not ready yet\n");
 			return;
 		}
@@ -680,6 +680,7 @@ static void ts_fsm_allstate(struct osmo_fsm_inst *fi, uint32_t event, void *data
 	struct gsm_bts_trx_ts *ts = ts_fi_ts(fi);
 	switch (event) {
 	case TS_EV_OML_DOWN:
+		ts->is_oml_ready = false;
 		if (fi->state != TS_ST_NOT_INITIALIZED)
 			osmo_fsm_inst_state_chg(fi, TS_ST_NOT_INITIALIZED, 0, 0);
 		OSMO_ASSERT(fi->state == TS_ST_NOT_INITIALIZED);
@@ -689,6 +690,7 @@ static void ts_fsm_allstate(struct osmo_fsm_inst *fi, uint32_t event, void *data
 		break;
 
 	case TS_EV_RSL_DOWN:
+		ts->is_rsl_ready = false;
 		if (fi->state != TS_ST_NOT_INITIALIZED)
 			osmo_fsm_inst_state_chg(fi, TS_ST_NOT_INITIALIZED, 0, 0);
 		OSMO_ASSERT(fi->state == TS_ST_NOT_INITIALIZED);
diff --git a/tests/handover/handover_test.c b/tests/handover/handover_test.c
index 3a5748e5a..172e620fa 100644
--- a/tests/handover/handover_test.c
+++ b/tests/handover/handover_test.c
@@ -210,6 +210,7 @@ static struct gsm_bts *create_bts(int arfcn)
 
 	for (i = 0; i < ARRAY_SIZE(bts->c0->ts); i++) {
 		/* make sure ts->lchans[] get initialized */
+		osmo_fsm_inst_dispatch(bts->c0->ts[i].fi, TS_EV_RSL_READY, 0);
 		osmo_fsm_inst_dispatch(bts->c0->ts[i].fi, TS_EV_OML_READY, 0);
 	}
 	return bts;