[XL] Change in osmocom-bb[master]: ASCI: Add group receive and transmit mode support to RR layer
Jenkins Builder 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 1: (5 comments) File src/host/layer23/src/mobile/gsm48_rr.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-11164): https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/0a85918b_6c805ed3 PS1, Line 510: while((msg = msgb_dequeue(&rr->downqueue))) space required before the open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-gerrit-lint-11164): https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/181b3ab1_a88dd160 PS1, Line 1529: if (bitvec_get_bit_high(&bv) == H) { braces {} are not necessary for single statement blocks Robot Comment from checkpatch (run ID jenkins-gerrit-lint-11164): https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/47e0d30d_0665a654 PS1, Line 1963: /* Note: Other informations are not used. */ 'informations' may be misspelled - perhaps 'information'? Robot Comment from checkpatch (run ID jenkins-gerrit-lint-11164): https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/2be13072_755c2368 PS1, Line 5501: reject: labels should not be indented Robot Comment from checkpatch (run ID jenkins-gerrit-lint-11164): https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/a15c330a_469edcb7 PS1, Line 6389: /* Leave uplink, wait for uplink beeing free, channel release or channel/link failure. */ 'beeing' may be misspelled - perhaps 'being'? -- 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: 1 Gerrit-Owner: jolly Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Thu, 21 Sep 2023 09:39:31 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[XL] Change in osmocom-bb[master]: ASCI: Add group receive and transmit mode support to RR layer
Attention is currently required from: jolly. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmocom-bb/+/34491?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified-1 by Jenkins Builder Change subject: ASCI: Add group receive and transmit mode support to RR layer .. ASCI: Add group receive and transmit mode support to RR layer Related: OS#5364 Change-Id: Ia55c182a1b4cde777a0e16dcfe0cd32e0f2e38eb --- M src/host/layer23/include/osmocom/bb/common/settings.h M src/host/layer23/include/osmocom/bb/mobile/gsm48_rr.h M src/host/layer23/src/mobile/gsm48_rr.c 3 files changed, 1,386 insertions(+), 84 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/91/34491/2 -- 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: 2 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: jolly Gerrit-MessageType: newpatchset
[XL] Change in osmocom-bb[master]: ASCI: Add group receive and transmit mode support to RR layer
Attention is currently required from: jolly. Jenkins Builder 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 2: (5 comments) File src/host/layer23/src/mobile/gsm48_rr.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-11187): https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/3c4cbcfc_3896902e PS2, Line 510: while((msg = msgb_dequeue(&rr->downqueue))) space required before the open parenthesis '(' Robot Comment from checkpatch (run ID jenkins-gerrit-lint-11187): https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/a70ff070_d9203d79 PS2, Line 1529: if (bitvec_get_bit_high(&bv) == H) { braces {} are not necessary for single statement blocks Robot Comment from checkpatch (run ID jenkins-gerrit-lint-11187): https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/7e4c8b7a_3a849f2a PS2, Line 1963: /* Note: Other informations are not used. */ 'informations' may be misspelled - perhaps 'information'? Robot Comment from checkpatch (run ID jenkins-gerrit-lint-11187): https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/55485bd5_cac4ac8e PS2, Line 5501: reject: labels should not be indented Robot Comment from checkpatch (run ID jenkins-gerrit-lint-11187): https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/286eef42_178bfe11 PS2, Line 6389: /* Leave uplink, wait for uplink beeing free, channel release or channel/link failure. */ 'beeing' may be misspelled - perhaps 'being'? -- 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: 2 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: jolly Gerrit-Comment-Date: Thu, 21 Sep 2023 10:34:54 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[XL] Change in osmocom-bb[master]: ASCI: Add group receive and transmit mode support to RR layer
Attention is currently required from: jolly. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmocom-bb/+/34491?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Verified-1 by Jenkins Builder Change subject: ASCI: Add group receive and transmit mode support to RR layer .. ASCI: Add group receive and transmit mode support to RR layer Related: OS#5364 Change-Id: Ia55c182a1b4cde777a0e16dcfe0cd32e0f2e38eb --- M src/host/layer23/include/osmocom/bb/common/settings.h M src/host/layer23/include/osmocom/bb/mobile/gsm48_rr.h M src/host/layer23/src/mobile/gsm48_rr.c 3 files changed, 1,385 insertions(+), 84 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmocom-bb refs/changes/91/34491/3 -- 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: 3 Gerrit-Owner: jolly Gerrit-Reviewer: Jenkins Builder Gerrit-Attention: jolly Gerrit-MessageType: newpatchset
[XL] Change in osmocom-bb[master]: ASCI: Add group receive and transmit mode support to RR layer
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 Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin Gerrit-Attention: jolly Gerrit-Comment-Date: Thu, 21 Sep 2023 14:15:57 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[XL] Change in osmocom-bb[master]: ASCI: Add group receive and transmit mode support to RR layer
Attention is currently required from: jolly, pespin. fixeria 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: (1 comment) File src/host/layer23/include/osmocom/bb/mobile/gsm48_rr.h: https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/93099814_1baff3f0 PS5, Line 220: /* group call */ > what about putting all this inside "gc" substruct? Ack -- 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 Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-CC: pespin Gerrit-Attention: jolly Gerrit-Attention: pespin Gerrit-Comment-Date: Thu, 21 Sep 2023 22:39:48 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[XL] Change in osmocom-bb[master]: ASCI: Add group receive and transmit mode support to RR layer
Attention is currently required from: fixeria, pespin. jolly 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/d7301427_57bd0550 PS5, Line 89: #define GSM48_RR_GST_OFF 0 > can we have this as an enum? Done https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/5038e36f_e615c5c6 PS5, Line 220: /* group call */ > Ack Done https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/82da7549_fe3f184a PS5, Line 222: int group_state;/* extension to RR state for group transmit/receive modes */ > can we have this as an enum? Done https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/9b5fec6e_e5346a10 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)? Done File src/host/layer23/src/mobile/gsm48_rr.c: https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/c9afe6b7_93ed519c PS5, Line 73: * GSM48_MM_EVENT_UPLINK_BUSY: Notify MM layer about uplink becoming busy. > I think you added these a few commits before. […] Only GSM48_MM_EVENT_NOTIFICATION has been added before. These comments describe the new interface to the process implemented with this patch. https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/ff0008c2_33132097 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. It is out of the scope of this patch. The functions are not changed, they are just defined here, because they are used in code above the implementation. https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/ddb252b8_9fbff766 PS5, Line 414: if (rr->state == state && state != GSM48_RR_ST_IDLE) { > are you sure this is correct' this should be == iiuc, not !=. It is allowed to change from IDLE to IDLE. The group sub-state may have changed, but the state itself. https://gerrit.osmocom.org/c/osmocom-bb/+/34491/comment/68171800_e120be3e PS5, Line 7080: > we should start thinking about splitting this file into smaller pieces ;) Maybe. This is one layer. I changed the location of the new functions inside the gsm48_rr.c file. There is also a group now for all functions releated to uplink control. Also I split this patch into several smaller patches for easier review. -- 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 Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-CC: pespin Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Comment-Date: Tue, 26 Sep 2023 10:37:01 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[XL] Change in osmocom-bb[master]: ASCI: Add group receive and transmit mode support to RR layer
jolly has abandoned 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 .. Abandoned This patch has been split. -- 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 Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria Gerrit-CC: pespin Gerrit-MessageType: abandon