phablet-extras

Code review comment for lp:~awe/phablet-extras/ofono-lp1204644

ofono-lp1204644
Merge into ofono

Revision history for this message

Tony Espy (awe) wrote on 2013-07-27:

I'm usually accused of "over-commenting" code, and in this case I failed... Let me try and explain, and I will add more comments to the code as well.

RIL offers no control whatsoever of the GPRS 'attached' state. It's automatic, unless of course you you switch the modem into manual network selection mode, or power the modem off.

Prior to this change, it was impossible to cause GPRS to go offline by setting the ConnectionManager's 'Powered' property to false. The core gprs codde ( /src/gprs.c ) would call the driver's 'set_attached' function which in our case always immediately invoked the callback with a success return code. This in turn caused the core code to invoke the driver's 'attached_status' function to see if the attach/detach operation succeeded. In the disable GPRS case, the core code has called 'set_attached' with 'attached=0', however the status function would then return the actual state which is still attached. This causes a tight loop, as ofono would again tries to disable GPRS, ad infinitum...

So, what I've done is borrowed a trick from ofono itself, and handled this problem similar to the way ofono handles roaming. 'gprs_data.ofono_attached' is onfono's desired attached state. If true, and the modem is actually attached, then our code returns true to the core. If it's false, then it essentially overrides the actual modem's state. Look at the function gprs_netreg_update() in /src/gprs.c to see how the roaming case is handled.

Think of the attached state as "use-able-for-data". When the upper layers see that the modem isn't attached, it prevents any data calls from being activated. If any calls ( note, we only support a single active call currently ) are active when 'Powered' is toggled, they'll be disconnected due to the core calling the gprs_context_driver's 'detach_shutdown' function.

Airplane mode will be handled slightly different in that ModemManager's 'Powered' property will be toggled as opposed to the ConnectionManager's property. That in turn will result in an actual call to RIL requesting the radio to be powered off. In the GPRS/ConnectionManager case, we're merely preventing any data usage; voice calls and non-MMS text messages still work.

I'm usually accused of "over-commenting" code, and in this case I failed...  Let me try and explain, and I will add more comments to the code as well.

RIL offers no control whatsoever of the GPRS 'attached' state.  It's automatic, unless of course you you switch the modem into manual network selection mode, or power the modem off.

Prior to this change, it was impossible to cause GPRS to go offline by setting the ConnectionManager's 'Powered' property to false.  The core gprs codde ( /src/gprs.c ) would call the driver's 'set_attached' function which in our case always immediately invoked the callback with a success return code.  This in turn caused the core code to invoke the driver's 'attached_status' function to see if the attach/detach operation succeeded.  In the disable GPRS case, the core code has called 'set_attached' with 'attached=0', however the status function would then return the actual state which is still attached.  This causes a tight loop, as ofono would again tries to disable GPRS, ad infinitum...

So, what I've done is borrowed a trick from ofono itself, and handled this problem similar to the way ofono handles roaming.  'gprs_data.ofono_attached' is onfono's desired attached state.  If true, and the modem is actually attached, then our code returns true to the core.  If it's false, then it essentially overrides the actual modem's state.  Look at the function gprs_netreg_update() in /src/gprs.c to see how the roaming case is handled.

Think of the attached state as "use-able-for-data".  When the upper layers see that the modem isn't attached, it prevents any data calls from being activated.  If any calls ( note, we only support a single active call currently ) are active when 'Powered' is toggled, they'll be disconnected due to the core calling the gprs_context_driver's 'detach_shutdown' function.

Airplane mode will be handled slightly different in that ModemManager's 'Powered' property will be toggled as opposed to the ConnectionManager's property.  That in turn will result in an actual call to RIL requesting the radio to be powered off.  In the GPRS/ConnectionManager case, we're merely preventing any data usage; voice calls and non-MMS text messages still work.

« Back to merge proposal