pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/20990 )
Change subject: pcuif: Improve BTS-supported CS/MCS handling ...................................................................... Patch Set 1: (5 comments) https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/gprs_bssgp_pcu.cpp File src/gprs_bssgp_pcu.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/gprs_bssgp_pcu.cpp@749 PS1, Line 749: break > Also, what if none of the MCS variants is supported? Would we leave 'num' > uninitialized? Ack https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/gprs_bssgp_pcu.cpp@769 PS1, Line 769: break > Same here, but in this case it's less likely given that CS-1 is mandatory > AFAIR. Ack https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/pcu_l1_if.cpp File src/pcu_l1_if.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/pcu_l1_if.cpp@619 PS1, Line 619: > two tabs? why? Ack https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/pcu_l1_if.cpp@620 PS1, Line 620: 0 > Also, shouldn't this be 'i' like below? Ack https://gerrit.osmocom.org/c/osmo-pcu/+/20990/1/src/pcu_l1_if.cpp@621 PS1, Line 621: any_cs_supported > What is the motivation here? If the BTS indicates that none is supported, > then it's a bug. I'm just keeping old behavior here, see previous version of the patch line 648 -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/20990 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Ie8f0215ba17da1e545e98bec9325c02f1e8efaea Gerrit-Change-Number: 20990 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Comment-Date: Mon, 02 Nov 2020 10:23:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <[email protected]> Gerrit-MessageType: comment
