[PATCH 1/3] sim800: add sim800 support.

Discussion:

Clement Viel

2018-11-13 09:29:41 UTC

---
plugins/sim900.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 82 insertions(+), 9 deletions(-)

diff --git a/plugins/sim900.c b/plugins/sim900.c
index a7728cd..bf7b9ec 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -24,6 +24,7 @@
#endif

#include <errno.h>
+#include <string.h>
#include <stdlib.h>
#include <glib.h>
#include <gatchat.h>
@@ -60,13 +61,77 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",

static const char *none_prefix[] = { NULL };

+typedef enum type {
+ SIM800,
+ SIM900,
+ SIM_UNKNOWN,
+} type_t;
+
struct sim900_data {
GIOChannel *device;
GAtMux *mux;
GAtChat * dlcs[NUM_DLC];
guint frame_size;
+ type_t modem_type;
};

+static void set_modem_type (const char *type, struct ofono_modem *modem)
+{
+ struct sim900_data *data = ofono_modem_get_data(modem);
+
+ if (strcmp(type, "sim800") == 0)
+ data->modem_type = SIM800;
+ else if (strcmp(type, "sim900") == 0)
+ data->modem_type = SIM900;
+ else
+ data->modem_type = SIM_UNKNOWN;
+ DBG("modem type is %d",data->modem_type);
+}
+
+static void mux_ready_notify(GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim900_data *data = ofono_modem_get_data(modem);
+ struct ofono_gprs *gprs = NULL;
+ struct ofono_gprs_context *gc;
+
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ data->dlcs[SMS_DLC]);
+
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
+
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
+ "atmodem", data->dlcs[GPRS_DLC]);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+
+
+}
+
+static void check_model(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ GAtResultIter iter;
+ char const *model;
+
+ DBG("");
+
+ g_at_result_iter_init(&iter, result);
+
+ while (g_at_result_iter_next(&iter, NULL)) {
+ if (!g_at_result_iter_next_unquoted_string(&iter, &model))
+ continue;
+
+ DBG("setting type %s", model);
+ if (strstr(model, "SIM800"))
+ set_modem_type("sim800", modem);
+ else if (strstr(model, "SIM900"))
+ set_modem_type("sim900", modem);
+ }
+}
+
static int sim900_probe(struct ofono_modem *modem)
{
struct sim900_data *data;
@@ -78,7 +143,6 @@ static int sim900_probe(struct ofono_modem *modem)
return -ENOMEM;

ofono_modem_set_data(modem, data);
-
return 0;
}

@@ -111,6 +175,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
GHashTable *options;

device = ofono_modem_get_string(modem, key);
+
if (device == NULL)
return NULL;

@@ -232,6 +297,11 @@ static void setup_internal_mux(struct ofono_modem *modem)
goto error;
}
}
+ if (data->modem_type == SIM800) {
+ for (i = 0; i<NUM_DLC; i++) {
+ g_at_chat_register(data->dlcs[i], "SMS Ready", mux_ready_notify, FALSE, modem, NULL);
+ }
+ }

ofono_modem_set_powered(modem, TRUE);

@@ -294,6 +364,7 @@ static int sim900_enable(struct ofono_modem *modem)
return -EINVAL;

g_at_chat_send(data->dlcs[SETUP_DLC], "ATE0", NULL, NULL, NULL, NULL);
+ g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CGMM", NULL,check_model, modem, NULL);

/* For obtain correct sms service number */
g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CSCS=\"GSM\"", NULL,
@@ -353,18 +424,20 @@ static void sim900_post_sim(struct ofono_modem *modem)

DBG("%p", modem);

- ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
- ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ if (data->modem_type != SIM800) {
+ ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
data->dlcs[SMS_DLC]);

- gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
- if (gprs == NULL)
- return;
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;

- gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
"atmodem", data->dlcs[GPRS_DLC]);
- if (gc)
- ofono_gprs_add_context(gprs, gc);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+ }
}

