* [PATCH 1/3] sim800: add support for sim800 modem.
@ 2018-11-07 10:14 Clement Viel
2018-11-07 10:14 ` [PATCH 2/3] sim800: add documentation Clement Viel
` (2 more replies)
0 siblings, 3 replies; 17+ messages in thread
From: Clement Viel @ 2018-11-07 10:14 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5154 bytes --]
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 2/3] sim800: add documentation.
2018-11-07 10:14 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
@ 2018-11-07 10:14 ` Clement Viel
2018-11-07 13:50 ` Jonas Bonn
2018-11-07 10:14 ` [PATCH 3/3] sim800: add udev detection Clement Viel
2018-11-07 13:47 ` [PATCH 1/3] sim800: add support for sim800 modem Jonas Bonn
2 siblings, 1 reply; 17+ messages in thread
From: Clement Viel @ 2018-11-07 10:14 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 517 bytes --]
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] sim800: add documentation.
2018-11-07 10:14 ` [PATCH 2/3] sim800: add documentation Clement Viel
@ 2018-11-07 13:50 ` Jonas Bonn
2018-11-07 13:53 ` =?unknown-8bit?q?Pi=C4=8Dugins?= Arsenijs
0 siblings, 1 reply; 17+ messages in thread
From: Jonas Bonn @ 2018-11-07 13:50 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 747 bytes --]
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 2/3] sim800: add documentation.
2018-11-07 13:50 ` Jonas Bonn
@ 2018-11-07 13:53 ` =?unknown-8bit?q?Pi=C4=8Dugins?= Arsenijs
0 siblings, 0 replies; 17+ messages in thread
From: =?unknown-8bit?q?Pi=C4=8Dugins?= Arsenijs @ 2018-11-07 13:53 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1359 bytes --]
> 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@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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] sim800: add udev detection.
2018-11-07 10:14 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
2018-11-07 10:14 ` [PATCH 2/3] sim800: add documentation Clement Viel
@ 2018-11-07 10:14 ` Clement Viel
2018-11-07 13:30 ` Jonas Bonn
2018-11-07 13:47 ` [PATCH 1/3] sim800: add support for sim800 modem Jonas Bonn
2 siblings, 1 reply; 17+ messages in thread
From: Clement Viel @ 2018-11-07 10:14 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5614 bytes --]
---
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
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-07 10:14 ` [PATCH 3/3] sim800: add udev detection Clement Viel
@ 2018-11-07 13:30 ` Jonas Bonn
2018-11-07 13:58 ` Clement Viel
0 siblings, 1 reply; 17+ messages in thread
From: Jonas Bonn @ 2018-11-07 13:30 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 6638 bytes --]
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-07 13:30 ` Jonas Bonn
@ 2018-11-07 13:58 ` Clement Viel
0 siblings, 0 replies; 17+ messages in thread
From: Clement Viel @ 2018-11-07 13:58 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 7023 bytes --]
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.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/3] sim800: add support for sim800 modem.
2018-11-07 10:14 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
2018-11-07 10:14 ` [PATCH 2/3] sim800: add documentation Clement Viel
2018-11-07 10:14 ` [PATCH 3/3] sim800: add udev detection Clement Viel
@ 2018-11-07 13:47 ` Jonas Bonn
2 siblings, 0 replies; 17+ messages in thread
From: Jonas Bonn @ 2018-11-07 13:47 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 6376 bytes --]
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] sim800: add udev detection.
@ 2018-11-07 14:00 Clement Viel
2018-11-07 14:02 ` Jonas Bonn
0 siblings, 1 reply; 17+ messages in thread
From: Clement Viel @ 2018-11-07 14:00 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 501 bytes --]
---
plugins/udevng.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index ff6e1fc..d0a849d 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1308,6 +1308,7 @@ static struct {
{ "calypso", setup_serial_modem },
{ "cinterion", setup_serial_modem },
{ "nokiacdma", setup_serial_modem },
+ { "sim800", setup_serial_modem },
{ "sim900", setup_serial_modem },
{ "wavecom", setup_wavecom },
{ "tc65", setup_tc65 },
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-07 14:00 [PATCH 3/3] sim800: add udev detection Clement Viel
@ 2018-11-07 14:02 ` Jonas Bonn
2018-11-07 14:08 ` Clement Viel
0 siblings, 1 reply; 17+ messages in thread
From: Jonas Bonn @ 2018-11-07 14:02 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 744 bytes --]
On 07/11/2018 15:00, Clement Viel wrote:
> ---
> plugins/udevng.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index ff6e1fc..d0a849d 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -1308,6 +1308,7 @@ static struct {
> { "calypso", setup_serial_modem },
> { "cinterion", setup_serial_modem },
> { "nokiacdma", setup_serial_modem },
> + { "sim800", setup_serial_modem },
Right... that's much simpler. And given that you are extending the
sim900 driver to handle the sim800 model, you don't need the above line
either.
/Jonas
> { "sim900", setup_serial_modem },
> { "wavecom", setup_wavecom },
> { "tc65", setup_tc65 },
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-07 14:02 ` Jonas Bonn
@ 2018-11-07 14:08 ` Clement Viel
2018-11-08 16:59 ` Denis Kenzior
0 siblings, 1 reply; 17+ messages in thread
From: Clement Viel @ 2018-11-07 14:08 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]
On Wed, Nov 07, 2018 at 03:02:54PM +0100, Jonas Bonn wrote:
>
>
> On 07/11/2018 15:00, Clement Viel wrote:
> >---
> > plugins/udevng.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> >diff --git a/plugins/udevng.c b/plugins/udevng.c
> >index ff6e1fc..d0a849d 100644
> >--- a/plugins/udevng.c
> >+++ b/plugins/udevng.c
> >@@ -1308,6 +1308,7 @@ static struct {
> > { "calypso", setup_serial_modem },
> > { "cinterion", setup_serial_modem },
> > { "nokiacdma", setup_serial_modem },
> >+ { "sim800", setup_serial_modem },
>
> Right... that's much simpler. And given that you are extending the sim900
> driver to handle the sim800 model, you don't need the above line either.
>
> /Jonas
>
> > { "sim900", setup_serial_modem },
> > { "wavecom", setup_wavecom },
> > { "tc65", setup_tc65 },
> >
I think I must keep this line because the udev rule added by the user will specify that modem is "OFONO_DRIVER=sim800" so udevng.c must know what to do with that.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-07 14:08 ` Clement Viel
@ 2018-11-08 16:59 ` Denis Kenzior
2018-11-12 14:16 ` Clement Viel
2018-11-12 14:21 ` Clement Viel
0 siblings, 2 replies; 17+ messages in thread
From: Denis Kenzior @ 2018-11-08 16:59 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 596 bytes --]
Hi Clement,
>>> + { "sim800", setup_serial_modem }, > I think I must keep this line because the udev rule added by the user
will specify that modem is "OFONO_DRIVER=sim800" so udevng.c must know
what to do with that.
This is kind of weird that we have a driver line for a modem driver that
doesn't exist. We never do things this way. Why don't you just modify
the driver instructions to have OFONO_DRIVER=sim900 instead?
Also, we could simply rename the sim900 driver into sim_x00 or
simcom_serial or something too. That could make things a bit more clear.
Regards,
-Denis
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-08 16:59 ` Denis Kenzior
@ 2018-11-12 14:16 ` Clement Viel
2018-11-12 16:34 ` Denis Kenzior
2018-11-12 14:21 ` Clement Viel
1 sibling, 1 reply; 17+ messages in thread
From: Clement Viel @ 2018-11-12 14:16 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]
On Thu, Nov 08, 2018 at 10:59:30AM -0600, Denis Kenzior wrote:
Hi Denis,
> Hi Clement,
> >>>+ { "sim800", setup_serial_modem }, > I think I must keep this line
> >>>because the udev rule added by the user
> will specify that modem is "OFONO_DRIVER=sim800" so udevng.c must know what
> to do with that.
> This is kind of weird that we have a driver line for a modem driver that
> doesn't exist. We never do things this way. Why don't you just modify the
> driver instructions to have OFONO_DRIVER=sim900 instead?
>
I agree with you, I was thinking the sim800 must be mentioned to avoid confusing
doc/sim900.txt to mention that sim800 is supported by this driver. Then, keep in
the users.
> Also, we could simply rename the sim900 driver into sim_x00 or simcom_serial
> or something too. That could make things a bit more clear.
I think we can keep that in mind for further development.
>
> Regards,
> -Denis
Just tell me whether I should add mention of sim800 in sim900.txt or rename sim900.txt
in simcom.txt to mention the two drivers. Then I'll upload the patchset with doc in it.
Regards
Clement
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH 3/3] sim800: add udev detection.
2018-11-08 16:59 ` Denis Kenzior
2018-11-12 14:16 ` Clement Viel
@ 2018-11-12 14:21 ` Clement Viel
1 sibling, 0 replies; 17+ messages in thread
From: Clement Viel @ 2018-11-12 14:21 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 786 bytes --]
On Thu, Nov 08, 2018 at 10:59:30AM -0600, Denis Kenzior wrote:
> Hi Clement,
> >>>+ { "sim800", setup_serial_modem }, > I think I must keep this line
> >>>because the udev rule added by the user
> will specify that modem is "OFONO_DRIVER=sim800" so udevng.c must know what
> to do with that.
> This is kind of weird that we have a driver line for a modem driver that
> doesn't exist. We never do things this way. Why don't you just modify the
> driver instructions to have OFONO_DRIVER=sim900 instead?
>
> Also, we could simply rename the sim900 driver into sim_x00 or simcom_serial
> or something too. That could make things a bit more clear.
>
> Regards,
> -Denis
Moreover if we go for the OFONO_DRIVER=sim900 solution, plugins/udevng.c will stay
the same.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] sim800: add support for sim800 modem.
@ 2018-10-15 17:27 Clement Viel
2018-10-15 17:27 ` [PATCH 3/3] sim800: add udev detection Clement Viel
0 siblings, 1 reply; 17+ messages in thread
From: Clement Viel @ 2018-10-15 17:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 4267 bytes --]
---
plugins/sim900.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 80 insertions(+), 9 deletions(-)
diff --git a/plugins/sim900.c b/plugins/sim900.c
index a7728cd..9f3f334 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,59 @@ 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 (struct ofono_modem *modem)
+{
+ struct sim900_data *data = ofono_modem_get_data(modem);
+ const char *path = ofono_modem_get_path(modem);
+
+ if (strstr(path, "sim800"))
+ data->modem_type = SIM800;
+ else if (strstr(path, "sim900"))
+ data->modem_type = SIM900;
+ else
+ data->modem_type = SIM_UNKNOWN;
+
+}
+
+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;
+ static int notified = 0;
+
+ if (!notified) {
+ 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 int sim900_probe(struct ofono_modem *modem)
{
struct sim900_data *data;
@@ -79,6 +126,7 @@ static int sim900_probe(struct ofono_modem *modem)
ofono_modem_set_data(modem, data);
+ set_modem_type(modem);
return 0;
}
@@ -111,6 +159,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 +281,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);
@@ -353,18 +407,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 +447,24 @@ 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);
+ ret += ofono_modem_driver_register(&sim900_driver);
+ return ret;
}
static void sim900_exit(void)
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 3/3] sim800: add udev detection
2018-10-15 17:27 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
@ 2018-10-15 17:27 ` Clement Viel
0 siblings, 0 replies; 17+ messages in thread
From: Clement Viel @ 2018-10-15 17:27 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 501 bytes --]
---
plugins/udevng.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index 3c39e68..202519e 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1300,6 +1300,7 @@ static struct {
{ "calypso", setup_serial_modem },
{ "cinterion", setup_serial_modem },
{ "nokiacdma", setup_serial_modem },
+ { "sim800", setup_serial_modem },
{ "sim900", setup_serial_modem },
{ "wavecom", setup_wavecom },
{ "tc65", setup_tc65 },
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 1/3] sim800: add support for sim800 modem.
@ 2018-10-03 13:28 Clement Viel
2018-10-03 13:28 ` [PATCH 3/3] sim800: add udev detection Clement Viel
0 siblings, 1 reply; 17+ messages in thread
From: Clement Viel @ 2018-10-03 13:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 4267 bytes --]
---
plugins/sim900.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 80 insertions(+), 9 deletions(-)
diff --git a/plugins/sim900.c b/plugins/sim900.c
index a7728cd..9f3f334 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,59 @@ 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 (struct ofono_modem *modem)
+{
+ struct sim900_data *data = ofono_modem_get_data(modem);
+ const char *path = ofono_modem_get_path(modem);
+
+ if (strstr(path, "sim800"))
+ data->modem_type = SIM800;
+ else if (strstr(path, "sim900"))
+ data->modem_type = SIM900;
+ else
+ data->modem_type = SIM_UNKNOWN;
+
+}
+
+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;
+ static int notified = 0;
+
+ if (!notified) {
+ 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 int sim900_probe(struct ofono_modem *modem)
{
struct sim900_data *data;
@@ -79,6 +126,7 @@ static int sim900_probe(struct ofono_modem *modem)
ofono_modem_set_data(modem, data);
+ set_modem_type(modem);
return 0;
}
@@ -111,6 +159,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 +281,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);
@@ -353,18 +407,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 +447,24 @@ 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);
+ ret += ofono_modem_driver_register(&sim900_driver);
+ return ret;
}
static void sim900_exit(void)
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 3/3] sim800: add udev detection
2018-10-03 13:28 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
@ 2018-10-03 13:28 ` Clement Viel
0 siblings, 0 replies; 17+ messages in thread
From: Clement Viel @ 2018-10-03 13:28 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 501 bytes --]
---
plugins/udevng.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index 3c39e68..202519e 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -1300,6 +1300,7 @@ static struct {
{ "calypso", setup_serial_modem },
{ "cinterion", setup_serial_modem },
{ "nokiacdma", setup_serial_modem },
+ { "sim800", setup_serial_modem },
{ "sim900", setup_serial_modem },
{ "wavecom", setup_wavecom },
{ "tc65", setup_tc65 },
--
2.7.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2018-11-12 16:34 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-07 10:14 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
2018-11-07 10:14 ` [PATCH 2/3] sim800: add documentation Clement Viel
2018-11-07 13:50 ` Jonas Bonn
2018-11-07 13:53 ` =?unknown-8bit?q?Pi=C4=8Dugins?= Arsenijs
2018-11-07 10:14 ` [PATCH 3/3] sim800: add udev detection Clement Viel
2018-11-07 13:30 ` Jonas Bonn
2018-11-07 13:58 ` Clement Viel
2018-11-07 13:47 ` [PATCH 1/3] sim800: add support for sim800 modem Jonas Bonn
-- strict thread matches above, loose matches on Subject: below --
2018-11-07 14:00 [PATCH 3/3] sim800: add udev detection Clement Viel
2018-11-07 14:02 ` Jonas Bonn
2018-11-07 14:08 ` Clement Viel
2018-11-08 16:59 ` Denis Kenzior
2018-11-12 14:16 ` Clement Viel
2018-11-12 16:34 ` Denis Kenzior
2018-11-12 14:21 ` Clement Viel
2018-10-15 17:27 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
2018-10-15 17:27 ` [PATCH 3/3] sim800: add udev detection Clement Viel
2018-10-03 13:28 [PATCH 1/3] sim800: add support for sim800 modem Clement Viel
2018-10-03 13:28 ` [PATCH 3/3] sim800: add udev detection Clement Viel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox