28 Jan
2025
28 Jan
'25
12:01 a.m.
Attention is currently required from: laforge, neels, pespin.
lynxis lazus has posted comments on this change by lynxis lazus. (
https://gerrit.osmocom.org/c/osmo-msc/+/38490?usp=email )
Change subject: vlr: add PS support
......................................................................
Patch Set 5:
(7 comments)
File src/libvlr/vlr.c:
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/af3cff48_9c583ff3?usp… :
PS5, Line 1490: return osmo_fsm_inst_dispatch(vsub->lu_fsm,
can go in same line.
Done
File src/libvlr/vlr_lu_fsm.c:
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/3759abb5_720d31c8?usp… :
PS5, Line 357: *!
about the doxygen format comment: […]
Done
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/0fab90b5_12deb888?usp… :
PS5, Line 589: LOGPFSML(fi, LOGL_ERROR, "LU Compl timeout %d / %d\n", fi->T,
lcvp->N);
let's have "T%d / N%d"
Done
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/c79d807a_bbd82098?usp… :
PS5, Line 598: break;
(we often have a default: OSMO_ASSERT(false) in these
to ensure we notice enum bugs)
in timeouts?
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/c3348b61_547ac77a?usp… :
PS5, Line 771: /*! count times timer T timed out */
.
I would like to replace the words
"count" and "out" with a word starting with t.
Any ideas?
Not sure if this comment can be more cleared, because the spec depends on this knowledge
anyways.
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/e6dfe964_6b4c48b9?usp… :
PS5, Line 1597: mian
typo
Done
https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/85d503e9_20c3c779?usp… :
PS5, Line 1611: fi->T
technically, middle arg should be 0 or -1 or so,
because vlr_is_cs() == false from above. […]
In this case, you could also pass -1 to
it.
We could use `osmo_tdef_get(vlr_tdefs, ps_timer, OSMO_TDEF_S, 0);`, not sure if it makes
it clearer the code instead of using vlr_timer_secs() as we did in all other places.
vlr_timer_secs() is usually used by fsm_state_chg() and you pass the CS and PS timer
values of the state into it, because CS has different Ts than PS.
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/38490?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: Ie9ffeb140c9d354b3a0f4822e2619f623235add0
Gerrit-Change-Number: 38490
Gerrit-PatchSet: 5
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 28 Jan 2025 00:01:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>