static void sim900_post_online(struct ofono_modem *modem)

--
2.7.4

Clement Viel

2018-11-13 15:00:09 UTC

Permalink

Hi Clement,

Post by Clement Viel
---
plugins/sim900.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 82 insertions(+), 9 deletions(-)
diff --git a/plugins/sim900.c b/plugins/sim900.c
index a7728cd..bf7b9ec 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -24,6 +24,7 @@
#endif
#include <errno.h>
+#include <string.h>
#include <stdlib.h>
#include <glib.h>
#include <gatchat.h>
@@ -60,13 +61,77 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
static const char *none_prefix[] = { NULL };
+typedef enum type {
+ SIM800,
+ SIM900,
+ SIM_UNKNOWN,

maybe SIMCOM_UNKNOWN ?

Yes, I do agree, updating patchset.

Post by Clement Viel
+} type_t;
+
struct sim900_data {
GIOChannel *device;
GAtMux *mux;
GAtChat * dlcs[NUM_DLC];
guint frame_size;
+ type_t modem_type;
};
+static void set_modem_type (const char *type, struct ofono_modem *modem)
+{
+ struct sim900_data *data = ofono_modem_get_data(modem);
+
+ if (strcmp(type, "sim800") == 0)
+ data->modem_type = SIM800;
+ else if (strcmp(type, "sim900") == 0)
+ data->modem_type = SIM900;
+ else
+ data->modem_type = SIM_UNKNOWN;
+ DBG("modem type is %d",data->modem_type);
+}
+
+static void mux_ready_notify(GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim900_data *data = ofono_modem_get_data(modem);
+ struct ofono_gprs *gprs = NULL;
+ struct ofono_gprs_context *gc;
+
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ data->dlcs[SMS_DLC]);
+
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
+
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
+ "atmodem", data->dlcs[GPRS_DLC]);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+
+
+}
+
+static void check_model(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ GAtResultIter iter;
+ char const *model;
+
+ DBG("");
+
+ g_at_result_iter_init(&iter, result);
+
+ while (g_at_result_iter_next(&iter, NULL)) {
+ if (!g_at_result_iter_next_unquoted_string(&iter, &model))
+ continue;
+
+ DBG("setting type %s", model);
+ if (strstr(model, "SIM800"))
+ set_modem_type("sim800", modem);
+ else if (strstr(model, "SIM900"))
+ set_modem_type("sim900", modem);
+ }
+}
+
static int sim900_probe(struct ofono_modem *modem)
{
struct sim900_data *data;
@@ -78,7 +143,6 @@ static int sim900_probe(struct ofono_modem *modem)
return -ENOMEM;
ofono_modem_set_data(modem, data);
-
return 0;
}
@@ -111,6 +175,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
GHashTable *options;
device = ofono_modem_get_string(modem, key);
+
if (device == NULL)
return NULL;
@@ -232,6 +297,11 @@ static void setup_internal_mux(struct ofono_modem *modem)
goto error;
}
}
+ if (data->modem_type == SIM800) {
+ for (i = 0; i<NUM_DLC; i++) {
+ g_at_chat_register(data->dlcs[i], "SMS Ready", mux_ready_notify, FALSE, modem, NULL);
+ }
+ }
ofono_modem_set_powered(modem, TRUE);
@@ -294,6 +364,7 @@ static int sim900_enable(struct ofono_modem *modem)
return -EINVAL;
g_at_chat_send(data->dlcs[SETUP_DLC], "ATE0", NULL, NULL, NULL, NULL);
+ g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CGMM", NULL,check_model, modem, NULL);
/* For obtain correct sms service number */
g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CSCS=\"GSM\"", NULL,
@@ -353,18 +424,20 @@ static void sim900_post_sim(struct ofono_modem *modem)
DBG("%p", modem);
- ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
- ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ if (data->modem_type != SIM800) {
+ ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
data->dlcs[SMS_DLC]);
- gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
- if (gprs == NULL)
- return;
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
- gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
"atmodem", data->dlcs[GPRS_DLC]);
- if (gc)
- ofono_gprs_add_context(gprs, gc);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+ }
}
static void sim900_post_online(struct ofono_modem *modem)
--

