Change in osmo-bsc[master]: bsc: timeslot_fsm: Switch to error state before dispatching error eve...

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Thu Nov 29 18:27:58 UTC 2018


Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/11995 )

Change subject: bsc: timeslot_fsm: Switch to error state before dispatching error event to lchans
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

please bear with my scepticism, as the way the FSMs interact can become quite complex and I want to understand the implications before I approve.

https://gerrit.osmocom.org/#/c/11995/2//COMMIT_MSG
Commit Message:

https://gerrit.osmocom.org/#/c/11995/2//COMMIT_MSG@10
PS2, Line 10: to handle them, as a result from dispatching LCHAN_EV_TS_ERROR previously.
I don't understand: which state is unable to handle TS_EV_LCHAN_UNUSED? If it should be able to, then let's make it handle that event properly?


https://gerrit.osmocom.org/#/c/11995/2/src/osmo-bsc/timeslot_fsm.c
File src/osmo-bsc/timeslot_fsm.c:

https://gerrit.osmocom.org/#/c/11995/2/src/osmo-bsc/timeslot_fsm.c@177
PS2, Line 177: 	ts_lchans_dispatch(ts, LCHAN_ST_WAIT_TS_READY, LCHAN_EV_TS_ERROR);
Sorry, I'm not convinced:

- my premise is that the lchan should be able to handle a TS_ERROR independently from the timeslot's state.

- when a timeslot changes its state, it might trigger immediate other actions, maybe also affecting lchans. I'm imagining an lchan being put up for more activity before it was even told that there was some problem before...

I'm sure you've seen an example that shows that this makes sense; instead I'm arguing from the angle of the ideas and premises I had in mind when writing the way the FSMs interact, trying to cover all (un)thinkable situations.



-- 
To view, visit https://gerrit.osmocom.org/11995
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If61493e7d5449bf2c2de9fd34cdf2410625e92ac
Gerrit-Change-Number: 11995
Gerrit-PatchSet: 2
Gerrit-Owner: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Comment-Date: Thu, 29 Nov 2018 18:27:58 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181129/f8508fc7/attachment.html>


More information about the gerrit-log mailing list