[PATCH 1/2] Telit HE910 general fixes for gprs, sms and voicecall

[PATCH 1/2] Telit HE910 general fixes for gprs, sms and voicecall

[Enrico Sau]

[-- Attachment #1: Type: text/plain, Size: 7523 bytes --]

diff --git a/plugins/he910.c b/plugins/he910.c
index 69362b8..304fd35 100644
--- a/plugins/he910.c
+++ b/plugins/he910.c
@@ -59,6 +59,8 @@
 #include <drivers/atmodem/atutil.h>
 #include <drivers/atmodem/vendor.h>

+#define TELIT_CFUN_DELAY 3*1000*1000 /* uS */
+
 static const char *none_prefix[] = { NULL };
 static const char *qss_prefix[] = { "#QSS:", NULL };

@@ -141,7 +143,7 @@ static void switch_sim_state_status(struct ofono_modem
*modem, int status)
        case 3: /* SIM inserted, SMS and phonebook ready */
                if (data->sms_phonebook_added == FALSE) {
                        ofono_phonebook_create(modem, 0, "atmodem",
data->chat);
-                       ofono_sms_create(modem, 0, "atmodem", data->chat);
+                       ofono_sms_create(modem, OFONO_VENDOR_TELIT,
"atmodem", data->chat);
                        data->sms_phonebook_added = TRUE;
                }
                break;
@@ -217,6 +219,8 @@ static void cfun_enable_cb(gboolean ok, GAtResult
*result, gpointer user_data)
         * When the DCD is disabled the modem does not hangup anymore
         * after the data connection.
         */
+       g_at_chat_send(data->modem, "AT&C0", NULL, NULL, NULL, NULL);
+
        g_at_chat_send(data->chat, "AT&C0", NULL, NULL, NULL, NULL);

        data->have_sim = FALSE;
@@ -224,24 +228,40 @@ static void cfun_enable_cb(gboolean ok, GAtResult
*result, gpointer user_data)
        ofono_modem_set_powered(modem, TRUE);

-       /*
-        * Tell the modem not to automatically initiate auto-attach
-        * proceedures on its own.
-        */
-       g_at_chat_send(data->chat, "AT#AUTOATT=0", none_prefix,
-                               NULL, NULL, NULL);
-
        /* Follow sim state */
        g_at_chat_register(data->chat, "#QSS:", he910_qss_notify,
                                FALSE, modem, NULL);

-       /* Enable sim state notification */
-       g_at_chat_send(data->chat, "AT#QSS=2", none_prefix, NULL, NULL,
NULL);
-
        g_at_chat_send(data->chat, "AT#QSS?", qss_prefix,
                        qss_query_cb, modem, NULL);
 }

+static void cfun_delay_cb(gboolean ok, GAtResult *result, gpointer
user_data)
+{
+       struct ofono_modem *modem = user_data;
+       struct he910_data *data = ofono_modem_get_data(modem);
+
+       DBG("%p", modem);
+
+       if (!ok) {
+               g_at_chat_unref(data->chat);
+               data->chat = NULL;
+
+               g_at_chat_unref(data->modem);
+               data->modem = NULL;
+
+               ofono_modem_set_powered(modem, FALSE);
+               return;
+       }
+
+       DBG("Wait for AT+CFUN=4 command completion");
+       usleep(TELIT_CFUN_DELAY);
+
+       /* Set phone functionality */
+       g_at_chat_send(data->chat, "AT+CFUN=1", none_prefix,
+                               cfun_enable_cb, modem, NULL);
+}
+
 static int he910_enable(struct ofono_modem *modem)
 {
        struct he910_data *data = ofono_modem_get_data(modem);
@@ -268,9 +288,11 @@ static int he910_enable(struct ofono_modem *modem)
        g_at_chat_send(data->chat, "ATE0 +CMEE=1", none_prefix,
                                NULL, NULL, NULL);

-       /* Set phone functionality */
-       g_at_chat_send(data->chat, "AT+CFUN=1", none_prefix,
-                               cfun_enable_cb, modem, NULL);
+       /* Set QSS notifications before restarting the SIM */
+       g_at_chat_send(data->chat, "AT#QSS=2", none_prefix,     NULL,
modem, NULL);
+
+       /* Power off module to get alla notifications on startup */
+       g_at_chat_send(data->chat, "AT+CFUN=4", none_prefix, cfun_delay_cb,
modem, NULL);

        return -EINPROGRESS;
 }
