[PATCH] sms: fix send sms in case of lte registration

2018-09-10 Thread Anirudh Gargi
CREG status 6 and 7 added in network registration status, sms atom
to consider new states also.
---
 src/common.h | 14 --
 src/sms.c|  2 ++
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/common.h b/src/common.h
index 1b6b01d..b826228 100644
--- a/src/common.h
+++ b/src/common.h
@@ -37,12 +37,14 @@ enum access_technology {
 
 /* 27.007 Section 7.2  */
 enum network_registration_status {
-   NETWORK_REGISTRATION_STATUS_NOT_REGISTERED =0,
-   NETWORK_REGISTRATION_STATUS_REGISTERED =1,
-   NETWORK_REGISTRATION_STATUS_SEARCHING = 2,
-   NETWORK_REGISTRATION_STATUS_DENIED =3,
-   NETWORK_REGISTRATION_STATUS_UNKNOWN =   4,
-   NETWORK_REGISTRATION_STATUS_ROAMING =   5,
+   NETWORK_REGISTRATION_STATUS_NOT_REGISTERED =0,
+   NETWORK_REGISTRATION_STATUS_REGISTERED =1,
+   NETWORK_REGISTRATION_STATUS_SEARCHING = 2,
+   NETWORK_REGISTRATION_STATUS_DENIED =3,
+   NETWORK_REGISTRATION_STATUS_UNKNOWN =   4,
+   NETWORK_REGISTRATION_STATUS_ROAMING =   5,
+   NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN = 6,
+   NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN =7,
 };
 
 /* 27.007 Section 7.3  */
diff --git a/src/sms.c b/src/sms.c
index b86158e..c604e05 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -782,6 +782,8 @@ static void netreg_status_watch(int status, int lac, int 
ci, int tech,
switch (status) {
case NETWORK_REGISTRATION_STATUS_REGISTERED:
case NETWORK_REGISTRATION_STATUS_ROAMING:
+   case NETWORK_REGISTRATION_STATUS_REGISTERED_SMS_EUTRAN:
+   case NETWORK_REGISTRATION_STATUS_ROAMING_SMS_EUTRAN:
sms->registered = TRUE;
break;
default:
-- 
2.7.4

___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono


Re: [PATCH] atmodem: add LTE state for AT+CREG

2018-09-10 Thread Denis Kenzior

Hi Slava,

On 09/10/2018 02:56 PM, Slava Monich wrote:

Hi Denis,

Hi Slava,

On 09/10/2018 01:26 PM, Slava Monich wrote:

Hi Anirudh, Denis at al.

@@ -669,6 +669,10 @@ const char *registration_status_to_string(int 
status)

  return "unknown";
  case NETWORK_REGISTRATION_STATUS_ROAMING:
  return "roaming";
+    case NETWORK_REGISTRATION_STATUS_REGISTERED_EUTRAN:
+    return "registered lte";
+    case NETWORK_REGISTRATION_STATUS_ROAMING_EUTRAN:
+    return "roaming lte";


What would be the difference between "roaming lte" and "roaming" (or 
"registered lte" vs "registered") from the viewpoint of D-Bus API 
clients? In other words, what would e.g. dialer do differently 
depending on whether registration status is "registered lte" or just 
"registered"? Is it worth the API break? The existing clients won't 
know how to handle the new values until they (clients) get updated.




I agree.  That is why I suggested a separate Property for this (and 
the CSFB case from 27.007) instead of modifying the Status property 
directly.


E.g. REGISTERED_SMS_EUTRAN would map to Status="registered" and 
SmsOnly=False.  ROAMING_SMS_EUTRAN would map to Status="roaming" and 
SmsOnly=True.


I assume that the Dialer can ignore this as the modem will fall back 
to 3G in case of a call?  Someone feel free to educate me.


The modems I dealt with do drop to 3G for voice calls. Once voice call 
terminates, they switch back to LTE (well, most of the time). Dialler 
(at least our dialler) doesn't care about access technology.


That is what I've seen as well.  But these are pre-VoLTE devices most 
likely.  VoLTE might act differently.




Just a thought - could the presence of org.ofono.VoiceCallManager and 
org.ofono.MessageManager interfaces be used to indicate whether voice 
calls and/or messaging is available? That wouldn't require any D-Bus API 
changes at all.




VoiceCallManager should always be available since you still need to make 
Emergency Calls.  There might be some really weird legacy devices which 
were Data-Only, but those aren't really the focus of oFono anyway.


What I assume is happening in 27.007 is that registration states 6/9 & 
7/10 allows one to discern whether Circuit Switched Fallback would be 
triggered.  I assume if the device (& network) is VoLTE capable, then 
you'd see state 9/10, 6/7 otherwise.  Perhaps all 4 states should be 
combined into a single CircuitSwitchedFallback property instead...


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


Re: [PATCH] atmodem: add LTE state for AT+CREG

2018-09-10 Thread Slava Monich

Hi Denis,

Hi Slava,

On 09/10/2018 01:26 PM, Slava Monich wrote:

Hi Anirudh, Denis at al.

@@ -669,6 +669,10 @@ const char *registration_status_to_string(int 
status)

  return "unknown";
  case NETWORK_REGISTRATION_STATUS_ROAMING:
  return "roaming";
+    case NETWORK_REGISTRATION_STATUS_REGISTERED_EUTRAN:
+    return "registered lte";
+    case NETWORK_REGISTRATION_STATUS_ROAMING_EUTRAN:
+    return "roaming lte";


What would be the difference between "roaming lte" and "roaming" (or 
"registered lte" vs "registered") from the viewpoint of D-Bus API 
clients? In other words, what would e.g. dialer do differently 
depending on whether registration status is "registered lte" or just 
"registered"? Is it worth the API break? The existing clients won't 
know how to handle the new values until they (clients) get updated.




I agree.  That is why I suggested a separate Property for this (and 
the CSFB case from 27.007) instead of modifying the Status property 
directly.


E.g. REGISTERED_SMS_EUTRAN would map to Status="registered" and 
SmsOnly=False.  ROAMING_SMS_EUTRAN would map to Status="roaming" and 
SmsOnly=True.


I assume that the Dialer can ignore this as the modem will fall back 
to 3G in case of a call?  Someone feel free to educate me.


The modems I dealt with do drop to 3G for voice calls. Once voice call 
terminates, they switch back to LTE (well, most of the time). Dialler 
(at least our dialler) doesn't care about access technology.


Just a thought - could the presence of org.ofono.VoiceCallManager and 
org.ofono.MessageManager interfaces be used to indicate whether voice 
calls and/or messaging is available? That wouldn't require any D-Bus API 
changes at all.


Cheers,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono


Re: [PATCH] atmodem: add LTE state for AT+CREG

2018-09-10 Thread Denis Kenzior

Hi Slava,

On 09/10/2018 01:26 PM, Slava Monich wrote:

Hi Anirudh, Denis at al.

@@ -669,6 +669,10 @@ const char *registration_status_to_string(int 
status)

  return "unknown";
  case NETWORK_REGISTRATION_STATUS_ROAMING:
  return "roaming";
+    case NETWORK_REGISTRATION_STATUS_REGISTERED_EUTRAN:
+    return "registered lte";
+    case NETWORK_REGISTRATION_STATUS_ROAMING_EUTRAN:
+    return "roaming lte";


What's would be the difference between "roaming lte" and "roaming" (or 
"registered lte" vs "registered") from the viewpoint of D-Bus API 
clients? In other words, what would e.g. dialer do differently depending 
on whether registration status is "registered lte" or just "registered"? 
Is it worth the API break? The existing clients won't know how to handle 
the new values until they (clients) get updated.




I agree.  That is why I suggested a separate Property for this (and the 
CSFB case from 27.007) instead of modifying the Status property directly.


E.g. REGISTERED_SMS_EUTRAN would map to Status="registered" and 
SmsOnly=False.  ROAMING_SMS_EUTRAN would map to Status="roaming" and 
SmsOnly=True.


I assume that the Dialer can ignore this as the modem will fall back to 
3G in case of a call?  Someone feel free to educate me.


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


Re: [PATCH] atmodem: add LTE state for AT+CREG

2018-09-10 Thread Slava Monich

Hi Anirudh, Denis at al.

@@ -669,6 +669,10 @@ const char *registration_status_to_string(int 
status)

  return "unknown";
  case NETWORK_REGISTRATION_STATUS_ROAMING:
  return "roaming";
+    case NETWORK_REGISTRATION_STATUS_REGISTERED_EUTRAN:
+    return "registered lte";
+    case NETWORK_REGISTRATION_STATUS_ROAMING_EUTRAN:
+    return "roaming lte";


What's would be the difference between "roaming lte" and "roaming" (or 
"registered lte" vs "registered") from the viewpoint of D-Bus API 
clients? In other words, what would e.g. dialer do differently depending 
on whether registration status is "registered lte" or just "registered"? 
Is it worth the API break? The existing clients won't know how to handle 
the new values until they (clients) get updated.


Cheers,
-Slava
___
ofono mailing list
ofono@ofono.org
https://lists.ofono.org/mailman/listinfo/ofono


Re: [PATCH] atmodem: add LTE state for AT+CREG

2018-09-10 Thread Denis Kenzior

Hi Anirudh,

On 09/10/2018 12:15 AM, "Anirudh Gargi anirudh.gargi"@intel.com wrote:

Something is funny with your email address here..


From: Anirudh Gargi 

CREG reports 6 and 7 for SMS only registration on E-UTRAN. Fix
sms atom to consider the new states while sending SMS.
---
  drivers/atmodem/network-registration.c | 11 +--
  src/common.c   |  4 
  src/common.h   |  2 ++
  src/sms.c  |  2 ++
  4 files changed, 17 insertions(+), 2 deletions(-)


This needs to be broken up into two patches per 'HACKING', 'Submitting 
patches' section.




diff --git a/drivers/atmodem/network-registration.c 
b/drivers/atmodem/network-registration.c
index 0854cd1..5945e1c 100644
--- a/drivers/atmodem/network-registration.c
+++ b/drivers/atmodem/network-registration.c
@@ -228,6 +228,10 @@ static void at_creg_cb(gboolean ok, GAtResult *result, 
gpointer user_data)
if ((status == 1 || status == 5) && (tech == -1))
tech = nd->tech;
  
+	/* Handle EUTRAN cases */

+   if ((status == 6 || status == 7) && (tech == -1))
+   tech = nd->tech;
+


Should tech be hardcoded to EUTRAN here?


cb(, status, lac, ci, tech, cbd->data);
  }
  
