[MERGED] osmo-bsc[master]: trigger acc ramping on state-changed-event reports
Harald Welte has submitted this change and it was merged. Change subject: trigger acc ramping on state-changed-event reports .. trigger acc ramping on state-changed-event reports Trigger ACC ramping not only when an Administrative State Change ACK is received from a BTS, but also when an administrative state change is reported for TRX 0 in a State Changed Event Report. This should allow ACC ramping to work with any BTS which reports an administrative state change to 'unlock' using either of these OML messages. Tested with a sysmobts and a nanobts. The sysmobts only reports TRX locked/unlock changes in Administrative State Change ACKs, not via State Changed Event Reports. The nanobts is known to send both of these OML messages in quick succession, so do not re-trigger ramping if it's already in progress. Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176 Related: OS#2591 --- M src/libbsc/acc_ramp.c 1 file changed, 10 insertions(+), 2 deletions(-) Approvals: Harald Welte: Looks good to me, approved Jenkins Builder: Verified diff --git a/src/libbsc/acc_ramp.c b/src/libbsc/acc_ramp.c index 6d9be59..16bce3f 100644 --- a/src/libbsc/acc_ramp.c +++ b/src/libbsc/acc_ramp.c @@ -144,7 +144,8 @@ struct acc_ramp *acc_ramp = handler_data; struct gsm_bts_trx *trx = NULL; - if (signal != S_NM_STATECHG_ADM) + /* Handled signals map to an Administrative State Change ACK, or a State Changed Event Report. */ + if (signal != S_NM_STATECHG_ADM && signal != S_NM_STATECHG_OPER) return 0; if (nsd->obj_class != NM_OC_RADIO_CARRIER) @@ -163,7 +164,14 @@ /* Trigger or abort ACC ramping based on the new 'RF lock' state of this TRX. */ switch (nsd->new_state->administrative) { case NM_STATE_UNLOCKED: - acc_ramp_trigger(acc_ramp); + /* +* Do not re-trigger ACC ramping if ramping is already in progress. +* A BTS might send several "unlock" change events: One in the Administrative +* State Change ACK, and/or another in a State Changed Event Report. +* For instance, the nanobts is known to send both. +*/ + if (!osmo_timer_pending(_ramp->step_timer)) + acc_ramp_trigger(acc_ramp); break; case NM_STATE_LOCKED: case NM_STATE_SHUTDOWN: -- To view, visit https://gerrit.osmocom.org/7770 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176 Gerrit-PatchSet: 2 Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Owner: Stefan SperlingGerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Stefan Sperling
osmo-bsc[master]: trigger acc ramping on state-changed-event reports
Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/7770 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176 Gerrit-PatchSet: 2 Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Owner: Stefan SperlingGerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Stefan Sperling Gerrit-HasComments: No
[PATCH] osmo-bsc[master]: trigger acc ramping on state-changed-event reports
Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/7770 to look at the new patch set (#2). trigger acc ramping on state-changed-event reports Trigger ACC ramping not only when an Administrative State Change ACK is received from a BTS, but also when an administrative state change is reported for TRX 0 in a State Changed Event Report. This should allow ACC ramping to work with any BTS which reports an administrative state change to 'unlock' using either of these OML messages. Tested with a sysmobts and a nanobts. The sysmobts only reports TRX locked/unlock changes in Administrative State Change ACKs, not via State Changed Event Reports. The nanobts is known to send both of these OML messages in quick succession, so do not re-trigger ramping if it's already in progress. Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176 Related: OS#2591 --- M src/libbsc/acc_ramp.c 1 file changed, 10 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/70/7770/2 diff --git a/src/libbsc/acc_ramp.c b/src/libbsc/acc_ramp.c index 6d9be59..16bce3f 100644 --- a/src/libbsc/acc_ramp.c +++ b/src/libbsc/acc_ramp.c @@ -144,7 +144,8 @@ struct acc_ramp *acc_ramp = handler_data; struct gsm_bts_trx *trx = NULL; - if (signal != S_NM_STATECHG_ADM) + /* Handled signals map to an Administrative State Change ACK, or a State Changed Event Report. */ + if (signal != S_NM_STATECHG_ADM && signal != S_NM_STATECHG_OPER) return 0; if (nsd->obj_class != NM_OC_RADIO_CARRIER) @@ -163,7 +164,14 @@ /* Trigger or abort ACC ramping based on the new 'RF lock' state of this TRX. */ switch (nsd->new_state->administrative) { case NM_STATE_UNLOCKED: - acc_ramp_trigger(acc_ramp); + /* +* Do not re-trigger ACC ramping if ramping is already in progress. +* A BTS might send several "unlock" change events: One in the Administrative +* State Change ACK, and/or another in a State Changed Event Report. +* For instance, the nanobts is known to send both. +*/ + if (!osmo_timer_pending(_ramp->step_timer)) + acc_ramp_trigger(acc_ramp); break; case NM_STATE_LOCKED: case NM_STATE_SHUTDOWN: -- To view, visit https://gerrit.osmocom.org/7770 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176 Gerrit-PatchSet: 2 Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Owner: Stefan SperlingGerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Stefan Sperling
osmo-bsc[master]: trigger acc ramping on state-changed-event reports
Patch Set 1: > Please also test with osmo-bts A sysmobts reports TRX lock/unlock only via Change Administrative State ACK. Its State Changed Event Reports only contain information about operational and availability states (as we would expect from reading the GSM spec). Unlike the nanobts, a sysmobts does not send a State Changed event Report when trx 0 is locked or unlocked. It looks like these messages do not trigger for administrative state changes. Regardless, ACC ramping still works as expected with a sysmobts. -- To view, visit https://gerrit.osmocom.org/7770 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176 Gerrit-PatchSet: 1 Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Owner: Stefan SperlingGerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: Stefan Sperling Gerrit-HasComments: No
osmo-bsc[master]: trigger acc ramping on state-changed-event reports
Patch Set 1: Please also test with osmo-bts -- To view, visit https://gerrit.osmocom.org/7770 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176 Gerrit-PatchSet: 1 Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Owner: Stefan SperlingGerrit-Reviewer: Harald Welte Gerrit-Reviewer: Jenkins Builder Gerrit-HasComments: No
[PATCH] osmo-bsc[master]: trigger acc ramping on state-changed-event reports
Review at https://gerrit.osmocom.org/7770 trigger acc ramping on state-changed-event reports Trigger ACC ramping not only when an Administrative State Change ACK is received from a BTS, but also when an administrative state change is reported for TRX 0 in a State Changed Event Report. This should allow ACC ramping to work with any BTS which reports an administrative state change to 'unlock' using either of these OML messages. Tested with a nanobts. The nanobts is known to send both of these OML messages in quick succession, so do not re-trigger ramping if it's already in progress. Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176 Related: OS#2591 --- M src/libbsc/acc_ramp.c 1 file changed, 10 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/70/7770/1 diff --git a/src/libbsc/acc_ramp.c b/src/libbsc/acc_ramp.c index 54626ad..9e66f31 100644 --- a/src/libbsc/acc_ramp.c +++ b/src/libbsc/acc_ramp.c @@ -144,7 +144,8 @@ struct acc_ramp *acc_ramp = handler_data; struct gsm_bts_trx *trx = NULL; - if (signal != S_NM_STATECHG_ADM) + /* Handled signals map to an Administrative State Change ACK, or a State Changed Event Report. */ + if (signal != S_NM_STATECHG_ADM && signal != S_NM_STATECHG_OPER) return 0; if (nsd->obj_class != NM_OC_RADIO_CARRIER) @@ -163,7 +164,14 @@ /* Trigger or abort ACC ramping based on the new 'RF lock' state of this TRX. */ switch (nsd->new_state->administrative) { case NM_STATE_UNLOCKED: - acc_ramp_trigger(acc_ramp); + /* +* Do not re-trigger ACC ramping if ramping is already in progress. +* A BTS might send several "unlock" change events: One in the Administrative +* State Change ACK, and/or another in a State Changed Event Report. +* For instance, the nanobts is known to send both. +*/ + if (!osmo_timer_pending(_ramp->step_timer)) + acc_ramp_trigger(acc_ramp); break; case NM_STATE_LOCKED: case NM_STATE_SHUTDOWN: -- To view, visit https://gerrit.osmocom.org/7770 To unsubscribe, visit https://gerrit.osmocom.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I097a113a3a63de02bcb8277020beb59cf485b176 Gerrit-PatchSet: 1 Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Owner: Stefan Sperling