@@ -333,7 +355,7 @@ static void he910_post_online(struct ofono_modem *modem)

        ofono_voicecall_create(modem, 0, "atmodem", data->chat);
        ofono_netreg_create(modem, OFONO_VENDOR_TELIT, "atmodem",
data->chat);
-       ofono_ussd_create(modem, 0, "atmodem", data->chat);
+       ofono_ussd_create(modem, OFONO_VENDOR_TELIT, "atmodem", data->chat);
        ofono_call_forwarding_create(modem, 0, "atmodem", data->chat);
        ofono_call_settings_create(modem, 0, "atmodem", data->chat);
        ofono_call_meter_create(modem, 0, "atmodem", data->chat);
@@ -349,6 +371,8 @@ static void he910_post_online(struct ofono_modem *modem)

        if (gprs && gc)
                ofono_gprs_add_context(gprs, gc);
+       g_at_chat_send(data->chat, "AT#AUTOATT=1", none_prefix,
+                               NULL, NULL, NULL);
 }

 static int he910_probe(struct ofono_modem *modem)
diff --git a/src/sim.c b/src/sim.c
index b5badf1..65e5c32 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -801,6 +801,29 @@ static DBusMessage *sim_change_pin(DBusConnection
*conn, DBusMessage *msg,
        return NULL;
 }

+static void sim_iccid_read_cb(int ok, int length, int record,
+                               const unsigned char *data,
+                               int record_length, void *userdata)
+{
+       struct ofono_sim *sim = userdata;
+       const char *path = __ofono_atom_get_path(sim->atom);
+       DBusConnection *conn = ofono_dbus_get_connection();
+       char iccid[21]; /* ICCID max length is 20 + 1 for NULL */
+
+       if (!ok || length < 10)
+               return;
+
+       extract_bcd_number(data, length, iccid);
+       iccid[20] = '\0';
+       sim->iccid = g_strdup(iccid);
+
+       ofono_dbus_signal_property_changed(conn, path,
+                                               OFONO_SIM_MANAGER_INTERFACE,
+                                               "CardIdentifier",
+                                               DBUS_TYPE_STRING,
+                                               &sim->iccid);
+}
+
 static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
 {
        struct ofono_sim *sim = data;
@@ -813,6 +836,11 @@ static void sim_enter_pin_cb(const struct ofono_error
*error, void *data)

        __ofono_dbus_pending_reply(&sim->pending, reply);

+       if (!sim->iccid)
+               ofono_sim_read(sim->early_context, SIM_EF_ICCID_FILEID,
+
 OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
+                                          sim_iccid_read_cb, sim);
+
        __ofono_sim_recheck_pin(sim);
 }

@@ -2045,29 +2073,6 @@ skip_efpl:
        sim->language_prefs_update = FALSE;
 }

-static void sim_iccid_read_cb(int ok, int length, int record,
-                               const unsigned char *data,
-                               int record_length, void *userdata)
-{
-       struct ofono_sim *sim = userdata;
-       const char *path = __ofono_atom_get_path(sim->atom);
-       DBusConnection *conn = ofono_dbus_get_connection();
-       char iccid[21]; /* ICCID max length is 20 + 1 for NULL */
-
-       if (!ok || length < 10)
-               return;
-
-       extract_bcd_number(data, length, iccid);
-       iccid[20] = '\0';
-       sim->iccid = g_strdup(iccid);
-
-       ofono_dbus_signal_property_changed(conn, path,
-                                               OFONO_SIM_MANAGER_INTERFACE,
-                                               "CardIdentifier",
-                                               DBUS_TYPE_STRING,
-                                               &sim->iccid);
-}
-
 static void sim_iccid_changed(int id, void *userdata)
 {
        struct ofono_sim *sim = userdata;

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 23583 bytes --]

Re: [PATCH 1/2] Telit HE910 general fixes for gprs, sms and voicecall

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 8606 bytes --]

Hi Enrico,

On 03/06/2015 10:07 AM, Enrico Sau wrote:
> diff --git a/plugins/he910.c b/plugins/he910.c
> index 69362b8..304fd35 100644

Could you submit patches using git send-email?  Otherwise I can't apply 
them.