@@ -1522,8 +1526,11 @@ static void creg_notify(GAtResult *result, gpointer user_data)

, , , nd->vendor) == FALSE)
return;
  
-	if (status != 1 && status != 5)

-   goto notify;
+   if (status != 1 && status != 5) {
+   /* Check EUTRAN cases */
+   if (status != 6 && status != 7)
+   goto notify;
+   }


So I would do something like:

if (status == 6 || status == 7) {
tech = EUTRAN;
goto notify;
}

if (status != 1 && status != 5)
goto notify;

  
  	tq = g_try_new0(struct tech_query, 1);

if (tq == NULL)
diff --git a/src/common.c b/src/common.c
index 3ccaf7c..79eed3e 100644
--- a/src/common.c
+++ b/src/common.c
@@ -669,6 +669,10 @@ const char *registration_status_to_string(int status)
return "unknown";
case NETWORK_REGISTRATION_STATUS_ROAMING:
return "roaming";
+   case NETWORK_REGISTRATION_STATUS_REGISTERED_EUTRAN:
+   return "registered lte";
+   case NETWORK_REGISTRATION_STATUS_ROAMING_EUTRAN:
+   return "roaming lte";


No spaces in our property values.  Also you're changing the D-Bus API, 
so you might want to add a separate commit to doc/network-api.txt.


