[PATCH] sms: fix send sms in case of lte registration
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
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
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
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
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
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(&error, status, lac, ci, tech, cbd->data); } @@ -1522,8 +1526,11 @@ static void creg_notify(GAtResult *result, gpointer user_data) &lac, &ci, &tech, 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
Re: [PATCH] udev:fix seg fault in case of vid and pid is NULL
Hi Anirudh, On 09/09/2018 11:42 PM, "Anirudh Gargi anirudh.gargi"@intel.com wrote: From: Anirudh Gargi In some case linux report 'driver' as valid yet vid and pid as NULL. Adding NULL check to prevent seg fault. Log: ofonod[23829]: plugins/udevng.c:udev_start() ofonod[23829]: plugins/udevng.c:enumerate_devices() ofonod[23829]: plugins/udevng.c:check_usb_device() hub [1d6b:0002] ofonod[23829]: plugins/udevng.c:check_usb_device() usb [1d6b:0002] ofonod[23829]: plugins/udevng.c:check_usb_device() usbhid [03f0:034a] ofonod[23829]: plugins/udevng.c:check_usb_device() usbhid [03f0:034a] ofonod[23829]: plugins/udevng.c:check_usb_device() usb [1d6b:0002] ofonod[23829]: plugins/udevng.c:check_usb_device() cdc_acm [(null):(null)] ofonod[23829]: Aborting (signal 11) [./src/ofonod] --- plugins/udevng.c | 3 +++ 1 file changed, 3 insertions(+) Applied, thanks. Regards, -Denis ___ ofono mailing list ofono@ofono.org https://lists.ofono.org/mailman/listinfo/ofono