Lets go through the changes one by one:

> --- a/plugins/he910.c
> +++ b/plugins/he910.c
> @@ -59,6 +59,8 @@
>   #include <drivers/atmodem/atutil.h>
>   #include <drivers/atmodem/vendor.h>
>
> +#define TELIT_CFUN_DELAY 3*1000*1000 /* uS */
> +
>   static const char *none_prefix[] = { NULL };
>   static const char *qss_prefix[] = { "#QSS:", NULL };
>
> @@ -141,7 +143,7 @@ static void switch_sim_state_status(struct
> ofono_modem *modem, int status)
>          case 3: /* SIM inserted, SMS and phonebook ready */
>                  if (data->sms_phonebook_added == FALSE) {
>                          ofono_phonebook_create(modem, 0, "atmodem",
> data->chat);
> -                       ofono_sms_create(modem, 0, "atmodem", data->chat);
> +                       ofono_sms_create(modem, OFONO_VENDOR_TELIT,
> "atmodem", data->chat);
>                          data->sms_phonebook_added = TRUE;
>                  }
>                  break;
> @@ -217,6 +219,8 @@ static void cfun_enable_cb(gboolean ok, GAtResult
> *result, gpointer user_data)
>           * When the DCD is disabled the modem does not hangup anymore
>           * after the data connection.
>           */
> +       g_at_chat_send(data->modem, "AT&C0", NULL, NULL, NULL, NULL);
> +
>          g_at_chat_send(data->chat, "AT&C0", NULL, NULL, NULL, NULL);
>
>          data->have_sim = FALSE;
> @@ -224,24 +228,40 @@ static void cfun_enable_cb(gboolean ok, GAtResult
> *result, gpointer user_data)
>          ofono_modem_set_powered(modem, TRUE);
>
> -       /*
> -        * Tell the modem not to automatically initiate auto-attach
> -        * proceedures on its own.
> -        */
> -       g_at_chat_send(data->chat, "AT#AUTOATT=0", none_prefix,
> -                               NULL, NULL, NULL);
> -

I really think this part is not necessary.  oFono expects autoattach to 
be disabled.

