Refactor and clarify tbf->triger_ass() code

Split the function into 2 functions, one for assignment on PACCH and one
for assignment on PCH. This makes code calling this API far more clearer
on what is the exact aim when assigning the TBF.

Change-Id: Ic92867e55337b0bd6b5bfc97f13b7982eedb1cb7

6 files changed, 43 insertions, 28 deletions

diff --git a/src/gprs_ms.c b/src/gprs_ms.c
index 5733a7cb..c072b0af 100644
--- a/src/gprs_ms.c
+++ b/src/gprs_ms.c
@@ -1085,7 +1085,10 @@ int ms_new_dl_tbf_assignment(struct GprsMs *ms)
 	LOGPTBFDL(dl_tbf, LOGL_DEBUG, "[DOWNLINK] START\n");
 
 	/* Trigger the assignment now. */
-	tbf_dl_trigger_ass(dl_tbf, ul_tbf_as_tbf(ul_tbf));
+	if (ul_tbf)
+		dl_tbf_trigger_ass_on_pacch(dl_tbf, ul_tbf_as_tbf(ul_tbf));
+	else
+		dl_tbf_trigger_ass_on_pch(dl_tbf);
 	return 0;
 }
 
diff --git a/src/tbf.cpp b/src/tbf.cpp
index d115b825..4bf4ce80 100644
--- a/src/tbf.cpp
+++ b/src/tbf.cpp
@@ -703,7 +703,7 @@ int gprs_rlcmac_tbf::establish_dl_tbf_on_pacch()
 	}
 
 	LOGPTBF(this, LOGL_DEBUG, "Trigger downlink assignment on PACCH\n");
-	new_tbf->trigger_ass(this);
+	dl_tbf_trigger_ass_on_pacch(new_tbf, this);
 
 	return 0;
 }
@@ -960,3 +960,8 @@ struct gprs_rlcmac_trx *tbf_get_trx(struct gprs_rlcmac_tbf *tbf)
 {
 	return tbf->trx;
 }
+
+void tbf_stop_timers(struct gprs_rlcmac_tbf *tbf, const char *reason)
+{
+	tbf->stop_timers(reason);
+}
\ No newline at end of file
diff --git a/src/tbf.h b/src/tbf.h
index d69ae5a2..09de188e 100644
--- a/src/tbf.h
+++ b/src/tbf.h
@@ -150,6 +150,7 @@ bool tbf_can_upgrade_to_multislot(const struct gprs_rlcmac_tbf *tbf);
 int tbf_update(struct gprs_rlcmac_tbf *tbf);
 int tbf_establish_dl_tbf_on_pacch(struct gprs_rlcmac_tbf *tbf);
 struct gprs_rlcmac_trx *tbf_get_trx(struct gprs_rlcmac_tbf *tbf);
+void tbf_stop_timers(struct gprs_rlcmac_tbf *tbf, const char *reason);
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/tbf_dl.cpp b/src/tbf_dl.cpp
index 0936e1c8..856dae69 100644
--- a/src/tbf_dl.cpp
+++ b/src/tbf_dl.cpp
@@ -418,29 +418,41 @@ struct msgb *gprs_rlcmac_dl_tbf::create_dl_acked_block(uint32_t fn, uint8_t ts,
 	return create_dl_acked_block(fn, ts, bsn, bsn2);
 }
 
