Attention is currently required from: jolly, pespin.

fixeria has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-bsc/+/34626?usp=email )

Change subject: ASCI: Add System Information 10 support
......................................................................


Patch Set 2:

(6 comments)

Patchset:

PS2:
> I also don't like the timer. […]
SI10 is actually not needed for the call establishment, it's needed for quicker 
cell change. Without SI10 a phone participating a group call would need to 
first tune to BCCH of each neighbor to see if the current group call is also 
ongoing there or not -- this would cause a gap in the call.

IIUC, the payload of SI10 is expected to remain static as long as a) there are 
no neighbor list changes, b) no ongoing group calls being released, and c) no 
new group calls being established. Maybe we can trigger SI10 generation and 
sending if a), b), or c) happens?


File src/osmo-bsc/system_information.c:

https://gerrit.osmocom.org/c/osmo-bsc/+/34626/comment/abfe25bf_b72c4afd
PS2, Line 1420: struct gsm_subscriber_connection *conn
should be `const`


https://gerrit.osmocom.org/c/osmo-bsc/+/34626/comment/b988a2f5_69d6cfbe
PS2, Line 1424: *n_bts, *l_bts
cosmetic: better declare these two separately, so they're not hidden behind the 
`s_bts` assignment.  Wait, they're only used within the for-loop? Then they 
should be moved there.


https://gerrit.osmocom.org/c/osmo-bsc/+/34626/comment/7c9d144a_cda5d3da
PS2, Line 1428: unsigned int save_cur_bit;
              :         struct gsm_subscriber_connection *c;
both vars can be moved to the scope of use (for-loop)


https://gerrit.osmocom.org/c/osmo-bsc/+/34626/comment/7983ba8f_7fe8cfe7
PS2, Line 1447: 32
where this limit is coming from?
is there a macro?  if not, at least add a comment?


https://gerrit.osmocom.org/c/osmo-bsc/+/34626/comment/668a1787_e10481d1
PS2, Line 1499:         /* Do spare padding. We cannot do it earlier, because 
encoding might corrupt it if differenctial cell info
> the same "differential cell info" mentioned a few lines above I guess...
Indeed, SI10 employs so-called differential encoding: you encode all the info 
for the first BTS and then only those parameters that differ for the remaining 
BTSs.



--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34626?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Icd3101e6dd935a57f003253aaef400c2cf95a0c3
Gerrit-Change-Number: 34626
Gerrit-PatchSet: 2
Gerrit-Owner: jolly <andr...@eversberg.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de>
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-CC: pespin <pes...@sysmocom.de>
Gerrit-Attention: jolly <andr...@eversberg.eu>
Gerrit-Attention: pespin <pes...@sysmocom.de>
Gerrit-Comment-Date: Wed, 04 Oct 2023 19:06:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <lafo...@osmocom.org>
Comment-In-Reply-To: pespin <pes...@sysmocom.de>
Gerrit-MessageType: comment

Reply via email to