>          /* Follow sim state */
>          g_at_chat_register(data->chat, "#QSS:", he910_qss_notify,
>                                  FALSE, modem, NULL);
>
> -       /* Enable sim state notification */
> -       g_at_chat_send(data->chat, "AT#QSS=2", none_prefix, NULL, NULL,
> NULL);
> -
>          g_at_chat_send(data->chat, "AT#QSS?", qss_prefix,
>                          qss_query_cb, modem, NULL);
>   }
>
> +static void cfun_delay_cb(gboolean ok, GAtResult *result, gpointer
> user_data)
> +{
> +       struct ofono_modem *modem = user_data;
> +       struct he910_data *data = ofono_modem_get_data(modem);
> +
> +       DBG("%p", modem);
> +
> +       if (!ok) {
> +               g_at_chat_unref(data->chat);
> +               data->chat = NULL;
> +
> +               g_at_chat_unref(data->modem);
> +               data->modem = NULL;
> +
> +               ofono_modem_set_powered(modem, FALSE);
> +               return;
> +       }
> +
> +       DBG("Wait for AT+CFUN=4 command completion");
> +       usleep(TELIT_CFUN_DELAY);
> +
> +       /* Set phone functionality */
> +       g_at_chat_send(data->chat, "AT+CFUN=1", none_prefix,
> +                               cfun_enable_cb, modem, NULL);
> +}
> +
>   static int he910_enable(struct ofono_modem *modem)
>   {
>          struct he910_data *data = ofono_modem_get_data(modem);
> @@ -268,9 +288,11 @@ static int he910_enable(struct ofono_modem *modem)
>          g_at_chat_send(data->chat, "ATE0 +CMEE=1", none_prefix,
>                                  NULL, NULL, NULL);
>
> -       /* Set phone functionality */
> -       g_at_chat_send(data->chat, "AT+CFUN=1", none_prefix,
> -                               cfun_enable_cb, modem, NULL);
> +       /* Set QSS notifications before restarting the SIM */
> +       g_at_chat_send(data->chat, "AT#QSS=2", none_prefix,     NULL,
> modem, NULL);
> +
> +       /* Power off module to get alla notifications on startup */
> +       g_at_chat_send(data->chat, "AT+CFUN=4", none_prefix,
> cfun_delay_cb, modem, NULL);
>

On older Telit firmware, CFUN=4 turns the SIM off.  If you have such 
firmware, I suggest upgrading it.  Otherwise the modem will not work as 
oFono expects it to.

This change should not be required with latest firmware.

>          return -EINPROGRESS;
>   }
> @@ -333,7 +355,7 @@ static void he910_post_online(struct ofono_modem *modem)
>
>          ofono_voicecall_create(modem, 0, "atmodem", data->chat);
>          ofono_netreg_create(modem, OFONO_VENDOR_TELIT, "atmodem",
> data->chat);
> -       ofono_ussd_create(modem, 0, "atmodem", data->chat);
> +       ofono_ussd_create(modem, OFONO_VENDOR_TELIT, "atmodem", data->chat);
>          ofono_call_forwarding_create(modem, 0, "atmodem", data->chat);
>          ofono_call_settings_create(modem, 0, "atmodem", data->chat);
>          ofono_call_meter_create(modem, 0, "atmodem", data->chat);
> @@ -349,6 +371,8 @@ static void he910_post_online(struct ofono_modem *modem)
>
>          if (gprs && gc)
>                  ofono_gprs_add_context(gprs, gc);
> +       g_at_chat_send(data->chat, "AT#AUTOATT=1", none_prefix,
> +                               NULL, NULL, NULL);
>   }
>
>   static int he910_probe(struct ofono_modem *modem)
> diff --git a/src/sim.c b/src/sim.c
> index b5badf1..65e5c32 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -801,6 +801,29 @@ static DBusMessage *sim_change_pin(DBusConnection
> *conn, DBusMessage *msg,
>          return NULL;
>   }
>
> +static void sim_iccid_read_cb(int ok, int length, int record,
> +                               const unsigned char *data,
> +                               int record_length, void *userdata)
> +{
> +       struct ofono_sim *sim = userdata;
> +       const char *path = __ofono_atom_get_path(sim->atom);
> +       DBusConnection *conn = ofono_dbus_get_connection();
> +       char iccid[21]; /* ICCID max length is 20 + 1 for NULL */
> +
> +       if (!ok || length < 10)
> +               return;
> +
> +       extract_bcd_number(data, length, iccid);
> +       iccid[20] = '\0';
> +       sim->iccid = g_strdup(iccid);
> +
> +       ofono_dbus_signal_property_changed(conn, path,
> +                                               OFONO_SIM_MANAGER_INTERFACE,
> +                                               "CardIdentifier",
> +                                               DBUS_TYPE_STRING,
> +                                               &sim->iccid);
> +}
> +
>   static void sim_enter_pin_cb(const struct ofono_error *error, void *data)
>   {
>          struct ofono_sim *sim = data;
> @@ -813,6 +836,11 @@ static void sim_enter_pin_cb(const struct
> ofono_error *error, void *data)
>
>          __ofono_dbus_pending_reply(&sim->pending, reply);
>
> +       if (!sim->iccid)
> +               ofono_sim_read(sim->early_context, SIM_EF_ICCID_FILEID,
> +
>   OFONO_SIM_FILE_STRUCTURE_TRANSPARENT,
> +                                          sim_iccid_read_cb, sim);
> +
>          __ofono_sim_recheck_pin(sim);
>   }
>
> @@ -2045,29 +2073,6 @@ skip_efpl:
>          sim->language_prefs_update = FALSE;
>   }
>
> -static void sim_iccid_read_cb(int ok, int length, int record,
> -                               const unsigned char *data,
> -                               int record_length, void *userdata)
> -{
> -       struct ofono_sim *sim = userdata;
> -       const char *path = __ofono_atom_get_path(sim->atom);
> -       DBusConnection *conn = ofono_dbus_get_connection();
> -       char iccid[21]; /* ICCID max length is 20 + 1 for NULL */
> -
> -       if (!ok || length < 10)
> -               return;
> -
> -       extract_bcd_number(data, length, iccid);
> -       iccid[20] = '\0';
> -       sim->iccid = g_strdup(iccid);
> -
> -       ofono_dbus_signal_property_changed(conn, path,
> -                                               OFONO_SIM_MANAGER_INTERFACE,
> -                                               "CardIdentifier",
> -                                               DBUS_TYPE_STRING,
> -                                               &sim->iccid);
> -}
> -
>   static void sim_iccid_changed(int id, void *userdata)
>   {
>          struct ofono_sim *sim = userdata;
>
>

ICCid changes should be in a separate patch.  I'm not sure they are 
still required with new firmware.

Regards,
-Denis

Re: [PATCH 1/2] Telit HE910 general fixes for gprs, sms and voicecall

[Enrico Sau]

[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]

​Hi Denis,​


>
> I really think this part is not necessary.  oFono expects autoattach to be
> disabled.


​Autoattach is the only part I'm not sure of.
As I said, I didn't go too deep in this problem, but leaving autoattach to
zero, made impossible to do more than one ppp session.
In this way my modem is working fine I believe.​


>
>
> On older Telit firmware, CFUN=4 turns the SIM off.  If you have such
> firmware, I suggest upgrading it.  Otherwise the modem will not work as
> oFono expects it to.
>
> This change should not be required with latest firmware.


​CFUN=4 always turns SIM off, even in new firmware.
The reason for this fix is because sometimes ofono is started when the
modem is already attached to the system, or it is in an inconsistent state.
This is a problem for ofono because lots of unsolicited messages are lost.
Setting CFUN=4, and then CFUN=1, is like soft resetting the modem in order
to get all the startup messages.
I understand this may look as a workaround, but I cannot find a better way
to do it!
​


>
> ICCid changes should be in a separate patch.  I'm not sure they are still
> required with new firmware.
>
> ​I made this because when the ICCD is checked, the modem is not ready yet.
So ICCD field is always empty.
In this way, if the field is empty it is retrieved again.
As before, I'm not fully inside ofono architecture, so, if you think this
may be done in a better way, just discard it.
I will send it in a separate patch anyway.​


​Thank you.
Enrico​

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 3739 bytes --]

