Attention is currently required from: osmith, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/34044 )
Change subject: rlcmac: Fix Starting Time rfn->fn calculation ...................................................................... Patch Set 6: Code-Review+1 (3 comments) File src/rlcmac/rlcmac_dec.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/34044/comment/169eaa6c_4c70e47b PS6, Line 437: t3 = tbf_start_time->N51 Are you sure this is correct? On the left (before this patch) `t3` field was assigned the value of `tbf_start_time->N26`, but now it's `tbf_start_time->N51`. Ah, now it matches what `gsm_fn2gsmtime()` is doing, so looks correct. Nevermind. https://gerrit.osmocom.org/c/libosmo-gprs/+/34044/comment/ba487d5a_6fce381f PS6, Line 439: 51 * OSMO_MOD_FLR((t3 - t2), 26) + t3 + 51 * 26 * t1p; AFAICS, this is pretty much a copy-paste from libosmogsm. Is there a reason to reinvent `gsm_gsmtime2fn()` here? Can't we keep the function as it was, and simply do `gsm_rfn2fn`? ``` const uint16_t rfn = gsm_gsmtime2fn(&g_time); const uint32_t fn = gsm_rfn2fn(rfn, curr_fn); ``` File tests/rlcmac/rlcmac_types_test.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/34044/comment/d0ec9690_54c93307 PS3, Line 54: printf > This allows easily separating internal logging from test specific logging. […] Not blocking, marking as resolved. -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/34044 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I00741289333853a8db472950ee2ac38dc2c74fd3 Gerrit-Change-Number: 34044 Gerrit-PatchSet: 6 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: osmith <[email protected]> Gerrit-Reviewer: pespin <[email protected]> Gerrit-Attention: osmith <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Comment-Date: Tue, 22 Aug 2023 17:53:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: osmith <[email protected]> Comment-In-Reply-To: pespin <[email protected]> Comment-In-Reply-To: fixeria <[email protected]> Gerrit-MessageType: comment
