Re: [PATCH 4/8] call-forwarding: class applied is the class given by SS code

2012-08-23 Thread Philippe Nunes

Hi Denis,

On 08/23/2012 01:36 AM, Denis Kenzior wrote:

Hi Philippe,

On 08/22/2012 11:18 AM, Philippe Nunes wrote:

GCF test cases 31.2.1.6.1/2 are asking to make a query according a
specific
class.
The default class is applied when no class is specified in the SS code.
---
src/call-forwarding.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 91e34c6..7531f07 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -1012,7 +1012,7 @@ static void ss_set_query_cf_callback(const
struct ofono_error *error, int total,

static void ss_set_query_next_cf_cond(struct ofono_call_forwarding *cf)
{
- cf-driver-query(cf, cf-query_next, BEARER_CLASS_DEFAULT,
+ cf-driver-query(cf, cf-query_next, cf-ss_req-cls,
ss_set_query_cf_callback, cf);
}

@@ -1167,6 +1167,16 @@ static gboolean cf_ss_control(int type, const
char *sc,
return TRUE;
}

+ /*
+ * Some modems don't understand all classes very well, particularly
+ * the older models. So if the bearer class is the default, we
+ * just use the more commonly understood value of 7 since BEARER_SMS
+ * is not applicable to CallForwarding conditions according to 22.004
+ * Annex A
+ */
+ if (cls == BEARER_CLASS_SS_DEFAULT)
+ cls = BEARER_CLASS_DEFAULT;
+


Strictly speaking this is wrong. You cannot modify the cls here as it is
used to generate the reply in cf_ss_control_reply. Since
BEARER_CLASS_SS_DEFAULT != BEARER_CLASS_DEFAULT you will generate the
wrong result.



OK, but I don't understand why we intend to generate a reply including 
classes for which we don't know the status?


With BEARER_CLASS_SS_DEFAULT, we are considering the classes 
DataSync/DataAsync/Fax/Sms/Voice.


But by default, the query is asking the status only for 
BEARER_CLASS_DEFAULT (Data/Fax/Voice).

So, why, don't we match strictly the reply with the query?

Meanwhile, I send you a new patch.

Regards,

Philippe.



cf-ss_req-ss_type = type;
cf-ss_req-cf_type = cf_type;
cf-ss_req-cls = cls;
@@ -1188,16 +1198,6 @@ static gboolean cf_ss_control(int type, const
char *sc,
break;
}

- /*
- * Some modems don't understand all classes very well, particularly
- * the older models. So if the bearer class is the default, we
- * just use the more commonly understood value of 7 since BEARER_SMS
- * is not applicable to CallForwarding conditions according to 22.004
- * Annex A
- */
- if (cls == BEARER_CLASS_SS_DEFAULT)
- cls = BEARER_CLASS_DEFAULT;
-
switch (cf-ss_req-ss_type) {
case SS_CONTROL_TYPE_REGISTRATION:
string_to_phone_number(sia,ph);


Regards,
-Denis



___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 4/8] call-forwarding: class applied is the class given by SS code

2012-08-23 Thread Denis Kenzior

Hi Philippe,

snip


+ /*
+ * Some modems don't understand all classes very well, particularly
+ * the older models. So if the bearer class is the default, we
+ * just use the more commonly understood value of 7 since BEARER_SMS
+ * is not applicable to CallForwarding conditions according to 22.004
+ * Annex A
+ */
+ if (cls == BEARER_CLASS_SS_DEFAULT)
+ cls = BEARER_CLASS_DEFAULT;
+


Strictly speaking this is wrong. You cannot modify the cls here as it is
used to generate the reply in cf_ss_control_reply. Since
BEARER_CLASS_SS_DEFAULT != BEARER_CLASS_DEFAULT you will generate the
wrong result.



OK, but I don't understand why we intend to generate a reply including
classes for which we don't know the status?

With BEARER_CLASS_SS_DEFAULT, we are considering the classes
DataSync/DataAsync/Fax/Sms/Voice.


For some bizarre reason 22.030 specifies that all bearers are queried in 
the case that the mmi service code is omitted.  That means SMS as well. 
 However, half the bearers are not even applicable to call forwarding.


When reporting the results we report all bearers that were queried, even 
if some of them are not applicable or not known.  This is because we do 
not want to require the calling application to have any intelligence in 
this area.  It should simply display the results as given by oFono.




But by default, the query is asking the status only for
BEARER_CLASS_DEFAULT (Data/Fax/Voice).
So, why, don't we match strictly the reply with the query?


We do, but querying with class is broken on many modems, hence the 
comment and why we optimize this case.  If we pass in an explicit mmi 
service code then that is what will be used instead.


If your question is why we always query with the default, the answer is 
why not?  Default literally means Voice, Data, Fax.  Where Data is 
equivalent to DataAsync | DataSync in 27.007.  So in effect we get a 
full set of results and we return only the applicable set to the 
application.  Just because the GCF testing methodology is broken in this 
area does not mean that this approach is incorrect.


Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


[PATCH 4/8] call-forwarding: class applied is the class given by SS code

2012-08-22 Thread Philippe Nunes
GCF test cases 31.2.1.6.1/2 are asking to make a query according a specific
class.
The default class is applied when no class is specified in the SS code.
---
 src/call-forwarding.c |   22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 91e34c6..7531f07 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -1012,7 +1012,7 @@ static void ss_set_query_cf_callback(const struct 
ofono_error *error, int total,
 
 static void ss_set_query_next_cf_cond(struct ofono_call_forwarding *cf)
 {
-   cf-driver-query(cf, cf-query_next, BEARER_CLASS_DEFAULT,
+   cf-driver-query(cf, cf-query_next, cf-ss_req-cls,
ss_set_query_cf_callback, cf);
 }
 
@@ -1167,6 +1167,16 @@ static gboolean cf_ss_control(int type, const char *sc,
return TRUE;
}
 
+   /*
+* Some modems don't understand all classes very well, particularly
+* the older models.  So if the bearer class is the default, we
+* just use the more commonly understood value of 7 since BEARER_SMS
+* is not applicable to CallForwarding conditions according to 22.004
+* Annex A
+*/
+   if (cls == BEARER_CLASS_SS_DEFAULT)
+   cls = BEARER_CLASS_DEFAULT;
+
cf-ss_req-ss_type = type;
cf-ss_req-cf_type = cf_type;
cf-ss_req-cls = cls;
@@ -1188,16 +1198,6 @@ static gboolean cf_ss_control(int type, const char *sc,
break;
}
 
-   /*
-* Some modems don't understand all classes very well, particularly
-* the older models.  So if the bearer class is the default, we
-* just use the more commonly understood value of 7 since BEARER_SMS
-* is not applicable to CallForwarding conditions according to 22.004
-* Annex A
-*/
-   if (cls == BEARER_CLASS_SS_DEFAULT)
-   cls = BEARER_CLASS_DEFAULT;
-
switch (cf-ss_req-ss_type) {
case SS_CONTROL_TYPE_REGISTRATION:
string_to_phone_number(sia, ph);
-- 
1.7.9.5

___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono


Re: [PATCH 4/8] call-forwarding: class applied is the class given by SS code

2012-08-22 Thread Denis Kenzior

Hi Philippe,

On 08/22/2012 11:18 AM, Philippe Nunes wrote:

GCF test cases 31.2.1.6.1/2 are asking to make a query according a specific
class.
The default class is applied when no class is specified in the SS code.
---
  src/call-forwarding.c |   22 +++---
  1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/call-forwarding.c b/src/call-forwarding.c
index 91e34c6..7531f07 100644
--- a/src/call-forwarding.c
+++ b/src/call-forwarding.c
@@ -1012,7 +1012,7 @@ static void ss_set_query_cf_callback(const struct 
ofono_error *error, int total,

  static void ss_set_query_next_cf_cond(struct ofono_call_forwarding *cf)
  {
-   cf-driver-query(cf, cf-query_next, BEARER_CLASS_DEFAULT,
+   cf-driver-query(cf, cf-query_next, cf-ss_req-cls,
ss_set_query_cf_callback, cf);
  }

@@ -1167,6 +1167,16 @@ static gboolean cf_ss_control(int type, const char *sc,
return TRUE;
}

+   /*
+* Some modems don't understand all classes very well, particularly
+* the older models.  So if the bearer class is the default, we
+* just use the more commonly understood value of 7 since BEARER_SMS
+* is not applicable to CallForwarding conditions according to 22.004
+* Annex A
+*/
+   if (cls == BEARER_CLASS_SS_DEFAULT)
+   cls = BEARER_CLASS_DEFAULT;
+


Strictly speaking this is wrong.  You cannot modify the cls here as it 
is used to generate the reply in cf_ss_control_reply.  Since 
BEARER_CLASS_SS_DEFAULT != BEARER_CLASS_DEFAULT you will generate the 
wrong result.



cf-ss_req-ss_type = type;
cf-ss_req-cf_type = cf_type;
cf-ss_req-cls = cls;
@@ -1188,16 +1198,6 @@ static gboolean cf_ss_control(int type, const char *sc,
break;
}

-   /*
-* Some modems don't understand all classes very well, particularly
-* the older models.  So if the bearer class is the default, we
-* just use the more commonly understood value of 7 since BEARER_SMS
-* is not applicable to CallForwarding conditions according to 22.004
-* Annex A
-*/
-   if (cls == BEARER_CLASS_SS_DEFAULT)
-   cls = BEARER_CLASS_DEFAULT;
-
switch (cf-ss_req-ss_type) {
case SS_CONTROL_TYPE_REGISTRATION:
string_to_phone_number(sia,ph);


Regards,
-Denis
___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono