fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/21789 )
Change subject: Revert "rest_octets: fix encoding of 3G Early Classmark Sending Restriction" ...................................................................... Patch Set 1: Code-Review-2 (1 comment) https://gerrit.osmocom.org/c/libosmocore/+/21789/1/src/gsm/gsm48_rest_octets.c File src/gsm/gsm48_rest_octets.c: https://gerrit.osmocom.org/c/libosmocore/+/21789/1/src/gsm/gsm48_rest_octets.c@613 PS1, Line 613: early_cm_restrict_3g I think we need to agree on what do we expect from 'early_cm_restrict_3g = true' and 'early_cm_restrict_3g = false' first. IMHO, the field name itself is confusing, because it reads/sounds like 'to restrict'. In reality, 3GPP TS 44.018 defines this bit as "3G Early Classmark Sending Restriction", and H does not mean 'restrict'. Your patch makes the encoding function behave asymmetrically from osmo_gsm48_rest_octets_si3_encode(): if (bitvec_get_bit_high(&bv) == H) si3->early_cm_restrict_3g = 1; else si3->early_cm_restrict_3g = 0; which by the way means that we don't have sufficient unit test coverage :/ I think the fix that I merged back then is correct, and we should rather fix osmo-bsc: src/osmo-bsc/system_information.c:968: si_info.early_cm_restrict_3g = !bts->early_classmark_allowed_3g; i.e. there is need to inverse the value/meaning of 'early_classmark_allowed_3g'. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/21789 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0ee48d3240c62c4d2e15063b26da7a2a617f383e Gerrit-Change-Number: 21789 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pes...@sysmocom.de> Gerrit-Reviewer: Hoernchen <ew...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de> Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Comment-Date: Sat, 19 Dec 2020 20:29:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
