Attention is currently required from: jolly.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/34491?usp=email )
Change subject: ASCI: Add group receive and transmit mode support to RR layer ......................................................................
Patch Set 5:
(8 comments)
File src/host/layer23/include/osmocom/bb/mobile/gsm48_rr.h:
https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/9a764c4b_b77e84b1 PS5, Line 89: #define GSM48_RR_GST_OFF 0 can we have this as an enum?
https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/67d7d1bf_7e586990 PS5, Line 220: /* group call */ what about putting all this inside "gc" substruct?
https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/ea586a2e_80cfb3c9 PS5, Line 222: int group_state; /* extension to RR state for group transmit/receive modes */ can we have this as an enum?
https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/911c9204_ca1db00e PS5, Line 230: int uplink_tries; /* Counts number of tries to access the uplink. */ if it's a counter, do we need negative values (int)?
File src/host/layer23/src/mobile/gsm48_rr.c:
https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/6933043e_360022c8 PS5, Line 73: * GSM48_MM_EVENT_UPLINK_BUSY: Notify MM layer about uplink becoming busy. I think you added these a few commits before. They should probably be in this commit instead since this is where you are using them.
https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/65122fac_c72b8f4e PS5, Line 134: static int gsm48_rr_render_ma(struct osmocom_ms *ms, struct gsm48_rr_cd *cd, uint16_t *ma, uint8_t *ma_len); I bet some of this pointers can be const.
https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/e5bc763e_f1826eea PS5, Line 414: if (rr->state == state && state != GSM48_RR_ST_IDLE) { are you sure this is correct' this should be == iiuc, not !=.
https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/8da2ae0f_633b05ad PS5, Line 7080: we should start thinking about splitting this file into smaller pieces ;)