On 03/01/2013 06:09 PM, Ben Chan wrote: > Yep, that should work. > Pushed that fix as well; thanks for reviewing it.
Cheers! > > On Fri, Mar 1, 2013 at 12:18 AM, Aleksander Morgado > <[email protected] <mailto:[email protected]>> wrote: > > On 03/01/2013 09:06 AM, Ben Chan wrote: > > > > > > > > On Thu, Feb 28, 2013 at 11:46 PM, Aleksander Morgado > > <[email protected] <mailto:[email protected]> > <mailto:[email protected] <mailto:[email protected]>>> wrote: > > > > On 02/28/2013 08:11 PM, Ben Chan wrote: > > > --- > > > src/mm-iface-modem-3gpp.c | 15 +++++++++++++++ > > > 1 file changed, 15 insertions(+) > > > > > > > I pushed it after modifying it to include the > > clear_deferred_registration_state_update() call in the first > > DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS step. As said > previously, we > > don't really mind if we get an unsolicited registration event > during the > > disabling process, as we're disabling anyway. > > > > > > I'm a bit confused. If an unsolicited registration event is received > > after clearing deferred_update_id, deferred_update_id is set to a new > > value, which will be scheduled after the modem is disabled. That will > > cause the modem state to become enabled due to the following code in > > mm-iface-modem-3gpp.c: > > > > mm_iface_modem_update_subsystem_state ( > > MM_IFACE_MODEM (self), > > SUBSYSTEM_3GPP, > > (new_state == MM_MODEM_3GPP_REGISTRATION_STATE_SEARCHING ? > > MM_MODEM_STATE_SEARCHING : > > MM_MODEM_STATE_ENABLED), > > MM_MODEM_STATE_CHANGE_REASON_UNKNOWN); > > > > Ah, good point; didn't consider that the deferred update could get > re-scheduled... In that case we should actually clear it *after* > DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS, once we know we > will not process those unsolicited events. How does this look? > > diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c > index 1d3fdce..6c8bb14 100644 > --- a/src/mm-iface-modem-3gpp.c > +++ b/src/mm-iface-modem-3gpp.c > @@ -1347,6 +1347,7 @@ typedef enum { > DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS, > DISABLING_STEP_DISABLE_UNSOLICITED_REGISTRATION_EVENTS, > DISABLING_STEP_CLEANUP_UNSOLICITED_REGISTRATION_EVENTS, > + DISABLING_STEP_CLEANUP_DEFERRED_REGISTRATION_UPDATE, > DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS, > DISABLING_STEP_DISABLE_UNSOLICITED_EVENTS, > DISABLING_STEP_REGISTRATION_STATE, > @@ -1419,8 +1420,6 @@ interface_disabling_step (DisablingContext *ctx) > case DISABLING_STEP_PERIODIC_REGISTRATION_CHECKS: > /* Disable periodic registration checks, if they were set */ > periodic_registration_check_disable (ctx->self); > - /* Prevent any deferred registration state update from > happening after the modem is disabled */ > - clear_deferred_registration_state_update (ctx->self); > /* Fall down to next step */ > ctx->step++; > > @@ -1462,6 +1461,12 @@ interface_disabling_step (DisablingContext *ctx) > /* Fall down to next step */ > ctx->step++; > > + case DISABLING_STEP_CLEANUP_DEFERRED_REGISTRATION_UPDATE: > + /* Prevent any deferred registration state update from > happening after the modem is disabled */ > + clear_deferred_registration_state_update (ctx->self); > + /* Fall down to next step */ > + ctx->step++; > + > case DISABLING_STEP_CLEANUP_UNSOLICITED_EVENTS: > if (MM_IFACE_MODEM_3GPP_GET_INTERFACE > (ctx->self)->cleanup_unsolicited_events && > MM_IFACE_MODEM_3GPP_GET_INTERFACE > (ctx->self)->cleanup_unsolicited_events_finish) { > > -- > Aleksander > > > > > _______________________________________________ > networkmanager-list mailing list > [email protected] > https://mail.gnome.org/mailman/listinfo/networkmanager-list > -- Aleksander _______________________________________________ networkmanager-list mailing list [email protected] https://mail.gnome.org/mailman/listinfo/networkmanager-list
