This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
Pau Espin Pedrol gerrit-no-reply at lists.osmocom.orgPatch Set 3: Code-Review-1 (5 comments) https://gerrit.osmocom.org/#/c/7784/3/src/libbsc/acc_ramp.c File src/libbsc/acc_ramp.c: Line 182: if (trx_is_usable(trx)) /* cross-check with operational state */ As discussed a few mins ago, when coming from S_NM_STATECHG_OPER signal in abis_nm_rx_statechg_rep, it can be that both operational and administrative status change at the same time (same call to this function). Case: admin LOCKED->UNLOCKED + oper DISABLED->ENABLED, we are not enabling ramping here because trx_is_usable() probably checks the current/old status which is still DISABLED. Line 190: break; we can probably removed this break to print an error too, unless NULL is really a valid state from the spec point of view (one to which we can change to). Line 201: if (trx->mo.nm_state.administrative == NM_STATE_UNLOCKED) Again, we need to check after the new one as it could have be different than the old/current one. Line 208: break; same, remove break? Line 222: if (trigger_ramping && !osmo_timer_pending(&acc_ramp->step_timer)) do we really need this osmo_timer_pending check here? This should never happen right? better move it inside the if() and add an OSMO_ASSERT() with it. -- To view, visit https://gerrit.osmocom.org/7784 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib3291439655598fb5ddc891a3e4cc35b0bad250f Gerrit-PatchSet: 3 Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de> Gerrit-HasComments: Yes