Regards,
Giacinto

Regards
Clement

Marcel Holtmann

2018-11-13 15:04:50 UTC

Permalink

Hi Clement,

Post by Clement Viel

maybe SIMCOM_UNKNOWN ?

Yes, I do agree, updating patchset.

and no typedef please.

Regards

Marcel

Jonas Bonn

2018-11-13 15:11:07 UTC

Permalink

Hi Clement,

I already provided review for this patch. This submission addresses
none of my earlier comments...

There's no need to typedef an enumeration; type_t is not a great name,
anyway... How about: enum simcom_model? Furthermore, it makes sense
to have the 'unknown' model be value 0.

Post by Clement Viel
struct sim900_data {
GIOChannel *device;
GAtMux *mux;
GAtChat * dlcs[NUM_DLC];
guint frame_size;
+ type_t modem_type;
};
+static void set_modem_type (const char *type, struct ofono_modem *modem)
+{
+ struct sim900_data *data = ofono_modem_get_data(modem);
+
+ if (strcmp(type, "sim800") == 0)
+ data->modem_type = SIM800;
+ else if (strcmp(type, "sim900") == 0)
+ data->modem_type = SIM900;
+ else
+ data->modem_type = SIM_UNKNOWN;
+ DBG("modem type is %d",data->modem_type);
+}
+

See my previous review of this patch. This function is pointless.

Post by Clement Viel
+static void mux_ready_notify(GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim900_data *data = ofono_modem_get_data(modem);
+ struct ofono_gprs *gprs = NULL;
+ struct ofono_gprs_context *gc;
+
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ data->dlcs[SMS_DLC]);
+
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
+
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
+ "atmodem", data->dlcs[GPRS_DLC]);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+
+
+}
+
+static void check_model(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ GAtResultIter iter;
+ char const *model;
+
+ DBG("");
+
+ g_at_result_iter_init(&iter, result);
+
+ while (g_at_result_iter_next(&iter, NULL)) {
+ if (!g_at_result_iter_next_unquoted_string(&iter, &model))
+ continue;
+
+ DBG("setting type %s", model);
+ if (strstr(model, "SIM800"))

strstr? The response begins with exactly this, as far as I can see.

Post by Clement Viel
+ set_modem_type("sim800", modem);

So you go from "SIM800" to "sim800" to an enumerated type... just short
out the call to set_modem_type().

Post by Clement Viel
+ else if (strstr(model, "SIM900"))
+ set_modem_type("sim900", modem);
+ }
+}
+
static int sim900_probe(struct ofono_modem *modem)
{
struct sim900_data *data;
@@ -78,7 +143,6 @@ static int sim900_probe(struct ofono_modem *modem)
return -ENOMEM;
ofono_modem_set_data(modem, data);
-

Random white space change

Post by Clement Viel
return 0;
}
@@ -111,6 +175,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
GHashTable *options;
device = ofono_modem_get_string(modem, key);
+

Random white space change.

Again, this is SIM900, specifically. Make the check for that. Or:

if (data->modem_type == SIM800)
return;

I think that's what you're doing here, effecively.

Post by Clement Viel
+ ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
data->dlcs[SMS_DLC]);
- gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
- if (gprs == NULL)
- return;
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
- gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
"atmodem", data->dlcs[GPRS_DLC]);
- if (gc)
- ofono_gprs_add_context(gprs, gc);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+ }
}
static void sim900_post_online(struct ofono_modem *modem)

/Jonas

Clement Viel

2018-11-13 15:28:51 UTC

Permalink

