Attention is currently required from: pespin.
dexter has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-pcu/+/33538
)
Change subject: nacc_fsm: Add support for NACC with UTRAN and E-UTRAN cells
......................................................................
Patch Set 3:
(6 comments)
File src/nacc_fsm.c:
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/859ae223_965633d8
PS2, Line 273: /* Classify the RAT type of the cell that is proposed in the
PacketCellChangeNotification. In case the RAT is GERAN,
Can you maybe convert these defines to an enum and
provide spec reference on where to find this exac […]
(I came up with this, those
values are just return codes. The spec encodes the RAT type in this weird tree
structure.)
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/e56f75ff_3b3d2ee5
PS2, Line 278: static int fill_neigh_key_from_bts_pkt_cell_chg_not(struct nacc_fsm_ctx
*ctx,
I actually think you don't need to return 0/1/2
here, see below.
Done
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/867fef4a_24da7415
PS2, Line 344: return NACC_EUTRAN_CELL;
maybe set ctx->neigh_key_valid = false explicitly
for readers to understand better (and to make sure […]
Done
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/242020b6_9df15c10
PS2, Line 398: } else if (rc == NACC_UTRAN_CELL || rc == NACC_EUTRAN_CELL) {
I think in here what you may actually want to do is:
[…]
I think this makes sense. This also simplifies
fill_neigh_key_from_bts_pkt_cell_chg_not() and we can check ctx->neigh_key_valid (now
renamed to ctx->neigh_key_present). No neigh_key_present + return code 0 => no
system information provided.
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/dcc28292_249da948
PS2, Line 433: else if (rc == NACC_UTRAN_CELL || rc == NACC_EUTRAN_CELL) {
see same as above.
Done
File src/neigh_cache.h:
https://gerrit.osmocom.org/c/osmo-pcu/+/33538/comment/e5b0e89f_012028c9
PS2, Line 68: /* TODO: This function seems to be used only in neigh_cache.c, make it
static? */
this is definetly another patch. Fine with making it
static.
Done
--
To view, visit
https://gerrit.osmocom.org/c/osmo-pcu/+/33538
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I96280f0ec5955ed3cb17641bf4118496c929bdac
Gerrit-Change-Number: 33538
Gerrit-PatchSet: 3
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 03 Jul 2023 10:55:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment