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=email : 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=email : PS5, Line 357: *! > about the doxygen format comment: […] Done https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/0fab90b5_12deb888?usp=email : 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=email : 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=email : 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=email : PS5, Line 1597: mian > typo Done https://gerrit.osmocom.org/c/osmo-msc/+/38490/comment/85d503e9_20c3c779?usp=email : 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 <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: neels <[email protected]> Gerrit-CC: pespin <[email protected]> Gerrit-Attention: neels <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Attention: pespin <[email protected]> Gerrit-Comment-Date: Tue, 28 Jan 2025 00:01:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <[email protected]> Comment-In-Reply-To: pespin <[email protected]>
