Hi,
2009/10/24 Denis Kenzior <[email protected]>:
>> > in set_registration_status:
>> > attached = (status != NETWORK_REGISTRATION_STATUS_REGISTERED &&
>> > status != NETWORK_REGISTRATION_STATUS_ROAMING);
>> > if (gprs->attached != (int) attached &&
>> > !(gprs->flags & GPRS_FLAG_ATTACHING)) {
>> > gprs->attached = (int) attached;
>> >
>> > ofono_dbus_signal_property_changed(conn, path,
>> > DATA_CONNECTION_MANAGER_INTERFACE,
>> > "Attached", DBUS_TYPE_BOOLEAN,
>> > &attached);
>> >
>> > gprs_netreg_update(gprs);
>> > }
>
> So the issue here is that I can actually understand what gprs_netreg_update
> does. This function I'm having trouble with.
>
> Lets suppose status=registered arrives and we're already attached. Here
> attached will be set to FALSE, Attached property will be set to false and
> emitted. gprs_netreg_update will presumably reset Attached back to True.
Now I see it and you're absolutely right to be alerted, I set
"attached" to the reverse of what it should be set to. We're attached
when we're either in REGISTERED or ROAMING state. If I had done it
right then the logic would look simple (I suppose):
If either we already knew that we were in that state then there's
nothing to do. Otherwise we set the D-Bus property to reflect current
state and call gprs_netreg_update to figure out if this is the state
we really wanted to be in.
If the notification indicates we were detached we will also need to
reset all current contexts to inactive here, originally I did this in
the driver.
>>
>> > gprs_netreg_update:
>> > /* Prevent further attempts to attach */
>> > if (!attach && gprs->powered) {
>> > gprs->powered = 0;
>> >
>> > path = __ofono_atom_get_path(gprs->atom);
>> > ofono_dbus_signal_property_changed(conn, path,
>> > DATA_CONNECTION_MANAGER_INTERFACE,
>> > "Powered", DBUS_TYPE_BOOLEAN,
>> > &value); }
>> >
>> > This is just too evil. Can't we set another flag that attached
>> > conditions have changed while we were attaching/detaching and that we
>> > should recheck those conditions when we return from attach/detach?
>>
>> Is it evil because we change a property that is writable?
>
> Yes, and also because this is a hack :)
>
>>
>> This is only for the case of RoamingAllowed = 0. In my understanding
>> "Powered" being set means that we're attached or attempting to attach,
>
> No, Powered means GPRS is on assuming all conditions are fulfilled (e.g.
> registration, roaming, network resources) This is a user setting that will be
> persisted (in IMSI based storage.) It should not be spuriously changed by the
> daemon.
This is a policy question, what do we do when the user disallowed
roaming but the modem wants to roam? If we decide that we will not
retry to attach until the user pokes us again then "Powered" really
should be reset to false because even if the conditions you listed are
met we still won't attach. Maybe instead we should do the retries
with increasing timeouts like you mentioned in another message, and
then "Powered" should stay set.
An unrelated thing: you keep the allowed cid range in the core and
only remember the min and max cid. However I think (but don't know
where to check for it) that +CGDCONT=? is free to return (1,2) instead
of (1-2), or even (1-2,5-6,13). Other AT commands do return ranges
with this syntax.
Regards
_______________________________________________
ofono mailing list
[email protected]
http://lists.ofono.org/listinfo/ofono