Re: [PATCH 1/2] Telit HE910 general fixes for gprs, sms and voicecall

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 515 bytes --]

Hi Enrico,

 > ​CFUN=4 always turns SIM off, even in new firmware.

I'm pretty sure we requested that they change this.  But its been a 
while and I did not follow up whether this change made it into the 
official firmware.

> The reason for this fix is because sometimes ofono is started when the
> modem is already attached to the system, or it is in an inconsistent state.

oFono powers down the modem when exiting.  So this should not be a 
problem in normal circumstances.

Regards,
-Denis

Re: [PATCH 1/2] Telit HE910 general fixes for gprs, sms and voicecall

[Enrico Sau]

[-- Attachment #1: Type: text/plain, Size: 366 bytes --]

Hi Denis,

In official AT command reference for HE910 is stated that CFUN=4 turns the
SIM off.
Then, I made the CFUN=4 trick to handle situations where ofono was sterted
with the modem already ON.
I understand this is not a normal situation anyway.

I will resubmit the patches with git send mail and without these "custom
tricks".

Thank you.

Enrico

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1140 bytes --]

RE: [PATCH 1/2] Telit HE910 general fixes for gprs, sms and voicecall

[Kallumari]

[-- Attachment #1: Type: text/plain, Size: 642 bytes --]

Enrico,

Any updates on the patches ?

Regards,
<Ram>

From: ofono [mailto:ofono-bounces(a)ofono.org] On Behalf Of Enrico Sau
Sent: Friday, March 13, 2015 3:32 PM
To: ofono(a)ofono.org
Subject: Re: [PATCH 1/2] Telit HE910 general fixes for gprs, sms and voicecall

Hi Denis,

In official AT command reference for HE910 is stated that CFUN=4 turns the SIM off.
Then, I made the CFUN=4 trick to handle situations where ofono was sterted with the modem already ON.
I understand this is not a normal situation anyway.

I will resubmit the patches with git send mail and without these "custom tricks".

Thank you.

Enrico


[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 5038 bytes --]

Re: [PATCH 1/2] Telit HE910 general fixes for gprs, sms and voicecall

[Enrico Sau]

[-- Attachment #1: Type: text/plain, Size: 117 bytes --]

Hi Ram,
I'm sorry, I'm very busy these days. I will submit my patches as soon I can review the changes. 

Enrico

[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 1290 bytes --]