On 10/28/2010 08:32 AM, Mika.Liljeberg(a)nokia.com wrote:
> Patch has been applied, thanks. I did make a couple of minor tweaks
Thanks. A question regarding this particular tweak:
diff --git a/src/radio-settings.c b/src/radio-settings.c
index cb0a598..eb2a42d 100644
@@ -126,8 +126,7 @@ static void radio_set_fast_dormancy(struct ofono_radio_settings *rs,
const char *path = __ofono_atom_get_path(rs->atom);
dbus_bool_t value = enable;
- if ((rs->flags & RADIO_SETTINGS_FLAG_CACHED) &&
- rs->fast_dormancy == enable)
+ if (rs->fast_dormancy == enable)
The fundamental problem here is that there is only a single CACHED flag for multiple
properties, which may be modified individually. So, either you get extra signals or you
get too few. I checked the CACHED flag here because otherwise the following might happen:
Yes, I know. But this problem is present in every single atom. oFono
does not guarantee that every attribute is signaled when the atom is
1. client tries to GetProperties() and gets the "Operation already in progress"
2. client waits for PropertyChanged signal to get the FastDormancy value
3. signal never comes because the default value happens to match the one returned by the
driver and the signal is suppressed
In general I think that for interfaces where this can happen, the
likelihood is very low that it actually will in the real world.
Do note that I have had the same argument with myself off and on for the
past two years. So far this was never raised as an issue. If this ever
becomes a problem, we can fix it properly using an appropriate idiom.
I do agree that sending extra signals is bad but I think that not
sending a signal is even worse. If the client cannot rely on getting a PropertyChanged
signal after a busy error, all it can do is resort to polling. I.e., every client has to
implement a polling pattern for GetProperties:
while (GetProperties() == BUSY)
Having a separate CACHED flag for each value would solve this optimally. Failing that, I
don't think a few extra signals is so bad. Forcing clients to poll is just ugly.
Honestly, if you expect multiple applications to battle over the
FastDormancy property, then it should be modeled differently. Perhaps
with application registration and lifetime tracking over D-Bus, similar
to how agents work.
Am I missing something?