On Tue, Nov 13, 2018 at 04:11:07PM +0100, Jonas Bonn wrote:
Hi Jonas,

Hi Clement,
I already provided review for this patch. This submission addresses none of
my earlier comments...

There's no need to typedef an enumeration; type_t is not a great name,
anyway... How about: enum simcom_model? Furthermore, it makes sense to
have the 'unknown' model be value 0.

Yeah you're right, the next patchset will fix that.

See my previous review of this patch. This function is pointless.

I prefer adding a function than adding 10 lines to antoher. To my mind it's clearer.

strstr? The response begins with exactly this, as far as I can see.

Nope, CGMM does not reply exactly "SIM800" or "SIM900". Because there is no need to differentiate revision of modems for now, I prefer using strstr.

Post by Clement Viel
+ set_modem_type("sim800", modem);

So you go from "SIM800" to "sim800" to an enumerated type... just short out
the call to set_modem_type().

I find it simplier to work with enumerated than with strings.

Random white space change

Good catch !

Post by Clement Viel
return 0;
}
@@ -111,6 +175,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
GHashTable *options;
device = ofono_modem_get_string(modem, key);
+

Random white space change.

if (data->modem_type == SIM800)
return;
I think that's what you're doing here, effecively.

Good catch ! Because this check allows a SIMCOM_UNKNOWN modem to execute this code.
I'll check for SIM900.

/Jonas

Clement Viel

2018-11-13 15:33:24 UTC

Permalink

---
plugins/sim900.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 82 insertions(+), 9 deletions(-)

diff --git a/plugins/sim900.c b/plugins/sim900.c
index a7728cd..f280e5e 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -24,6 +24,7 @@
#endif

#include <errno.h>
+#include <string.h>
#include <stdlib.h>
#include <glib.h>
#include <gatchat.h>
@@ -60,13 +61,77 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",

static const char *none_prefix[] = { NULL };

+enum type {
+ SIM800,
+ SIM900,
+ SIMCOM_UNKNOWN,
+};
+
struct sim900_data {
GIOChannel *device;
GAtMux *mux;
GAtChat * dlcs[NUM_DLC];
guint frame_size;
+ enum type modem_type;
};

+static void set_modem_type (const char *type, struct ofono_modem *modem)
+{
+ struct sim900_data *data = ofono_modem_get_data(modem);
+
+ if (strcmp(type, "sim800") == 0)
+ data->modem_type = SIM800;
+ else if (strcmp(type, "sim900") == 0)
+ data->modem_type = SIM900;
+ else
+ data->modem_type = SIMCOM_UNKNOWN;
+ DBG("modem type is %d",data->modem_type);
+}
+
+static void mux_ready_notify(GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim900_data *data = ofono_modem_get_data(modem);
+ struct ofono_gprs *gprs = NULL;
+ struct ofono_gprs_context *gc;
+
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ data->dlcs[SMS_DLC]);
+
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
+
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
+ "atmodem", data->dlcs[GPRS_DLC]);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+
+
+}
+
+static void check_model(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ GAtResultIter iter;
+ char const *model;
+
+ DBG("");
+
+ g_at_result_iter_init(&iter, result);
+
+ while (g_at_result_iter_next(&iter, NULL)) {
+ if (!g_at_result_iter_next_unquoted_string(&iter, &model))
+ continue;
+
+ DBG("setting type %s", model);
+ if (strstr(model, "SIM800"))
+ set_modem_type("sim800", modem);
+ else if (strstr(model, "SIM900"))
+ set_modem_type("sim900", modem);
+ }
+}
+
static int sim900_probe(struct ofono_modem *modem)
{
struct sim900_data *data;
@@ -78,7 +143,6 @@ static int sim900_probe(struct ofono_modem *modem)
return -ENOMEM;

ofono_modem_set_data(modem, data);
-
return 0;
}

@@ -111,6 +175,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
GHashTable *options;

