Hi Jonas,

On 03/02/2018 09:29 AM, Jonas Bonn wrote:
On 03/01/2018 05:11 PM, Denis Kenzior wrote:
Hi Jonas,


Hi Denis,

I've spent most of the day trying to make sense of this.  I'll send some new logs when I get a chance but I thought I'd just ask a couple of questions that popped up during the day:

i)  For LTE, does it really make sense for the Attached property to be set before the context is Active?  The two are intricately linked.

So in general the API does not guarantee any particular ordering of messages, and the client should be able to handle any message order. We make exceptions to this where a particular order makes implementation easy. For example, ConnectionContext.Settings is signaled prior to ConnectionContext.Active.

We set Attached mostly not to confuse ConnMan. Since ConnMan doesn't know about LTE, we tried to preserve the overall behavior.

From an API consistency perspective & conceptually, I think setting Attached before ConnectionContext.Active makes sense. Having said that, if there's consensus from the connman guys that signaling Active then Attached for this particular case, we can make the change...


ii)  For LTE, I'm not sure we should call ofono_gprs_status_notify() when we are registered to the network.  Instead, I suspect we should just wait until cid_activated() gets called.  Is that sufficient?  It looks to be, mostly, in my eyes and didn't break terribly when I tried it.

There might be some weirdness in the core around this area actually. Looking at src/gprs.c it seems like we expect ofono_gprs_cid_activated to be called prior to ofono_gprs_status_notify. We probably need to introduce some checks in ofono_gprs_status_notify in case the call order is switched.

When in LTE this status should be largely ignored anyhow. We can't 'detach' in case we're roaming, so much of the logic doesn't apply.


iii)  Looking at src/gprs, couldn't we just set gprs->attached and gprs->driver_attached right away if we are registered on an LTE network.  It seems that conceptually that would be true...???  If we do that, I think we can drop most of the _ATTACHING flags which might make the code easier to understand.

Possibly, though you may be a bit optimistic that any of this can be made simpler :)

Right now we always set the _ATTACHING flag inside ofono_gprs_cid_activated. This isn't really correct, we should only do so on EUTRAN and if this is the default bearer activation (e.g. we think we're not attached yet). The network can automatically activate contexts on 3G or even LTE beyond the initial one.

This logic then proceeds to signal Attached=True when .read_settings returns. There are probably some extra checks we need to put into pri_read_settings_callback to make sure we only signal Attached on the initial default bearer activation.


iv)  Even if we do iii), I'm inclined to not set the Attached DBUS property until _cid_activated() returns, and then _after_ context Active instead of before.

The code already tries to do the first part, e.g. querying the default bearer settings and then signaling Attached & Active in that order.


v)  If we detach from the network, do things break conceptually if we set Attached=false before setting context Active=false?

Again, conceptually I would say Active=false then Attached=false makes the most sense.


A lot of the above comes from my understanding of what connman is trying to do:  if it sees the state as Attached and has no Active contexts then it starts trying to activate the first context.  Ideally, it would never see an inactive context for LTE, at least not in the Attached state. However, I may be missing something in my understanding of how this is intended to work, so any clarification would be appreciated.

Or it should give up if it sees the InProgress error, but yes it might be that re-ordering messages is the right approach. Daniel, Marcel?

Regards,
-Denis
_______________________________________________
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono

Reply via email to