On 03/02/2018 09:29 AM, Jonas Bonn wrote:
On 03/01/2018 05:11 PM, Denis Kenzior wrote:
> Hi Jonas,
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
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
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?