Attention is currently required from: fixeria, pespin. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/27356 )
Change subject: bitvec2freq_list(): fix handling of E-GSM ARFCNs ...................................................................... Patch Set 1: Code-Review-1 (3 comments) Patchset: PS1: this is the kind of issue where a clear-cut unit test showing the bug should be introduced first, and then the bug should be fixed in a follow-up commit, demonstrating that the unit test now passes. I don't see immediately how his commit fixes anything - all it does is to return error values to the caller and log something, but it's not immediately obvious how an alternative encoding is chosen. File src/osmo-bsc/system_information.c: https://gerrit.osmocom.org/c/osmo-bsc/+/27356/comment/14e3e6ae_1cdc5154 PS1, Line 508: && bts->c0->arfcn >= 1 && bts->c0->arfcn <= 124) : pgsm = true; isn't this the true bug? PGSM is primary GSM, and not E-GSM 900. So the pgsm flag should never be set in a situation where E-GSM channels are used. Maybe we should not just check for c0->arfcn but iterate over all channels and set pgsm only true if all of them are within the P-GSM range? https://gerrit.osmocom.org/c/osmo-bsc/+/27356/comment/a9653cb1_024d2227 PS1, Line 522: / how are we attempting to use a different format here? we return an error. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/27356 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I17739e6845cd84e2a81bc406dd532541f7c52cb6 Gerrit-Change-Number: 27356 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: pespin <[email protected]> Gerrit-Attention: fixeria <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Comment-Date: Wed, 02 Mar 2022 09:13:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
