Re: [PATCH] voicecall: fix callheld indicator for PTS

Hi Frédéric, On 07/18/2011 09:33 AM, Frédéric Danis wrote: > Fix PTS test TP/TWC/BV-03-I [Call Waiting- Hold Active/Retrieve > Waiting Call or Held]. > > PTS test fails after receiving intermediate update of callheld indicator > with value 0 (no held call) before it receives update to value 1 > (active and held calls). This is due to the non-atomic update of calls > status after call swap (moving first call to active state before moving > second one to hold state). Which scenario fails exactly? -<1-N> Active + Waiting -> Active + Held -<1-N> Active + 1 Held -> 1 Active +<1-N> Held - 1 Active +<1-N> Held -> <1-N> Active + 1 Held Also note that you can't really count on the order of the updates, these are completely up to the implementation.

> > HFP 1.5 spec specifies that an update of callheld indicator to 1 should > be sent after AT+CHLD=2 command. > As oFono emulator sends +CIEV only if the indicator value changes, we > need to use an intermediate state for callheld indicator (2, all calls on > hold). > > So, in case of multiple active calls, or an active call with an active > mutiparty call, force update of callheld indicator to 2. > --- > src/voicecall.c | 18 ++++++++++++++---- > 1 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/src/voicecall.c b/src/voicecall.c > index 3646951..4b8373e 100644 > --- a/src/voicecall.c > +++ b/src/voicecall.c > @@ -733,7 +733,8 @@ static void emulator_callheld_status_cb(struct ofono_atom *atom, void *data) > static void notify_emulator_call_status(struct ofono_voicecall *vc) > { > struct ofono_modem *modem = __ofono_atom_get_modem(vc->atom); > - gboolean call = FALSE; > + unsigned int non_mpty = 0; > + gboolean multiparty = FALSE; > gboolean held = FALSE; > gboolean incoming = FALSE; > gboolean dialing = FALSE; > @@ -750,7 +751,12 @@ static void notify_emulator_call_status(struct ofono_voicecall *vc) > > switch (v->call->status) { > case CALL_STATUS_ACTIVE: > - call = TRUE; > + if (g_slist_find_custom(vc->multiparty_list, > + GINT_TO_POINTER(v->call->id), > + call_compare_by_id)) > + multiparty = TRUE; > + else > + non_mpty++; > break; What is this trying to achieve? You cannot have multiple calls in active / held state unless they are part of the mpty. So if you have 3 active calls, all 3 must be on the mpty list.

> > case CALL_STATUS_HELD: > @@ -775,7 +781,8 @@ static void notify_emulator_call_status(struct ofono_voicecall *vc) > } > } > > - data.status = call || held ? OFONO_EMULATOR_CALL_ACTIVE : > + data.status = non_mpty || multiparty || held ? > + OFONO_EMULATOR_CALL_ACTIVE : > OFONO_EMULATOR_CALL_INACTIVE; > > __ofono_modem_foreach_registered_atom(modem, > @@ -799,8 +806,11 @@ static void notify_emulator_call_status(struct ofono_voicecall *vc) > &data); > > if (held) > - data.status = call ? OFONO_EMULATOR_CALLHELD_MULTIPLE : > + data.status = (non_mpty || multiparty) ? > + OFONO_EMULATOR_CALLHELD_MULTIPLE : > OFONO_EMULATOR_CALLHELD_ON_HOLD; > + else if (non_mpty> 1 || (non_mpty&& multiparty)) > + data.status = OFONO_EMULATOR_CALLHELD_ON_HOLD; > else > data.status = OFONO_EMULATOR_CALLHELD_NONE; > I think a different approach is needed. One idea off the top of my head: Can we mark calls which we foresee being as part of the transaction and only send the status updates once all of them have entered the desired state.

Regards, -Denis