And another thing, this is an SMS-only registration according to 27.007. 
 So 'registered lte' makes no sense.   Perhaps we need a separate 
property for this and the CSFB cases.  E.g.

boolean SmsOnly [readonly] - Current registration is only valid for SMS.


}
  
  	return "";

diff --git a/src/common.h b/src/common.h
index 1b6b01d..44d7c6f 100644
--- a/src/common.h
+++ b/src/common.h
@@ -43,6 +43,8 @@ enum network_registration_status {
NETWORK_REGISTRATION_STATUS_DENIED =3,
NETWORK_REGISTRATION_STATUS_UNKNOWN =   4,
NETWORK_REGISTRATION_STATUS_ROAMING =   5,
+   NETWORK_REGISTRATION_STATUS_REGISTERED_EUTRAN = 6,
+   NETWORK_REGISTRATION_STATUS_ROAMING_EUTRAN =7,

REGISTERED_SMS_EUTRAN?

  };
  
  /* 27.007 Section 7.3  */

diff --git a/src/sms.c b/src/sms.c
index b86158e..e229425 100644
--- a/src/sms.c
+++ b/src/sms.c
@@ -782,6 +782,8 @@ static void netreg_status_watch(int status, int lac, int 
ci, int tech,
switch (status) {
case NETWORK_REGISTRATION_STATUS_REGISTERED:
case NETWORK_REGISTRATION_STATUS_ROAMING:
+   case NETWORK_REGISTRATION_STATUS_REGISTERED_EUTRAN:
+   case NETWORK_REGISTRATION_STATUS_ROAMING_EUTRAN:
sms->registered = TRUE;
break;
default:



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