device = ofono_modem_get_string(modem, key);
+
if (device == NULL)
return NULL;

@@ -232,6 +297,11 @@ static void setup_internal_mux(struct ofono_modem *modem)
goto error;
}
}
+ if (data->modem_type == SIM800) {
+ for (i = 0; i<NUM_DLC; i++) {
+ g_at_chat_register(data->dlcs[i], "SMS Ready", mux_ready_notify, FALSE, modem, NULL);
+ }
+ }

ofono_modem_set_powered(modem, TRUE);

@@ -294,6 +364,7 @@ static int sim900_enable(struct ofono_modem *modem)
return -EINVAL;

g_at_chat_send(data->dlcs[SETUP_DLC], "ATE0", NULL, NULL, NULL, NULL);
+ g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CGMM", NULL,check_model, modem, NULL);

/* For obtain correct sms service number */
g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CSCS=\"GSM\"", NULL,
@@ -353,18 +424,20 @@ static void sim900_post_sim(struct ofono_modem *modem)

DBG("%p", modem);

- ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
- ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ if (data->modem_type != SIM800) {
+ ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
data->dlcs[SMS_DLC]);

- gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
- if (gprs == NULL)
- return;
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;

- gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
"atmodem", data->dlcs[GPRS_DLC]);
- if (gc)
- ofono_gprs_add_context(gprs, gc);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+ }
}

static void sim900_post_online(struct ofono_modem *modem)

--
2.7.4

Jonas Bonn

2018-11-13 16:53:56 UTC

Permalink

Hi,

Post by Clement Viel
---
plugins/sim900.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 82 insertions(+), 9 deletions(-)
diff --git a/plugins/sim900.c b/plugins/sim900.c
index a7728cd..f280e5e 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -24,6 +24,7 @@
#endif
#include <errno.h>
+#include <string.h>
#include <stdlib.h>
#include <glib.h>
#include <gatchat.h>
@@ -60,13 +61,77 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
static const char *none_prefix[] = { NULL };
+enum type {
+ SIM800,
+ SIM900,
+ SIMCOM_UNKNOWN,
+};
+

Makes more sense to have UNKNOWN be the first entry in the enumeration.

Post by Clement Viel
struct sim900_data {
GIOChannel *device;
GAtMux *mux;
GAtChat * dlcs[NUM_DLC];
guint frame_size;
+ enum type modem_type;
};
+static void set_modem_type (const char *type, struct ofono_modem *modem)
+{
+ struct sim900_data *data = ofono_modem_get_data(modem);
+
+ if (strcmp(type, "sim800") == 0)
+ data->modem_type = SIM800;
+ else if (strcmp(type, "sim900") == 0)
+ data->modem_type = SIM900;
+ else
+ data->modem_type = SIMCOM_UNKNOWN;
+ DBG("modem type is %d",data->modem_type);
+}
+
+static void mux_ready_notify(GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim900_data *data = ofono_modem_get_data(modem);
+ struct ofono_gprs *gprs = NULL;
+ struct ofono_gprs_context *gc;
+
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ data->dlcs[SMS_DLC]);
+
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
+
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,
+ "atmodem", data->dlcs[GPRS_DLC]);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+
+
+}
+
+static void check_model(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ GAtResultIter iter;
+ char const *model;
+
+ DBG("");
+
+ g_at_result_iter_init(&iter, result);
+
+ while (g_at_result_iter_next(&iter, NULL)) {
+ if (!g_at_result_iter_next_unquoted_string(&iter, &model))
+ continue;

The documentation doesn't indicate that the model type is given as an
arbitrary string at a random location in the response. It's pretty
specific about the SIM800/SIM900 token appearing as the first element of
the response. Is the documentation not correct on this point?

Post by Clement Viel
+
+ DBG("setting type %s", model);
+ if (strstr(model, "SIM800"))
+ set_modem_type("sim800", modem);

Instead of doing this dance with strings, just directly set the type:

data->modem_type = SIM800;

Whitespace.

Post by Clement Viel
return 0;
}
@@ -111,6 +175,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
GHashTable *options;
device = ofono_modem_get_string(modem, key);
+

Ditto.

Post by Clement Viel
if (device == NULL)
return NULL;
@@ -232,6 +297,11 @@ static void setup_internal_mux(struct ofono_modem *modem)
goto error;
}
}
+ if (data->modem_type == SIM800) {
+ for (i = 0; i<NUM_DLC; i++) {
+ g_at_chat_register(data->dlcs[i], "SMS Ready", mux_ready_notify, FALSE, modem, NULL);
+ }
+ }
ofono_modem_set_powered(modem, TRUE);
@@ -294,6 +364,7 @@ static int sim900_enable(struct ofono_modem *modem)
return -EINVAL;
g_at_chat_send(data->dlcs[SETUP_DLC], "ATE0", NULL, NULL, NULL, NULL);
+ g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CGMM", NULL,check_model, modem, NULL);
/* For obtain correct sms service number */
g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CSCS=\"GSM\"", NULL,
@@ -353,18 +424,20 @@ static void sim900_post_sim(struct ofono_modem *modem)
DBG("%p", modem);
- ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
- ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ if (data->modem_type != SIM800) {
+ ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
data->dlcs[SMS_DLC]);
- gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
- if (gprs == NULL)
- return;
+ gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);
+ if (gprs == NULL)
+ return;
- gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
+ gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM_SIM900,
"atmodem", data->dlcs[GPRS_DLC]);
- if (gc)
- ofono_gprs_add_context(gprs, gc);
+ if (gc)
+ ofono_gprs_add_context(gprs, gc);
+ }

