laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/14678 )
Change subject: WIP: rest_octets: add Serving Cell Priority Parameters ...................................................................... Patch Set 1: Code-Review-1 (3 comments) It's really good that you appear to have manged to uncover the mystery of UEs stickin to GSM in combined GSM+LTE setups. However, some stylistic comments below on bitvec API usage. https://gerrit.osmocom.org/#/c/14678/1/src/osmo-bsc/rest_octets.c File src/osmo-bsc/rest_octets.c: https://gerrit.osmocom.org/#/c/14678/1/src/osmo-bsc/rest_octets.c@198 PS1, Line 198: bitvec_set_bit(bv, 0); : bitvec_set_bit(bv, 0); : bitvec_set_bit(bv, 0); I suppose this is a 3bit field encoding an unsigned integer so I would encode it not as 3 individual bits but use bitvec_set_uint(bv, 0, 3); https://gerrit.osmocom.org/#/c/14678/1/src/osmo-bsc/rest_octets.c@203 PS1, Line 203: bitvec_set_bit(bv, 0); : bitvec_set_bit(bv, 0); : bitvec_set_bit(bv, 0); : bitvec_set_bit(bv, 0); same here. it's a 4-bit integer that should use bitvec_set_uint(bv, 0, 4); The advantage is that later on, if you want to use anything else but '0' you have one actual integer value in the code to change, rather than four individual bits. https://gerrit.osmocom.org/#/c/14678/1/src/osmo-bsc/rest_octets.c@208 PS1, Line 208: /* THRESH_GSM_low */ : bitvec_set_bit(bv, 0); : bitvec_set_bit(bv, 0); : bitvec_set_bit(bv, 0); : bitvec_set_bit(bv, 0); : : /* H_PRIO */ : bitvec_set_bit(bv, 0); : bitvec_set_bit(bv, 0); : : /* T_Reselection */ : bitvec_set_bit(bv, 0); : bitvec_set_bit(bv, 1); : same comment applies for these, I suppose -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/14678 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I7eaf7de4386fe8aea404e8a187d8a1f5ed596ead Gerrit-Change-Number: 14678 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <[email protected]> Gerrit-Reviewer: Hoernchen <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <[email protected]> Gerrit-Comment-Date: Sun, 07 Jul 2019 08:57:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
