Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27712 )
Change subject: Add subscr_conn_fsm ......................................................................
Patch Set 3:
(7 comments)
File include/osmocom/bsc_nat/bssap.h:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27712/comment/00c102bf_20721566 PS2, Line 30: struct bssmap_assignment_rqst {
bssmap_ass_req?
Done
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27712/comment/03790ba0_9b9cba3d PS2, Line 47: struct bssmap_assignment_complete {
bssmap_ass_compl?
Done
File src/osmo-bsc-nat/bssap_conn.c:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27712/comment/2e653571_9d03d99b PS2, Line 69: static int bssmap_cn_handle_assignment_rqst(struct subscr_conn *subscr_conn, struct msgb *msg, unsigned int length)
bssmap_cn_handle_ass_req?
Done
File src/osmo-bsc-nat/subscr_conn.c:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27712/comment/85bb7e60_ea5bbadf PS2, Line 32: int subscr_conn_get_next_id(enum subscr_conn_id_type id_type)
I'd rather split in 3 functions, using a macro if necessary.
Done
File src/osmo-bsc-nat/subscr_conn_fsm.c:
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27712/comment/02e773ca_62f00c8a PS2, Line 72: case SUBSCR_CONN_FSM_EV_BSSMAP_CLEAR_COMMAND:
missing break. Probably not affecting in behavior but I don't see an intended fall-through here.
This was intentional, but added a break to make it explicit.
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27712/comment/e545b812_a5b5d305 PS2, Line 172: static int tx_assignment_request_to_ran(const struct subscr_conn *subscr_conn)
If possible please move helper functions to the top, and put all the state FM functions together.
Done
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27712/comment/d8535d82_7b0a4c30 PS2, Line 351: ,
nice style to lose one an extra line for no good reason :D (j/k)
Neels invented this, but I like how it's diff friendly :)