Attention is currently required from: laforge, fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/28086 ) Change subject: measurement: fix matching of SUB frames by TDMA FN ...................................................................... Patch Set 1: (4 comments) File src/common/measurement.c: https://gerrit.osmocom.org/c/osmo-bts/+/28086/comment/21f79b23_84dcb824 PS1, Line 23: /* TCH/H(0): 0, 2, 4, 6, 52, 54, 56, 58 */ Maybe use ";" to separate between the 2 blocks, it's easier to understand: /* TCH/H(0): 0, 2, 4, 6; 52, 54, 56, 58 */ Or is 1 block 8 FNs here? https://gerrit.osmocom.org/c/osmo-bts/+/28086/comment/33b55a18_01d7f7cb PS1, Line 26: /* TCH/H(1): 14, 16, 18, 20, 66, 68, 70, 72 */ same https://gerrit.osmocom.org/c/osmo-bts/+/28086/comment/c99c5bc8_cfc2f016 PS1, Line 65: * There is only one *complete* block in this subset starting at FN=52. So you first say there's a complete block starting at FN=52, but in next line you say the one starting at 52 is incomplete? File tests/meas/meas_test.c: https://gerrit.osmocom.org/c/osmo-bts/+/28086/comment/9f31b6f5_eb6b5008 PS1, Line 385: if (fn == 0) /* H0 block { 0, 2, 4, 6} */ As shared in another patch, switch sounds more clean here imho. Not important though. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/28086 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I8cc3a755a8ad4dc564439aab2298c1e97ac0592d Gerrit-Change-Number: 28086 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Tue, 10 May 2022 10:09:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment