Change in ...osmo-bsc[master]: WIP: rest_octets: add Serving Cell Priority Parameters

laforge gerrit-no-reply at lists.osmocom.org
Sun Jul 7 08:57:48 UTC 2019


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 <pmaier at sysmocom.de>
Gerrit-Reviewer: Hoernchen <ewild at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at gnumonks.org>
Gerrit-Comment-Date: Sun, 07 Jul 2019 08:57:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190707/6e0c6c4c/attachment.html>


More information about the gerrit-log mailing list