2:25 p.m.
---
plugins/speedupcdma.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 100 insertions(+), 2 deletions(-)
diff --git a/plugins/speedupcdma.c b/plugins/speedupcdma.c
index b7e54a1..1dcff88 100644
--- a/plugins/speedupcdma.c
+++ b/plugins/speedupcdma.c
@@ -41,11 +41,25 @@
#include <drivers/atmodem/atutil.h>
static const char *none_prefix[] = { NULL };
+static const char *sysinfo_prefix[] = { "^SYSINFO:", NULL };
+
+enum speedupcdma_sim_state {
+ SPEEDUPCDMA_SIM_STATE_VALID = 1,
+ SPEEDUPCDMA_SIM_STATE_ROMSIM = 240,
+ SPEEDUPCDMA_SIM_STATE_NOT_EXISTENT = 255
+};
struct speedupcdma_data {
GAtChat *chat;
+ enum speedupcdma_sim_state sim_state;
+ guint sim_poll_timeout;
+ guint sim_poll_count;
};
+#define MAX_SIM_POLL_COUNT 5
+
+static gboolean query_sim_state(gpointer user_data);
+
static void speedupcdma_debug(const char *str, void *data)
{
const char *prefix = data;
@@ -79,6 +93,90 @@ static void speedupcdma_remove(struct ofono_modem *modem)
g_free(data);
}
+static gboolean notify_sim_state(struct ofono_modem *modem,
+ enum speedupcdma_sim_state sim_state)
+{
+ struct speedupcdma_data *data = ofono_modem_get_data(modem);
+
+ DBG("%d", sim_state);
+
+ data->sim_state = sim_state;
+
+ switch (sim_state) {
+ case SPEEDUPCDMA_SIM_STATE_NOT_EXISTENT:
+ /* SIM is not ready, try again a bit later */
+ return TRUE;
+ case SPEEDUPCDMA_SIM_STATE_VALID:
+ case SPEEDUPCDMA_SIM_STATE_ROMSIM:
+ if (data->sim_poll_timeout) {
+ g_source_remove(data->sim_poll_timeout);
+ data->sim_poll_timeout = 0;
+ }
+
+ ofono_modem_set_powered(modem, TRUE);
+
+ return FALSE;
+ }
+
+ return FALSE;
+}
+
+static void sysinfo_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct speedupcdma_data *data = ofono_modem_get_data(modem);
+ gboolean rerun;
+ gint sim_state;
+ GAtResultIter iter;
+
+ if (!ok)
+ return;
+
+ g_at_result_iter_init(&iter, result);
+
+ if (!g_at_result_iter_next(&iter, "^SYSINFO:"))
+ return;
+
+ if (!g_at_result_iter_skip_next(&iter))
+ return;
+
+ if (!g_at_result_iter_skip_next(&iter))
+ return;
+
+ if (!g_at_result_iter_skip_next(&iter))
+ return;
+
+ if (!g_at_result_iter_skip_next(&iter))
+ return;
+
+ if (!g_at_result_iter_next_number(&iter, &sim_state))
+ return;
+
+ rerun = notify_sim_state(modem, (enum speedupcdma_sim_state) sim_state);
+
+ if (rerun && data->sim_poll_count < MAX_SIM_POLL_COUNT) {
+ data->sim_poll_count++;
+ data->sim_poll_timeout = g_timeout_add_seconds(2,
+ query_sim_state,
+ modem);
+ }
+}
+
+static gboolean query_sim_state(gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct speedupcdma_data *data = ofono_modem_get_data(modem);
+
+ DBG("");
+
+ data->sim_poll_timeout = 0;
+
+ g_at_chat_send(data->chat, "AT^SYSINFO", sysinfo_prefix,
+ sysinfo_cb, modem, NULL);
+
+ return FALSE;
+}
+
static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
{
struct ofono_modem *modem = user_data;
@@ -93,8 +191,6 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer
user_data)
ofono_modem_set_powered(modem, FALSE);
return;
}
-
- ofono_modem_set_powered(modem, TRUE);
}
static int speedupcdma_enable(struct ofono_modem *modem)
@@ -132,6 +228,8 @@ static int speedupcdma_enable(struct ofono_modem *modem)
g_at_chat_send(data->chat, "AT+CFUN=1", none_prefix,
cfun_enable, modem, NULL);
+ query_sim_state(modem);
+
return -EINPROGRESS;
}
--
1.7.4.1
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
7:33 p.m.
Hi Bertrand,
please do not make up new commit prefixes. Use simple lower case strings
like the modem plugin name, the subdirectory in drivers or for the core
the logical name.
plugins/speedupcdma.c | 102
++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 100 insertions(+), 2 deletions(-)
diff --git a/plugins/speedupcdma.c b/plugins/speedupcdma.c
index b7e54a1..1dcff88 100644
--- a/plugins/speedupcdma.c
+++ b/plugins/speedupcdma.c
@@ -41,11 +41,25 @@
#include <drivers/atmodem/atutil.h>
static const char *none_prefix[] = { NULL };
+static const char *sysinfo_prefix[] = { "^SYSINFO:", NULL };
+
+enum speedupcdma_sim_state {
+ SPEEDUPCDMA_SIM_STATE_VALID = 1,
+ SPEEDUPCDMA_SIM_STATE_ROMSIM = 240,
+ SPEEDUPCDMA_SIM_STATE_NOT_EXISTENT = 255
Add last , as well. We do this for enum so that if they get extended the
patches look cleaner and not have to reshuffle.
+};
struct speedupcdma_data {
GAtChat *chat;
+ enum speedupcdma_sim_state sim_state;
+ guint sim_poll_timeout;
+ guint sim_poll_count;
};
+#define MAX_SIM_POLL_COUNT 5
+
+static gboolean query_sim_state(gpointer user_data);
+
static void speedupcdma_debug(const char *str, void *data)
{
const char *prefix = data;
@@ -79,6 +93,90 @@ static void speedupcdma_remove(struct ofono_modem *modem)
g_free(data);
}
+static gboolean notify_sim_state(struct ofono_modem *modem,
+ enum speedupcdma_sim_state sim_state)
+{
+ struct speedupcdma_data *data = ofono_modem_get_data(modem);
+
+ DBG("%d", sim_state);
+
+ data->sim_state = sim_state;
+
+ switch (sim_state) {
+ case SPEEDUPCDMA_SIM_STATE_NOT_EXISTENT:
+ /* SIM is not ready, try again a bit later */
+ return TRUE;
+ case SPEEDUPCDMA_SIM_STATE_VALID:
+ case SPEEDUPCDMA_SIM_STATE_ROMSIM:
+ if (data->sim_poll_timeout) {
+ g_source_remove(data->sim_poll_timeout);
+ data->sim_poll_timeout = 0;
+ }
+
+ ofono_modem_set_powered(modem, TRUE);
Use break; here.
And can you include OFONO_AT_DEBUG traces to show what is going on.
+
+ return FALSE;
+ }
+
+ return FALSE;
+}
+
+static void sysinfo_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct speedupcdma_data *data = ofono_modem_get_data(modem);
+ gboolean rerun;
+ gint sim_state;
+ GAtResultIter iter;
+
+ if (!ok)
+ return;
+
+ g_at_result_iter_init(&iter, result);
+
+ if (!g_at_result_iter_next(&iter, "^SYSINFO:"))
+ return;
+
+ if (!g_at_result_iter_skip_next(&iter))
+ return;
+
+ if (!g_at_result_iter_skip_next(&iter))
+ return;
+
+ if (!g_at_result_iter_skip_next(&iter))
+ return;
+
+ if (!g_at_result_iter_skip_next(&iter))
+ return;
+
+ if (!g_at_result_iter_next_number(&iter, &sim_state))
+ return;
+
+ rerun = notify_sim_state(modem, (enum speedupcdma_sim_state) sim_state);
+
+ if (rerun && data->sim_poll_count < MAX_SIM_POLL_COUNT) {
+ data->sim_poll_count++;
+ data->sim_poll_timeout = g_timeout_add_seconds(2,
+ query_sim_state,
+ modem);
+ }
+}
This logic is copied from the Huawei GSM plugin and there I am not even
sure that it is correct. Please show us AT command traces. Since I do
not think that it is this complicated for CDMA.
The Huawei plugin is a bad example to copy code from.
+static gboolean query_sim_state(gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct speedupcdma_data *data = ofono_modem_get_data(modem);
+
+ DBG("");
+
+ data->sim_poll_timeout = 0;
+
+ g_at_chat_send(data->chat, "AT^SYSINFO", sysinfo_prefix,
+ sysinfo_cb, modem, NULL);
+
+ return FALSE;
+}
+
static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
{
struct ofono_modem *modem = user_data;
@@ -93,8 +191,6 @@ static void cfun_enable(gboolean ok, GAtResult *result, gpointer
user_data)
ofono_modem_set_powered(modem, FALSE);
return;
}
-
- ofono_modem_set_powered(modem, TRUE);
}
static int speedupcdma_enable(struct ofono_modem *modem)
@@ -132,6 +228,8 @@ static int speedupcdma_enable(struct ofono_modem *modem)
g_at_chat_send(data->chat, "AT+CFUN=1", none_prefix,
cfun_enable, modem, NULL);
+ query_sim_state(modem);
+
This is wrong code. So in case CFUN=1 fails, you still go ahead and
query the SIM state. That is rather pointless and might result on
unforseen feedback from the modem.
In addition you do not handle the set_powered in case ^SYSINFO fails or
you have polled max attempts.
Now looking over this. I do not like this at all. Please do not copy any
code from Huawei plugin and write this from scratch based on actual
traces and the documentation.
Regards
Marcel
7:43 p.m.
Hi Marcel,
please do not make up new commit prefixes. Use simple lower case
strings
like the modem plugin name, the subdirectory in drivers or for the core
the logical name.
Ok, I will do this.
> plugins/speedupcdma.c | 102
++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 files changed, 100 insertions(+), 2 deletions(-)
>
> diff --git a/plugins/speedupcdma.c b/plugins/speedupcdma.c
> index b7e54a1..1dcff88 100644
> --- a/plugins/speedupcdma.c
> +++ b/plugins/speedupcdma.c
> @@ -41,11 +41,25 @@
> #include <drivers/atmodem/atutil.h>
>
> static const char *none_prefix[] = { NULL };
> +static const char *sysinfo_prefix[] = { "^SYSINFO:", NULL };
> +
> +enum speedupcdma_sim_state {
> + SPEEDUPCDMA_SIM_STATE_VALID = 1,
> + SPEEDUPCDMA_SIM_STATE_ROMSIM = 240,
> + SPEEDUPCDMA_SIM_STATE_NOT_EXISTENT = 255
Add last , as well. We do this for enum so that if they get extended the
patches look cleaner and not have to reshuffle.
I see.
> +};
>
> struct speedupcdma_data {
> GAtChat *chat;
> + enum speedupcdma_sim_state sim_state;
> + guint sim_poll_timeout;
> + guint sim_poll_count;
> };
>
> +#define MAX_SIM_POLL_COUNT 5
> +
> +static gboolean query_sim_state(gpointer user_data);
> +
> static void speedupcdma_debug(const char *str, void *data)
> {
> const char *prefix = data;
> @@ -79,6 +93,90 @@ static void speedupcdma_remove(struct ofono_modem *modem)
> g_free(data);
> }
>
> +static gboolean notify_sim_state(struct ofono_modem *modem,
> + enum speedupcdma_sim_state sim_state)
> +{
> + struct speedupcdma_data *data = ofono_modem_get_data(modem);
> +
> + DBG("%d", sim_state);
> +
> + data->sim_state = sim_state;
> +
> + switch (sim_state) {
> + case SPEEDUPCDMA_SIM_STATE_NOT_EXISTENT:
> + /* SIM is not ready, try again a bit later */
> + return TRUE;
> + case SPEEDUPCDMA_SIM_STATE_VALID:
> + case SPEEDUPCDMA_SIM_STATE_ROMSIM:
> + if (data->sim_poll_timeout) {
> + g_source_remove(data->sim_poll_timeout);
> + data->sim_poll_timeout = 0;
> + }
> +
> + ofono_modem_set_powered(modem, TRUE);
Use break; here.
And can you include OFONO_AT_DEBUG traces to show what is going on.
> +
> + return FALSE;
> + }
> +
> + return FALSE;
> +}
> +
> +static void sysinfo_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> + struct ofono_modem *modem = user_data;
> + struct speedupcdma_data *data = ofono_modem_get_data(modem);
> + gboolean rerun;
> + gint sim_state;
> + GAtResultIter iter;
> +
> + if (!ok)
> + return;
> +
> + g_at_result_iter_init(&iter, result);
> +
> + if (!g_at_result_iter_next(&iter, "^SYSINFO:"))
> + return;
> +
> + if (!g_at_result_iter_skip_next(&iter))
> + return;
> +
> + if (!g_at_result_iter_skip_next(&iter))
> + return;
> +
> + if (!g_at_result_iter_skip_next(&iter))
> + return;
> +
> + if (!g_at_result_iter_skip_next(&iter))
> + return;
> +
> + if (!g_at_result_iter_next_number(&iter, &sim_state))
> + return;
> +
> + rerun = notify_sim_state(modem, (enum speedupcdma_sim_state)
sim_state);
> +
> + if (rerun && data->sim_poll_count < MAX_SIM_POLL_COUNT) {
> + data->sim_poll_count++;
> + data->sim_poll_timeout = g_timeout_add_seconds(2,
> + query_sim_state,
> + modem);
> + }
> +}
This logic is copied from the Huawei GSM plugin and there I am not even
sure that it is correct. Please show us AT command traces. Since I do
not think that it is this complicated for CDMA.
I also think so.
After a full day of testing (while implementing NetworkRegistration for CDMA), I have
found a best way to handle this. In fact the correct way. As you said, I have copy/paste
this code from Huawei driver, and I have simply some part but not all. And I have to
rework this.
The Huawei plugin is a bad example to copy code from.
> +static gboolean query_sim_state(gpointer user_data)
> +{
> + struct ofono_modem *modem = user_data;
> + struct speedupcdma_data *data = ofono_modem_get_data(modem);
> +
> + DBG("");
> +
> + data->sim_poll_timeout = 0;
> +
> + g_at_chat_send(data->chat, "AT^SYSINFO", sysinfo_prefix,
> + sysinfo_cb, modem, NULL);
> +
> + return FALSE;
> +}
> +
> static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
> {
> struct ofono_modem *modem = user_data;
> @@ -93,8 +191,6 @@ static void cfun_enable(gboolean ok, GAtResult *result,
gpointer user_data)
> ofono_modem_set_powered(modem, FALSE);
> return;
> }
> -
> - ofono_modem_set_powered(modem, TRUE);
> }
>
> static int speedupcdma_enable(struct ofono_modem *modem)
> @@ -132,6 +228,8 @@ static int speedupcdma_enable(struct ofono_modem *modem)
> g_at_chat_send(data->chat, "AT+CFUN=1", none_prefix,
> cfun_enable, modem, NULL);
>
> + query_sim_state(modem);
> +
This is wrong code. So in case CFUN=1 fails, you still go ahead and
query the SIM state. That is rather pointless and might result on
unforseen feedback from the modem.
In addition you do not handle the set_powered in case ^SYSINFO fails or
you have polled max attempts.
Now looking over this. I do not like this at all. Please do not copy any
code from Huawei plugin and write this from scratch based on actual
traces and the documentation.
I totally agree, I will rework this part.
I will set the powered flag after CFUN, and use the SYSINFO to broadcast SIM info
(available or not).
Regards,
Bertrand
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
7:53 p.m.
Hi Bertrand,
> This is wrong code. So in case CFUN=1 fails, you still go ahead
and
> query the SIM state. That is rather pointless and might result on
> unforseen feedback from the modem.
>
> In addition you do not handle the set_powered in case ^SYSINFO fails or
> you have polled max attempts.
>
> Now looking over this. I do not like this at all. Please do not copy any
> code from Huawei plugin and write this from scratch based on actual
> traces and the documentation.
I totally agree, I will rework this part.
I will set the powered flag after CFUN, and use the SYSINFO to broadcast SIM info
(available or not).
please post OFONO_AT_DEBUG traces to the mailing list first.
If you wanna handle some fake CDMA SIM states, then we might need a
simplified SIM atom driver for CDMA as well. Or something only CDMA
specific.
Otherwise the check for a valid CDMA SIM, ROM SIM or whatever they call
it should be done during the enable callback handling. If it is not
there, then it is not there. Nothing you can do about it anyway. Failing
the modem enabling seems to be the best approach in my book.
Regards
Marcel
7:57 p.m.
Hi,
> > This is wrong code. So in case CFUN=1 fails, you still go
ahead and
> > query the SIM state. That is rather pointless and might result on
> > unforseen feedback from the modem.
> >
> > In addition you do not handle the set_powered in case ^SYSINFO fails or
> > you have polled max attempts.
> >
> > Now looking over this. I do not like this at all. Please do not copy any
> > code from Huawei plugin and write this from scratch based on actual
> > traces and the documentation.
>
> I totally agree, I will rework this part.
>
> I will set the powered flag after CFUN, and use the SYSINFO to broadcast SIM
info (available or not).
please post OFONO_AT_DEBUG traces to the mailing list first.
If you wanna handle some fake CDMA SIM states, then we might need a
simplified SIM atom driver for CDMA as well. Or something only CDMA
specific.
No. In fact modems seems to react correctly. There is SIM state notification, but we got
it once over 10. So I do not want to rely on.
SYSINFO will give me the info, and it's always correct so I do not need a polling
mechanism.
Otherwise the check for a valid CDMA SIM, ROM SIM or whatever they
call
it should be done during the enable callback handling. If it is not
there, then it is not there. Nothing you can do about it anyway. Failing
the modem enabling seems to be the best approach in my book.
Exactly, I have to check for SIM available or not. I will send once the SYSINFO in the
cfun callback, and based on the result notify the availability of the SIM.
Regards,
Bertrand
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
4076
days inactive
4076
days old
4 comments
3 participants
participants (3)
-
Aygon, Bertrand -
Bertrand Aygon
-
Marcel Holtmann