-/* depending on the current TBF, we assign on PACCH or AGCH */
-void gprs_rlcmac_dl_tbf::trigger_ass(struct gprs_rlcmac_tbf *old_tbf)
+/* old_tbf (UL TBF or DL TBF) will send a Pkt Dl Ass on PACCH to assign tbf.
+ * Note: It is possible that "tbf == old_tbf" if the TBF is being updated. This can
+ * happen when we first assign over PCH (only single slot is possible) and we want
+ * to upgrade the DL-TBF to be multislot. See code calling tbf_update() for more
+ * information.
+ */
+void dl_tbf_trigger_ass_on_pacch(struct gprs_rlcmac_dl_tbf *tbf, struct gprs_rlcmac_tbf *old_tbf)
 {
+	OSMO_ASSERT(tbf);
+	OSMO_ASSERT(old_tbf);
 	/* stop pending timer */
-	stop_timers("assignment (DL-TBF)");
+	tbf_stop_timers(tbf, "DL assignment (PACCH)");
 
-	/* check for downlink tbf:  */
-	if (old_tbf) {
-		LOGPTBFDL(this, LOGL_DEBUG, "Send downlink assignment on PACCH, because %s exists\n", old_tbf->name());
-		osmo_fsm_inst_dispatch(old_tbf->dl_ass_fsm.fi, TBF_DL_ASS_EV_SCHED_ASS, NULL);
+	LOGPTBFDL(tbf, LOGL_DEBUG, "Send downlink assignment on PACCH, because %s exists\n", old_tbf->name());
+	osmo_fsm_inst_dispatch(old_tbf->dl_ass_fsm.fi, TBF_DL_ASS_EV_SCHED_ASS, NULL);
 
-		/* change state */
-		osmo_fsm_inst_dispatch(this->state_fsm.fi, TBF_EV_ASSIGN_ADD_PACCH, NULL);
-	} else {
-		LOGPTBFDL(this, LOGL_DEBUG, "Send downlink assignment on PCH, no TBF exist (IMSI=%s)\n",
-			  imsi());
+	/* change state */
+	osmo_fsm_inst_dispatch(tbf->state_fsm.fi, TBF_EV_ASSIGN_ADD_PACCH, NULL);
 
-		/* change state */
-		osmo_fsm_inst_dispatch(this->state_fsm.fi, TBF_EV_ASSIGN_ADD_CCCH, NULL);
+}
 
-		/* send immediate assignment */
-		bts_snd_dl_ass(bts, this);
-	}
+void dl_tbf_trigger_ass_on_pch(struct gprs_rlcmac_dl_tbf *tbf)
+{
+	/* stop pending timer */
+	struct GprsMs *ms = tbf_ms(tbf);
+
+	tbf_stop_timers(tbf, "DL assignment (PCH)");
+
+	LOGPTBFDL(tbf, LOGL_DEBUG, "Send downlink assignment on PCH, no TBF exist (IMSI=%s)\n", ms_imsi(ms));
+
+	/* change state */
+	osmo_fsm_inst_dispatch(tbf->state_fsm.fi, TBF_EV_ASSIGN_ADD_CCCH, NULL);
+
+	/* send immediate assignment */
+	bts_snd_dl_ass(ms->bts, tbf);
 }
 
 void gprs_rlcmac_dl_tbf::schedule_next_frame()
@@ -1272,8 +1284,3 @@ struct gprs_rlcmac_dl_tbf *tbf_as_dl_tbf(struct gprs_rlcmac_tbf *tbf)
 	else
 		return NULL;
 }
-
-void tbf_dl_trigger_ass(struct gprs_rlcmac_dl_tbf *tbf, struct gprs_rlcmac_tbf *old_tbf)
-{
-	return tbf->trigger_ass(old_tbf);
-}
diff --git a/src/tbf_dl.h b/src/tbf_dl.h
index e0daf570..ba5d582d 100644
--- a/src/tbf_dl.h
+++ b/src/tbf_dl.h
@@ -41,7 +41,6 @@ struct gprs_rlcmac_dl_tbf : public gprs_rlcmac_tbf {
 
 	int rcvd_dl_ack(bool final_ack, unsigned first_bsn, struct bitvec *rbb);
 	struct msgb *create_dl_acked_block(uint32_t fn, uint8_t ts, enum mcs_kind req_mcs_kind = EGPRS);
-	void trigger_ass(struct gprs_rlcmac_tbf *old_tbf);
 
 	void request_dl_ack();
 	bool need_poll_for_dl_ack_nack() const;
@@ -138,7 +137,8 @@ int dl_tbf_handle(struct gprs_rlcmac_bts *bts,
 		  const uint8_t egprs_ms_class, const uint16_t delay_csec,
 		  const uint8_t *data, const uint16_t len);
 
-void tbf_dl_trigger_ass(struct gprs_rlcmac_dl_tbf *tbf, struct gprs_rlcmac_tbf *old_tbf);
+void dl_tbf_trigger_ass_on_pacch(struct gprs_rlcmac_dl_tbf *tbf, struct gprs_rlcmac_tbf *old_tbf);
+void dl_tbf_trigger_ass_on_pch(struct gprs_rlcmac_dl_tbf *tbf);
 void tbf_dl_request_dl_ack(struct gprs_rlcmac_dl_tbf *tbf);
 
 static inline struct gprs_rlcmac_tbf *dl_tbf_as_tbf(struct gprs_rlcmac_dl_tbf *dl_tbf)
diff --git a/src/tbf_fsm.c b/src/tbf_fsm.c
index 25253fdd..eeccd6b1 100644
--- a/src/tbf_fsm.c
+++ b/src/tbf_fsm.c
@@ -387,8 +387,7 @@ static void handle_timeout_X2002(struct tbf_fsm_ctx *ctx)
 		ctx->state_flags &= GPRS_RLCMAC_FLAG_TO_MASK;
 
 		tbf_update(ctx->tbf);
-
-		tbf_dl_trigger_ass(dl_tbf, ctx->tbf);
+		dl_tbf_trigger_ass_on_pacch(dl_tbf, ctx->tbf);
 	} else
 		LOGPTBF(ctx->tbf, LOGL_NOTICE, "Continue flow after IMM.ASS confirm\n");
 }