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 ;) -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/34491?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ia55c182a1b4cde777a0e16dcfe0cd32e0f2e38eb Gerrit-Change-Number: 34491 Gerrit-PatchSet: 5 Gerrit-Owner: jolly <andr...@eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pes...@sysmocom.de> Gerrit-Attention: jolly <andr...@eversberg.eu> Gerrit-Comment-Date: Thu, 21 Sep 2023 14:15:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