All this is duplicated in mux_ready_notify... maybe move it a function
of its own...?

Post by Clement Viel
}
static void sim900_post_online(struct ofono_modem *modem)

Clement Viel

2018-11-13 17:11:22 UTC

Permalink

Post by Jonas Bonn
Hi,

Post by Clement Viel
---
plugins/sim900.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 82 insertions(+), 9 deletions(-)
diff --git a/plugins/sim900.c b/plugins/sim900.c
index a7728cd..f280e5e 100644
--- a/plugins/sim900.c
+++ b/plugins/sim900.c
@@ -24,6 +24,7 @@
#endif
#include <errno.h>
+#include <string.h>
#include <stdlib.h>
#include <glib.h>
#include <gatchat.h>
@@ -60,13 +61,77 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
static const char *none_prefix[] = { NULL };
+enum type {
+ SIM800,
+ SIM900,
+ SIMCOM_UNKNOWN,
+};
+

Makes more sense to have UNKNOWN be the first entry in the enumeration.

You're right !

Post by Jonas Bonn

The documentation doesn't indicate that the model type is given as an
arbitrary string at a random location in the response. It's pretty specific
about the SIM800/SIM900 token appearing as the first element of the
response. Is the documentation not correct on this point?

Post by Clement Viel
+
+ DBG("setting type %s", model);
+ if (strstr(model, "SIM800"))
+ set_modem_type("sim800", modem);

data->modem_type = SIM800;

Right again.

Post by Jonas Bonn

Whitespace.

Post by Clement Viel
return 0;
}
@@ -111,6 +175,7 @@ static GAtChat *open_device(struct ofono_modem *modem,
GHashTable *options;
device = ofono_modem_get_string(modem, key);
+

Ditto.

All this is duplicated in mux_ready_notify... maybe move it a function of
its own...?

this is duplicated because the ofono atoms cannot be created at the same point for the two modems :
for sim800, we must wait for an URC stating that channels have been created whereas sim900 is much straightforward.
That's why i've splitted : one in post_sim (sim900) another in mux_ready_notify (sim800).

Post by Jonas Bonn

Post by Clement Viel
}
static void sim900_post_online(struct ofono_modem *modem)