9:14 p.m.
---
plugins/sim900.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 98 insertions(+), 10 deletions(-)
diff --git a/plugins/sim900.c b/plugins/sim900.c
index a7728cd..c12576d 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,73 @@ 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;
+
+ 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;
+
+ 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 +139,6 @@ static int sim900_probe(struct ofono_modem *modem)
return -ENOMEM;
ofono_modem_set_data(modem, data);
-
return 0;
}
@@ -111,6 +171,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 +293,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 +360,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 +420,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)
@@ -391,9 +460,28 @@ static struct ofono_modem_driver sim900_driver = {
.post_online = sim900_post_online,
};
+
+static struct ofono_modem_driver sim800_driver = {
+ .name = "sim800",
+ .probe = sim900_probe,
+ .remove = sim900_remove,
+ .enable = sim900_enable,
+ .disable = sim900_disable,
+ .pre_sim = sim900_pre_sim,
+ .post_sim = sim900_post_sim,
+ .post_online = sim900_post_online,
+};
+
static int sim900_init(void)
{
- return ofono_modem_driver_register(&sim900_driver);
+ int ret = 0;
+ ret = ofono_modem_driver_register(&sim800_driver);
+ if (!ret)
+ return ret;
+ ret = ofono_modem_driver_register(&sim900_driver);
+ if (!ret)
+ return ret;
+ return ret;
}
static void sim900_exit(void)
--
2.7.4
9:14 p.m.
New subject: [PATCH 2/3] sim800: add documentation.
---
doc/sim800-modem.txt | 7 +++++++
1 file changed, 7 insertions(+)
create mode 100644 doc/sim800-modem.txt
diff --git a/doc/sim800-modem.txt b/doc/sim800-modem.txt
new file mode 100644
index 0000000..a299e97
--- /dev/null
+++ b/doc/sim800-modem.txt
@@ -0,0 +1,7 @@
+SIM800 modem usage
+===================
+
+To enable SIM800 module support you need to put the following
+udev rule into appropriate file in /{etc,lib}/udev/rules.d:
+
+KERNEL=="ttyS0", ENV{OFONO_DRIVER}="sim800"
--
2.7.4
12:50 a.m.
New subject: [PATCH 2/3] sim800: add documentation.
Hi Clement,
On 07/11/2018 11:14, Clement Viel wrote:
---
doc/sim800-modem.txt | 7 +++++++
1 file changed, 7 insertions(+)
create mode 100644 doc/sim800-modem.txt
diff --git a/doc/sim800-modem.txt b/doc/sim800-modem.txt
new file mode 100644
index 0000000..a299e97
--- /dev/null
+++ b/doc/sim800-modem.txt
@@ -0,0 +1,7 @@
+SIM800 modem usage
+===================
+
+To enable SIM800 module support you need to put the following
+udev rule into appropriate file in /{etc,lib}/udev/rules.d:
+
+KERNEL=="ttyS0", ENV{OFONO_DRIVER}="sim800"
Confused by this. Isn't the sim900 the serial modem and sim800 the USB
model? The above rule is for a strictly "serial" modem.
/Jonas
12:53 a.m.
New subject: [PATCH 2/3] sim800: add documentation.
Confused by this. Isn't the sim900 the serial modem and sim800
the USB
model? The above rule is for a strictly "serial" modem.
Nope, the SIM800 is a serial modem - it's, as far as I can tell, the next generation
after SIM900 or something along those lines. It also has a USB port, IIRC,
but I never had a chance to use that.
-Arsenijs
07.11.2018, 15:50, "Jonas Bonn" <jonas(a)norrbonn.se>:
> Hi Clement,
>
> On 07/11/2018 11:14, Clement Viel wrote:
>> ---
>> doc/sim800-modem.txt | 7 +++++++
>> 1 file changed, 7 insertions(+)
>> create mode 100644 doc/sim800-modem.txt
>>
>> diff --git a/doc/sim800-modem.txt b/doc/sim800-modem.txt
>> new file mode 100644
>> index 0000000..a299e97
>> --- /dev/null
>> +++ b/doc/sim800-modem.txt
>> @@ -0,0 +1,7 @@
>> +SIM800 modem usage
>> +===================
>> +
>> +To enable SIM800 module support you need to put the following
>> +udev rule into appropriate file in /{etc,lib}/udev/rules.d:
>> +
>> +KERNEL=="ttyS0", ENV{OFONO_DRIVER}="sim800"
>
Confused by this. Isn't the sim900 the serial modem and sim800
the USB
model? The above rule is for a strictly "serial" modem.
>
> /Jonas
> _______________________________________________
> ofono mailing list
> ofono(a)ofono.org
> https://lists.ofono.org/mailman/listinfo/ofono
9:14 p.m.
New subject: [PATCH 3/3] sim800: add udev detection.
---
plugins/udevng.c | 92 +++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 68 insertions(+), 24 deletions(-)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index ff6e1fc..ee6f9e8 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -711,8 +711,56 @@ static gboolean setup_telitqmi(struct modem_info *modem)
return TRUE;
}
-/* TODO: Not used as we have no simcom driver */
-static gboolean setup_simcom(struct modem_info *modem)
+
+static gboolean setup_sim800(struct modem_info *modem)
+{
+ const char *mdm = NULL, *aux = NULL, *gps = NULL, *diag = NULL;
+ GSList *list;
+
+ DBG("%s", modem->syspath);
+
+ for (list = modem->devices; list; list = list->next) {
+ struct device_info *info = list->data;
+
+ DBG("%s %s %s %s", info->devnode, info->interface,
+ info->number, info->label);
+
+ if (g_strcmp0(info->label, "aux") == 0) {
+ DBG("setting aux as info->devnode");
+ aux = info->devnode;
+ if (mdm != NULL)
+ break;
+ } else if (g_strcmp0(info->label, "modem") == 0) {
+ mdm = info->devnode;
+ if (aux != NULL)
+ break;
+ } else if (g_strcmp0(info->interface, "255/0/0") == 0) {
+ if (g_strcmp0(info->number, "00") == 0)
+ mdm = info->devnode;
+ else if (g_strcmp0(info->number, "01") == 0)
+ gps = info->devnode;
+ else if (g_strcmp0(info->number, "02") == 0)
+ aux = info->devnode;
+ else if (g_strcmp0(info->number, "03") == 0)
+ mdm = info->devnode;
+ }
+ }
+ DBG("modem=%s aux=%s gps=%s diag=%s", mdm, aux, gps, diag);
+
+ if (mdm == NULL) {
+ DBG("modem did not set up");
+ return FALSE;
+ }
+
+ ofono_modem_set_string(modem->modem, "Device", mdm);
+ ofono_modem_set_string(modem->modem, "Modem", mdm);
+
+ return TRUE;
+}
+
+
+
+static gboolean setup_sim900(struct modem_info *modem)
{
const char *mdm = NULL, *aux = NULL, *gps = NULL, *diag = NULL;
GSList *list;
@@ -962,6 +1010,8 @@ static gboolean setup_mbim(struct modem_info *modem)
ofono_modem_set_string(modem->modem, "Device", ctl);
ofono_modem_set_string(modem->modem, "NetworkInterface", net);
ofono_modem_set_string(modem->modem, "DescriptorFile", descriptors);
+ ofono_modem_set_string(modem->modem, "Vendor", modem->vendor);
+ ofono_modem_set_string(modem->modem, "Model", modem->model);
return TRUE;
}
@@ -1193,22 +1243,12 @@ static gboolean setup_xmm7xxx(struct modem_info *modem)
info->interface, info->number, info->label,
info->sysattr, info->subsystem);
- if (g_strcmp0(modem->model,"095a") == 0) {
- if (g_strcmp0(info->subsystem, "tty") == 0) {
- if (g_strcmp0(info->number, "00") == 0)
- mdm = info->devnode;
- } else if (g_strcmp0(info->subsystem, "net") == 0) {
- if (g_strcmp0(info->number, "06") == 0)
- net = info->devnode;
- }
- } else {
- if (g_strcmp0(info->subsystem, "tty") == 0) {
- if (g_strcmp0(info->number, "02") == 0)
- mdm = info->devnode;
- } else if (g_strcmp0(info->subsystem, "net") == 0) {
- if (g_strcmp0(info->number, "00") == 0)
- net = info->devnode;
- }
+ if (g_strcmp0(info->subsystem, "tty") == 0) {
+ if (g_strcmp0(info->number, "02") == 0)
+ mdm = info->devnode;
+ } else if (g_strcmp0(info->subsystem, "net") == 0) {
+ if (g_strcmp0(info->number, "00") == 0)
+ net = info->devnode;
}
}
@@ -1290,7 +1330,8 @@ static struct {
{ "nokia", setup_nokia },
{ "telit", setup_telit, "device/interface" },
{ "telitqmi", setup_telitqmi },
- { "simcom", setup_simcom },
+ { "sim800", setup_sim800 },
+ { "sim900", setup_sim900 },
{ "sim7100", setup_sim7100 },
{ "zte", setup_zte },
{ "icera", setup_icera },
@@ -1308,7 +1349,9 @@ static struct {
{ "calypso", setup_serial_modem },
{ "cinterion", setup_serial_modem },
{ "nokiacdma", setup_serial_modem },
+ { "sim800", setup_serial_modem },
{ "sim900", setup_serial_modem },
+ { "sim800", setup_serial_modem },
{ "wavecom", setup_wavecom },
{ "tc65", setup_tc65 },
{ "ehs6", setup_ehs6 },
@@ -1471,7 +1514,7 @@ static void add_serial_device(struct udev_device *dev)
const char *subsystem;
struct udev_device* mdev;
const char* driver;
-
+ DBG("adding %s interface", udev_device_get_devpath(dev));
mdev = get_serial_modem_device(dev);
if (!mdev) {
DBG("Device is missing required OFONO_DRIVER property");
@@ -1663,7 +1706,8 @@ static struct {
{ "alcatel", "option", "1bbb", "0017" },
{ "novatel", "option", "1410" },
{ "zte", "option", "19d2" },
- { "simcom", "option", "05c6", "9000" },
+ { "sim800", "option", "05c6", "9000" },
+ { "sim900", "option", "05c6", "9000" },
{ "sim7100", "option", "1e0e", "9001" },
{ "telit", "usbserial", "1bc7" },
{ "telit", "option", "1bc7" },
@@ -1693,7 +1737,6 @@ static struct {
{ "xmm7xxx", "cdc_ncm", "8087" },
{ }
};
-
static void check_usb_device(struct udev_device *device)
{
struct udev_device *usb_device;
@@ -1722,9 +1765,10 @@ static void check_usb_device(struct udev_device *device)
udev_device_get_parent_with_subsystem_devtype(
device, "usb", "usb_interface");
- if (usb_interface)
+ if (usb_interface) {
driver = udev_device_get_property_value(
usb_interface, "OFONO_DRIVER");
+ }
}
if (driver == NULL) {
@@ -1801,7 +1845,7 @@ static gboolean create_modem(gpointer key, gpointer value, gpointer
user_data)
struct modem_info *modem = value;
const char *syspath = key;
unsigned int i;
-
+ DBG("");
if (modem->modem != NULL)
return FALSE;
--
2.7.4
12:30 a.m.
New subject: [PATCH 3/3] sim800: add udev detection.
Hi Clement,
On 07/11/2018 11:14, Clement Viel wrote:
/Jonas
---
plugins/udevng.c | 92 +++++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 68 insertions(+), 24 deletions(-)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index ff6e1fc..ee6f9e8 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -711,8 +711,56 @@ static gboolean setup_telitqmi(struct modem_info *modem)
return TRUE;
}
-/* TODO: Not used as we have no simcom driver */
-static gboolean setup_simcom(struct modem_info *modem)
+
+static gboolean setup_sim800(struct modem_info *modem)
+{
+ const char *mdm = NULL, *aux = NULL, *gps = NULL, *diag = NULL;
+ GSList *list;
+
+ DBG("%s", modem->syspath);
+
+ for (list = modem->devices; list; list = list->next) {
+ struct device_info *info = list->data;
+
+ DBG("%s %s %s %s", info->devnode, info->interface,
+ info->number, info->label);
+
+ if (g_strcmp0(info->label, "aux") == 0) {
+ DBG("setting aux as info->devnode");
+ aux = info->devnode;
+ if (mdm != NULL)
+ break;
+ } else if (g_strcmp0(info->label, "modem") == 0) {
+ mdm = info->devnode;
+ if (aux != NULL)
+ break;
+ } else if (g_strcmp0(info->interface, "255/0/0") == 0) {
+ if (g_strcmp0(info->number, "00") == 0)
+ mdm = info->devnode;
+ else if (g_strcmp0(info->number, "01") == 0)
+ gps = info->devnode;
+ else if (g_strcmp0(info->number, "02") == 0)
+ aux = info->devnode;
+ else if (g_strcmp0(info->number, "03") == 0)
+ mdm = info->devnode;
+ }
+ }
+ DBG("modem=%s aux=%s gps=%s diag=%s", mdm, aux, gps, diag);
+
gps is unused.
+ if (mdm == NULL) {
+ DBG("modem did not set up");
+ return FALSE;
+ }
+
+ ofono_modem_set_string(modem->modem, "Device", mdm);
You probably mean 'aux' here.
+ ofono_modem_set_string(modem->modem, "Modem", mdm);
+
+ return TRUE;
+}
+
+
+
+static gboolean setup_sim900(struct modem_info *modem)
{
const char *mdm = NULL, *aux = NULL, *gps = NULL, *diag = NULL;
GSList *list;
@@ -962,6 +1010,8 @@ static gboolean setup_mbim(struct modem_info *modem)
ofono_modem_set_string(modem->modem, "Device", ctl);
ofono_modem_set_string(modem->modem, "NetworkInterface", net);
ofono_modem_set_string(modem->modem, "DescriptorFile", descriptors);
+ ofono_modem_set_string(modem->modem, "Vendor", modem->vendor);
+ ofono_modem_set_string(modem->modem, "Model", modem->model);
I don't see that these strings are being used anywhere in the modem driver.
return TRUE;
}
@@ -1193,22 +1243,12 @@ static gboolean setup_xmm7xxx(struct modem_info *modem)
info->interface, info->number, info->label,
info->sysattr, info->subsystem);
- if (g_strcmp0(modem->model,"095a") == 0) {
- if (g_strcmp0(info->subsystem, "tty") == 0) {
- if (g_strcmp0(info->number, "00") == 0)
- mdm = info->devnode;
- } else if (g_strcmp0(info->subsystem, "net") == 0) {
- if (g_strcmp0(info->number, "06") == 0)
- net = info->devnode;
- }
- } else {
- if (g_strcmp0(info->subsystem, "tty") == 0) {
- if (g_strcmp0(info->number, "02") == 0)
- mdm = info->devnode;
- } else if (g_strcmp0(info->subsystem, "net") == 0) {
- if (g_strcmp0(info->number, "00") == 0)
- net = info->devnode;
- }
+ if (g_strcmp0(info->subsystem, "tty") == 0) {
+ if (g_strcmp0(info->number, "02") == 0)
+ mdm = info->devnode;
+ } else if (g_strcmp0(info->subsystem, "net") == 0) {
+ if (g_strcmp0(info->number, "00") == 0)
+ net = info->devnode;
}
}
This change doesn't look like it belongs to this patch.
@@ -1290,7 +1330,8 @@ static struct {
{ "nokia", setup_nokia },
{ "telit", setup_telit, "device/interface" },
{ "telitqmi", setup_telitqmi },
- { "simcom", setup_simcom },
+ { "sim800", setup_sim800 },
+ { "sim900", setup_sim900 },
Is the sim900 not a serial modem? These setup routines are really only
for USB devices.
{ "sim7100", setup_sim7100 },
{ "zte", setup_zte },
{ "icera", setup_icera },
@@ -1308,7 +1349,9 @@ static struct {
{ "calypso", setup_serial_modem },
{ "cinterion", setup_serial_modem },
{ "nokiacdma", setup_serial_modem },
+ { "sim800", setup_serial_modem },
{ "sim900", setup_serial_modem },
+ { "sim800", setup_serial_modem },
You've added the same line twice. Furthermore, the sim800 is a USB
modem (???) so it should not be set up in this way.
{ "wavecom", setup_wavecom },
{ "tc65", setup_tc65 },
{ "ehs6", setup_ehs6 },
@@ -1471,7 +1514,7 @@ static void add_serial_device(struct udev_device *dev)
const char *subsystem;
struct udev_device* mdev;
const char* driver;
-
+ DBG("adding %s interface", udev_device_get_devpath(dev));
mdev = get_serial_modem_device(dev);
if (!mdev) {
DBG("Device is missing required OFONO_DRIVER property");
@@ -1663,7 +1706,8 @@ static struct {
{ "alcatel", "option", "1bbb", "0017" },
{ "novatel", "option", "1410" },
{ "zte", "option", "19d2" },
- { "simcom", "option", "05c6", "9000" },
+ { "sim800", "option", "05c6", "9000" }
+ { "sim900", "option", "05c6", "9000" },
The sim900 is a serial modem... there is no vendor id nor product id.
{ "sim7100", "option", "1e0e",
"9001" },
{ "telit", "usbserial", "1bc7" },
{ "telit", "option", "1bc7" },
@@ -1693,7 +1737,6 @@ static struct {
{ "xmm7xxx", "cdc_ncm", "8087" },
{ }
};
-
static void check_usb_device(struct udev_device *device)
{
struct udev_device *usb_device;
@@ -1722,9 +1765,10 @@ static void check_usb_device(struct udev_device *device)
udev_device_get_parent_with_subsystem_devtype(
device, "usb", "usb_interface");
- if (usb_interface)
+ if (usb_interface) {
driver = udev_device_get_property_value(
usb_interface, "OFONO_DRIVER");
+ }
}
No.
if (driver == NULL) {
@@ -1801,7 +1845,7 @@ static gboolean create_modem(gpointer key, gpointer value, gpointer
user_data)
struct modem_info *modem = value;
const char *syspath = key;
unsigned int i;
-
+ DBG("");
if (modem->modem != NULL)
return FALSE;
Doesn't belong to this patch, if at all.
12:58 a.m.
New subject: [PATCH 3/3] sim800: add udev detection.
On Wed, Nov 07, 2018 at 02:30:43PM +0100, Jonas Bonn wrote:
Hi Clement,
On 07/11/2018 11:14, Clement Viel wrote:
>---
> plugins/udevng.c | 92 +++++++++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 68 insertions(+), 24 deletions(-)
>
>diff --git a/plugins/udevng.c b/plugins/udevng.c
>index ff6e1fc..ee6f9e8 100644
>--- a/plugins/udevng.c
>+++ b/plugins/udevng.c
>@@ -711,8 +711,56 @@ static gboolean setup_telitqmi(struct modem_info *modem)
> return TRUE;
> }
>-/* TODO: Not used as we have no simcom driver */
>-static gboolean setup_simcom(struct modem_info *modem)
>+
>+static gboolean setup_sim800(struct modem_info *modem)
>+{
>+ const char *mdm = NULL, *aux = NULL, *gps = NULL, *diag = NULL;
>+ GSList *list;
>+
>+ DBG("%s", modem->syspath);
>+
>+ for (list = modem->devices; list; list = list->next) {
>+ struct device_info *info = list->data;
>+
>+ DBG("%s %s %s %s", info->devnode, info->interface,
>+ info->number, info->label);
>+
>+ if (g_strcmp0(info->label, "aux") == 0) {
>+ DBG("setting aux as info->devnode");
>+ aux = info->devnode;
>+ if (mdm != NULL)
>+ break;
>+ } else if (g_strcmp0(info->label, "modem") == 0) {
>+ mdm = info->devnode;
>+ if (aux != NULL)
>+ break;
>+ } else if (g_strcmp0(info->interface, "255/0/0") == 0) {
>+ if (g_strcmp0(info->number, "00") == 0)
>+ mdm = info->devnode;
>+ else if (g_strcmp0(info->number, "01") == 0)
>+ gps = info->devnode;
>+ else if (g_strcmp0(info->number, "02") == 0)
>+ aux = info->devnode;
>+ else if (g_strcmp0(info->number, "03") == 0)
>+ mdm = info->devnode;
>+ }
>+ }
>+ DBG("modem=%s aux=%s gps=%s diag=%s", mdm, aux, gps, diag);
>+
gps is unused.
>+ if (mdm == NULL) {
>+ DBG("modem did not set up");
>+ return FALSE;
>+ }
>+
>+ ofono_modem_set_string(modem->modem, "Device", mdm);
You probably mean 'aux' here.
>+ ofono_modem_set_string(modem->modem, "Modem", mdm);
>+
>+ return TRUE;
>+}
>+
>+
>+
>+static gboolean setup_sim900(struct modem_info *modem)
> {
> const char *mdm = NULL, *aux = NULL, *gps = NULL, *diag = NULL;
> GSList *list;
>@@ -962,6 +1010,8 @@ static gboolean setup_mbim(struct modem_info *modem)
> ofono_modem_set_string(modem->modem, "Device", ctl);
> ofono_modem_set_string(modem->modem, "NetworkInterface", net);
> ofono_modem_set_string(modem->modem, "DescriptorFile", descriptors);
>+ ofono_modem_set_string(modem->modem, "Vendor", modem->vendor);
>+ ofono_modem_set_string(modem->modem, "Model", modem->model);
I don't see that these strings are being used anywhere in the modem driver.
> return TRUE;
> }
>@@ -1193,22 +1243,12 @@ static gboolean setup_xmm7xxx(struct modem_info *modem)
> info->interface, info->number, info->label,
> info->sysattr, info->subsystem);
>- if (g_strcmp0(modem->model,"095a") == 0) {
>- if (g_strcmp0(info->subsystem, "tty") == 0) {
>- if (g_strcmp0(info->number, "00") == 0)
>- mdm = info->devnode;
>- } else if (g_strcmp0(info->subsystem, "net") == 0) {
>- if (g_strcmp0(info->number, "06") == 0)
>- net = info->devnode;
>- }
>- } else {
>- if (g_strcmp0(info->subsystem, "tty") == 0) {
>- if (g_strcmp0(info->number, "02") == 0)
>- mdm = info->devnode;
>- } else if (g_strcmp0(info->subsystem, "net") == 0) {
>- if (g_strcmp0(info->number, "00") == 0)
>- net = info->devnode;
>- }
>+ if (g_strcmp0(info->subsystem, "tty") == 0) {
>+ if (g_strcmp0(info->number, "02") == 0)
>+ mdm = info->devnode;
>+ } else if (g_strcmp0(info->subsystem, "net") == 0) {
>+ if (g_strcmp0(info->number, "00") == 0)
>+ net = info->devnode;
> }
> }
This change doesn't look like it belongs to this patch.
>@@ -1290,7 +1330,8 @@ static struct {
> { "nokia", setup_nokia },
> { "telit", setup_telit, "device/interface" },
> { "telitqmi", setup_telitqmi },
>- { "simcom", setup_simcom },
>+ { "sim800", setup_sim800 },
>+ { "sim900", setup_sim900 },
Is the sim900 not a serial modem? These setup routines are really only for
USB devices.
> { "sim7100", setup_sim7100 },
> { "zte", setup_zte },
> { "icera", setup_icera },
>@@ -1308,7 +1349,9 @@ static struct {
> { "calypso", setup_serial_modem },
> { "cinterion", setup_serial_modem },
> { "nokiacdma", setup_serial_modem },
>+ { "sim800", setup_serial_modem },
> { "sim900", setup_serial_modem },
>+ { "sim800", setup_serial_modem },
You've added the same line twice. Furthermore, the sim800 is a USB modem
(???) so it should not be set up in this way.
> { "wavecom", setup_wavecom },
> { "tc65", setup_tc65 },
> { "ehs6", setup_ehs6 },
>@@ -1471,7 +1514,7 @@ static void add_serial_device(struct udev_device *dev)
> const char *subsystem;
> struct udev_device* mdev;
> const char* driver;
>-
>+ DBG("adding %s interface", udev_device_get_devpath(dev));
> mdev = get_serial_modem_device(dev);
> if (!mdev) {
> DBG("Device is missing required OFONO_DRIVER property");
>@@ -1663,7 +1706,8 @@ static struct {
> { "alcatel", "option", "1bbb", "0017" },
> { "novatel", "option", "1410" },
> { "zte", "option", "19d2" },
>- { "simcom", "option", "05c6", "9000" },
>+ { "sim800", "option", "05c6", "9000" }
>+ { "sim900", "option", "05c6", "9000" },
The sim900 is a serial modem... there is no vendor id nor product id.
> { "sim7100", "option", "1e0e", "9001" },
> { "telit", "usbserial", "1bc7" },
> { "telit", "option", "1bc7" },
>@@ -1693,7 +1737,6 @@ static struct {
> { "xmm7xxx", "cdc_ncm", "8087" },
> { }
> };
>-
> static void check_usb_device(struct udev_device *device)
> {
> struct udev_device *usb_device;
>@@ -1722,9 +1765,10 @@ static void check_usb_device(struct udev_device *device)
> udev_device_get_parent_with_subsystem_devtype(
> device, "usb", "usb_interface");
>- if (usb_interface)
>+ if (usb_interface) {
> driver = udev_device_get_property_value(
> usb_interface, "OFONO_DRIVER");
>+ }
> }
No.
> if (driver == NULL) {
>@@ -1801,7 +1845,7 @@ static gboolean create_modem(gpointer key, gpointer value,
gpointer user_data)
> struct modem_info *modem = value;
> const char *syspath = key;
> unsigned int i;
>-
>+ DBG("");
> if (modem->modem != NULL)
> return FALSE;
Doesn't belong to this patch, if at all.
>
/Jonas
Hi Jonas,
Thanks for the review, this patch is all wrong because it's not the good one...litlle
bit tired, i'm sorry.
12:47 a.m.
Hi Clement,
On 07/11/2018 11:14, Clement Viel wrote:
---
plugins/sim900.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 98 insertions(+), 10 deletions(-)
diff --git a/plugins/sim900.c b/plugins/sim900.c
index a7728cd..c12576d 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,73 @@ static char *dlc_prefixes[NUM_DLC] = { "Voice: ",
"Net: ", "SMS: ",
static const char *none_prefix[] = { NULL };
+typedef enum type {
+ SIM800,
+ SIM900,
+ SIM_UNKNOWN,
+} type_t;
+
type_t is not a great name... furthermore, I don't think ofono wants
typedef'd enums. Just use: enum simcom_model {...} or similar.
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);
+}
+
This function is unnecessary... see below.
+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;
+
+ 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;
+
+ if (strstr(model, "SIM800"))
+ set_modem_type("sim800", modem);
+ else if (strstr(model, "SIM900"))
+ set_modem_type("sim900", modem);
Just set the type directly here... no need to pass an arbitrary string
through a secondary function to accomplish that.
Furthermore, the result is exactly "SIM800" or "SIM900", right? No
need
for strstr, use strcmp.
+ }
+}
+
static int sim900_probe(struct ofono_modem *modem)
{
struct sim900_data *data;
@@ -78,7 +139,6 @@ static int sim900_probe(struct ofono_modem *modem)
return -ENOMEM;
ofono_modem_set_data(modem, data);
-
return 0;
}
@@ -111,6 +171,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 +293,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 +360,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 +420,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) {
How about "== SIM900" because that's what this is for.
+ 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)
@@ -391,9 +460,28 @@ static struct ofono_modem_driver sim900_driver = {
.post_online = sim900_post_online,
};
+
+static struct ofono_modem_driver sim800_driver = {
+ .name = "sim800",
+ .probe = sim900_probe,
+ .remove = sim900_remove,
+ .enable = sim900_enable,
+ .disable = sim900_disable,
+ .pre_sim = sim900_pre_sim,
+ .post_sim = sim900_post_sim,
+ .post_online = sim900_post_online,
+};
This is identical to the sim900_driver.
Your driver is already taking into account the modem model and adjusting
functionality accordingly. You don't need to specify a new driver for
the sim800 where the only thing that changes is the name.
+
static int sim900_init(void)
{
- return ofono_modem_driver_register(&sim900_driver);
+ int ret = 0;
+ ret = ofono_modem_driver_register(&sim800_driver);
+ if (!ret)
+ return ret;
+ ret = ofono_modem_driver_register(&sim900_driver);
+ if (!ret)
+ return ret;
+ return ret;
}
static void sim900_exit(void)
/Jonas
1411
days inactive
1411
days old
7 comments
3 participants
participants (3)
-
Clement Viel -
Jonas Bonn -
Pičugins Arsenijs