* [PATCH 1/3] sim800: adding support for Simcom SIM800 modem
@ 2018-09-18 20:36 =?unknown-8bit?q?Cl=C3=A9mentViel?=
2018-09-18 20:36 ` [PATCH 2/3] sim800: add udev detection =?unknown-8bit?q?Cl=C3=A9mentViel?=
` (2 more replies)
0 siblings, 3 replies; 19+ messages in thread
From: =?unknown-8bit?q?Cl=C3=A9mentViel?= @ 2018-09-18 20:36 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 11416 bytes --]
From: clem <vielclement@gmail.com>
---
Makefile.am | 4 +
plugins/sim800.c | 424 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 428 insertions(+)
create mode 100644 plugins/sim800.c
diff --git a/Makefile.am b/Makefile.am
index 6dee4ce..f4d03b6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -496,6 +496,10 @@ builtin_sources += plugins/speedupcdma.c
builtin_modules += samsung
builtin_sources += plugins/samsung.c
+builtin_modules += sim800
+builtin_sources += plugins/sim800.c
+
+
builtin_modules += sim900
builtin_sources += plugins/sim900.c
diff --git a/plugins/sim800.c b/plugins/sim800.c
new file mode 100644
index 0000000..a69fe6d
--- /dev/null
+++ b/plugins/sim800.c
@@ -0,0 +1,424 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2008-2011 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <errno.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <glib.h>
+#include <gatchat.h>
+#include <gattty.h>
+#include <gatmux.h>
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include <ofono/plugin.h>
+#include <ofono/modem.h>
+#include <ofono/devinfo.h>
+#include <ofono/netreg.h>
+#include <ofono/sim.h>
+#include <ofono/sms.h>
+#include <ofono/ussd.h>
+#include <ofono/gprs.h>
+#include <ofono/gprs-context.h>
+#include <ofono/phonebook.h>
+#include <ofono/history.h>
+#include <ofono/log.h>
+#include <ofono/voicecall.h>
+#include <ofono/call-volume.h>
+#include <drivers/atmodem/vendor.h>
+
+#define NUM_DLC 5
+
+#define VOICE_DLC 0
+#define NETREG_DLC 1
+#define SMS_DLC 2
+#define GPRS_DLC 3
+#define SETUP_DLC 4
+
+static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
+ "GPRS: " , "Setup: "};
+
+static const char *none_prefix[] = { NULL };
+
+struct sim800_data {
+ GIOChannel *device;
+ GAtMux *mux;
+ GAtChat * dlcs[NUM_DLC];
+ guint frame_size;
+};
+
+static int sim800_probe(struct ofono_modem *modem)
+{
+ struct sim800_data *data;
+
+ DBG("%p", modem);
+
+ data = g_try_new0(struct sim800_data, 1);
+ if (data == NULL)
+ return -ENOMEM;
+
+ ofono_modem_set_data(modem, data);
+
+ return 0;
+}
+
+static void sim800_remove(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("%p", modem);
+
+ ofono_modem_set_data(modem, NULL);
+
+ g_free(data);
+}
+
+static void sim800_debug(const char *str, void *user_data)
+{
+ const char *prefix = user_data;
+
+ ofono_info("%s%s", prefix, str);
+}
+
+static GAtChat *open_device(struct ofono_modem *modem,
+ const char *key, char *debug)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+ const char *device;
+ GAtSyntax *syntax;
+ GIOChannel *channel;
+ GAtChat *chat;
+ GHashTable *options;
+
+ device = ofono_modem_get_string(modem, key);
+ if (device == NULL) {
+ DBG("couldn't get string %s",key);
+ return NULL;
+ }
+
+ DBG("%s %s", key, device);
+
+ options = g_hash_table_new(g_str_hash, g_str_equal);
+ if (options == NULL) {
+ DBG("options is null");
+ return NULL;
+ }
+
+ g_hash_table_insert(options, "Baud", "115200");
+ g_hash_table_insert(options, "Parity", "none");
+ g_hash_table_insert(options, "StopBits", "1");
+ g_hash_table_insert(options, "DataBits", "8");
+ g_hash_table_insert(options, "XonXoff", "off");
+ g_hash_table_insert(options, "Local", "off");
+ g_hash_table_insert(options, "RtsCts", "off");
+ g_hash_table_insert(options, "Read", "on");
+
+ channel = g_at_tty_open(device, options);
+ g_hash_table_destroy(options);
+
+ if (channel == NULL){
+ DBG("serial channel couldn't be opened");
+ return NULL;
+ }
+
+ data->device = channel;
+ syntax = g_at_syntax_new_gsm_permissive();
+ chat = g_at_chat_new(channel, syntax);
+ g_at_syntax_unref(syntax);
+
+ if (chat == NULL) {
+ g_io_channel_unref(data->device);
+ data->device = NULL;
+ DBG("at channel couldn't be opened");
+
+ return NULL;
+ }
+
+ if (getenv("OFONO_AT_DEBUG"))
+ g_at_chat_set_debug(chat, sim800_debug, debug);
+
+ return chat;
+}
+
+static GAtChat *create_chat(GIOChannel *channel, struct ofono_modem *modem,
+ char *debug)
+{
+ GAtSyntax *syntax;
+ GAtChat *chat;
+
+ if (channel == NULL)
+ return NULL;
+
+ syntax = g_at_syntax_new_gsmv1();
+ chat = g_at_chat_new(channel, syntax);
+ g_at_syntax_unref(syntax);
+ g_io_channel_unref(channel);
+
+ if (chat == NULL)
+ return NULL;
+
+ if (getenv("OFONO_AT_DEBUG")) {
+ DBG("AT DEBUG enabled");
+ g_at_chat_set_debug(chat, sim800_debug, debug);
+ }
+
+ return chat;
+}
+
+static void shutdown_device(struct sim800_data *data)
+{
+ int i;
+
+ DBG("");
+
+ for (i = 0; i < NUM_DLC; i++) {
+ if (data->dlcs[i] == NULL)
+ continue;
+
+ g_at_chat_unref(data->dlcs[i]);
+ data->dlcs[i] = NULL;
+ }
+
+ if (data->mux) {
+ g_at_mux_shutdown(data->mux);
+ g_at_mux_unref(data->mux);
+ data->mux = NULL;
+ }
+
+ g_io_channel_unref(data->device);
+ data->device = NULL;
+}
+
+static void setup_internal_mux(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+ int i;
+
+ DBG("");
+
+ data->frame_size = 128;
+
+ data->mux = g_at_mux_new_gsm0710_basic(data->device,
+ data->frame_size);
+ if (data->mux == NULL)
+ goto error;
+
+ if (getenv("OFONO_MUX_DEBUG"))
+ g_at_mux_set_debug(data->mux, sim800_debug, "MUX: ");
+
+ if (!g_at_mux_start(data->mux)) {
+ g_at_mux_shutdown(data->mux);
+ g_at_mux_unref(data->mux);
+ goto error;
+ }
+
+ for (i = 0; i < NUM_DLC; i++) {
+ GIOChannel *channel = g_at_mux_create_channel(data->mux);
+
+ data->dlcs[i] = create_chat(channel, modem, dlc_prefixes[i]);
+ if (data->dlcs[i] == NULL) {
+ ofono_error("Failed to create channel");
+ goto error;
+ }
+ }
+
+ ofono_modem_set_powered(modem, TRUE);
+ return;
+
+error:
+ shutdown_device(data);
+ ofono_modem_set_powered(modem, FALSE);
+}
+
+static void mux_setup_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("");
+
+ g_at_chat_unref(data->dlcs[SETUP_DLC]);
+ data->dlcs[SETUP_DLC] = NULL;
+
+ if (!ok)
+ goto error;
+
+ setup_internal_mux(modem);
+ return;
+
+error:
+ DBG("If you are on a SIM800H modem with firmware version lower \
+ than ... CMUX command is not supported thus \
+ preventing GPRS, Voice and SMS managers to be executed simultaneously.");
+
+ shutdown_device(data);
+ ofono_modem_set_powered(modem, FALSE);
+}
+
+static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("");
+
+ if (!ok) {
+ g_at_chat_unref(data->dlcs[SETUP_DLC]);
+ data->dlcs[SETUP_DLC] = NULL;
+ ofono_modem_set_powered(modem, FALSE);
+ return;
+ }
+ DBG("sending CMUX");
+ g_at_chat_send(data->dlcs[SETUP_DLC],"AT+CMUX=0,0,5,128,10,3,30,10,2", NULL,mux_setup_cb, modem, NULL);
+
+}
+
+static int sim800_enable(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("%p", modem);
+
+ data->dlcs[SETUP_DLC] = open_device(modem, "Device", "Setup: ");
+ if (data->dlcs[SETUP_DLC] == NULL){
+ DBG("couldn't open device");
+ return -EINVAL;
+ }
+
+ g_at_chat_send(data->dlcs[SETUP_DLC], "ATE0", NULL, NULL, NULL, NULL);
+ g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CFUN=1", none_prefix,
+ cfun_enable, modem, NULL);
+
+ return -EINPROGRESS;
+}
+
+static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("");
+
+ shutdown_device(data);
+
+ if (ok)
+ ofono_modem_set_powered(modem, FALSE);
+}
+
+static int sim800_disable(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("%p", modem);
+
+ g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CFUN=4", none_prefix,
+ cfun_disable, modem, NULL);
+
+ return -EINPROGRESS;
+}
+
+static void sim800_pre_sim(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+ struct ofono_sim *sim;
+
+ DBG("%p %x", modem, data);
+
+ ofono_devinfo_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ DBG("devinfo created");
+ sim = ofono_sim_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ data->dlcs[VOICE_DLC]);
+ DBG("sim created created");
+
+ if (sim)
+ ofono_sim_inserted_notify(sim, TRUE);
+
+ DBG("pre sim finished");
+}
+
+static void sim800_post_sim(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+ struct ofono_gprs *gprs;
+ struct ofono_gprs_context *gc;
+
+ DBG("%p", modem);
+
+ /* Dirty Hack : give some time to sim800 for multiplexing
+ * to be effective and avoid VOICE_DLC to be
+ * flooded thus leading to a "famine" situation
+ */
+
+ sleep(2);
+ 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 sim800_post_online(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("%p", modem);
+
+ ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_netreg_create(modem, OFONO_VENDOR_SIMCOM,
+ "atmodem", data->dlcs[NETREG_DLC]);
+ ofono_ussd_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_voicecall_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_call_volume_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+}
+
+static struct ofono_modem_driver sim800_driver = {
+ .name = "sim800",
+ .probe = sim800_probe,
+ .remove = sim800_remove,
+ .enable = sim800_enable,
+ .disable = sim800_disable,
+ .pre_sim = sim800_pre_sim,
+ .post_sim = sim800_post_sim,
+ .post_online = sim800_post_online,
+};
+
+static int sim800_init(void)
+{
+ return ofono_modem_driver_register(&sim800_driver);
+}
+
+static void sim800_exit(void)
+{
+ ofono_modem_driver_unregister(&sim800_driver);
+}
+
+OFONO_PLUGIN_DEFINE(sim800, "SIM800 modem driver", VERSION,
+ OFONO_PLUGIN_PRIORITY_DEFAULT, sim800_init, sim800_exit)
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH 2/3] sim800: add udev detection
2018-09-18 20:36 [PATCH 1/3] sim800: adding support for Simcom SIM800 modem =?unknown-8bit?q?Cl=C3=A9mentViel?=
@ 2018-09-18 20:36 ` =?unknown-8bit?q?Cl=C3=A9mentViel?=
2018-09-19 16:46 ` Denis Kenzior
2018-09-18 20:36 ` [PATCH 3/3] sim800: adding documentation to inform about the udev rule the user must add =?unknown-8bit?q?Cl=C3=A9mentViel?=
2018-09-19 16:51 ` [PATCH 1/3] sim800: adding support for Simcom SIM800 modem Denis Kenzior
2 siblings, 1 reply; 19+ messages in thread
From: =?unknown-8bit?q?Cl=C3=A9mentViel?= @ 2018-09-18 20:36 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2423 bytes --]
From: clem <vielclement@gmail.com>
---
plugins/udevng.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 49 insertions(+)
diff --git a/plugins/udevng.c b/plugins/udevng.c
index 02d049e..9a1be6a 100644
--- a/plugins/udevng.c
+++ b/plugins/udevng.c
@@ -710,6 +710,53 @@ static gboolean setup_telitqmi(struct modem_info *modem)
return TRUE;
}
+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;
+}
+
/* TODO: Not used as we have no simcom driver */
static gboolean setup_simcom(struct modem_info *modem)
{
@@ -1282,6 +1329,7 @@ static struct {
{ "telit", setup_telit, "device/interface" },
{ "telitqmi", setup_telitqmi },
{ "simcom", setup_simcom },
+ { "sim800", setup_sim800 },
{ "sim7100", setup_sim7100 },
{ "zte", setup_zte },
{ "icera", setup_icera },
@@ -1654,6 +1702,7 @@ static struct {
{ "alcatel", "option", "1bbb", "0017" },
{ "novatel", "option", "1410" },
{ "zte", "option", "19d2" },
+ { "sim800", "option", "05c6", "9000" },
{ "simcom", "option", "05c6", "9000" },
{ "sim7100", "option", "1e0e", "9001" },
{ "telit", "usbserial", "1bc7" },
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 2/3] sim800: add udev detection
2018-09-18 20:36 ` [PATCH 2/3] sim800: add udev detection =?unknown-8bit?q?Cl=C3=A9mentViel?=
@ 2018-09-19 16:46 ` Denis Kenzior
2018-09-19 20:01 ` Clement Viel
0 siblings, 1 reply; 19+ messages in thread
From: Denis Kenzior @ 2018-09-19 16:46 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2806 bytes --]
Hi,
On 09/18/2018 03:36 PM, ClémentViel wrote:
> From: clem <vielclement@gmail.com>
>
> ---
> plugins/udevng.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/plugins/udevng.c b/plugins/udevng.c
> index 02d049e..9a1be6a 100644
> --- a/plugins/udevng.c
> +++ b/plugins/udevng.c
> @@ -710,6 +710,53 @@ static gboolean setup_telitqmi(struct modem_info *modem)
> return TRUE;
> }
>
> +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;
> + }
> + }
> +
So I'm confused now. If sim800 is a serial device, why do you have all
this here? Shouldn't this be handled by setup_serial_modem ?
> + 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;
> +}
> +
> /* TODO: Not used as we have no simcom driver */
> static gboolean setup_simcom(struct modem_info *modem)
> {
> @@ -1282,6 +1329,7 @@ static struct {
> { "telit", setup_telit, "device/interface" },
> { "telitqmi", setup_telitqmi },
> { "simcom", setup_simcom },
> + { "sim800", setup_sim800 },
> { "sim7100", setup_sim7100 },
> { "zte", setup_zte },
> { "icera", setup_icera },
> @@ -1654,6 +1702,7 @@ static struct {
> { "alcatel", "option", "1bbb", "0017" },
> { "novatel", "option", "1410" },
> { "zte", "option", "19d2" },
> + { "sim800", "option", "05c6", "9000" },
> { "simcom", "option", "05c6", "9000" },
> { "sim7100", "option", "1e0e", "9001" },
> { "telit", "usbserial", "1bc7" },
>
Regards,
-Denis
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 2/3] sim800: add udev detection
2018-09-19 16:46 ` Denis Kenzior
@ 2018-09-19 20:01 ` Clement Viel
2018-09-19 20:03 ` Denis Kenzior
0 siblings, 1 reply; 19+ messages in thread
From: Clement Viel @ 2018-09-19 20:01 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 3220 bytes --]
On Wed, Sep 19, 2018 at 11:46:34AM -0500, Denis Kenzior wrote:
Hi,
> Hi,
>
> On 09/18/2018 03:36 PM, ClémentViel wrote:
> >From: clem <vielclement@gmail.com>
> >
> >---
> > plugins/udevng.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 49 insertions(+)
> >
> >diff --git a/plugins/udevng.c b/plugins/udevng.c
> >index 02d049e..9a1be6a 100644
> >--- a/plugins/udevng.c
> >+++ b/plugins/udevng.c
> >@@ -710,6 +710,53 @@ static gboolean setup_telitqmi(struct modem_info *modem)
> > return TRUE;
> > }
> >+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;
> >+ }
> >+ }
> >+
>
> So I'm confused now. If sim800 is a serial device, why do you have all this
> here? Shouldn't this be handled by setup_serial_modem ?
>
Because the modem was seen through a serial/USB converter, my kernel sees it as a USB device. So I used this function.
Should this be coded in a more generic way ?
> >+ 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;
> >+}
> >+
> > /* TODO: Not used as we have no simcom driver */
> > static gboolean setup_simcom(struct modem_info *modem)
> > {
> >@@ -1282,6 +1329,7 @@ static struct {
> > { "telit", setup_telit, "device/interface" },
> > { "telitqmi", setup_telitqmi },
> > { "simcom", setup_simcom },
> >+ { "sim800", setup_sim800 },
> > { "sim7100", setup_sim7100 },
> > { "zte", setup_zte },
> > { "icera", setup_icera },
> >@@ -1654,6 +1702,7 @@ static struct {
> > { "alcatel", "option", "1bbb", "0017" },
> > { "novatel", "option", "1410" },
> > { "zte", "option", "19d2" },
> >+ { "sim800", "option", "05c6", "9000" },
> > { "simcom", "option", "05c6", "9000" },
> > { "sim7100", "option", "1e0e", "9001" },
> > { "telit", "usbserial", "1bc7" },
> >
I must use tabs instead of spaces....I have no excuses...
>
> Regards,
> -Denis
Regards
Clem
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 2/3] sim800: add udev detection
2018-09-19 20:01 ` Clement Viel
@ 2018-09-19 20:03 ` Denis Kenzior
2018-09-19 20:10 ` Clement Viel
0 siblings, 1 reply; 19+ messages in thread
From: Denis Kenzior @ 2018-09-19 20:03 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 481 bytes --]
Hi,
>>
>> So I'm confused now. If sim800 is a serial device, why do you have all this
>> here? Shouldn't this be handled by setup_serial_modem ?
>>
>
> Because the modem was seen through a serial/USB converter, my kernel sees it as a USB device. So I used this function.
> Should this be coded in a more generic way ?
>
Yes, you should use the setup_serial_modem bits and the UDEV rule
outlined in patch 3 should take care of the detection.
Regards,
-Denis
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] sim800: add udev detection
2018-09-19 20:03 ` Denis Kenzior
@ 2018-09-19 20:10 ` Clement Viel
0 siblings, 0 replies; 19+ messages in thread
From: Clement Viel @ 2018-09-19 20:10 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 665 bytes --]
On Wed, Sep 19, 2018 at 03:03:57PM -0500, Denis Kenzior wrote:
Hi,
> Hi,
>
> >>
> >>So I'm confused now. If sim800 is a serial device, why do you have all this
> >>here? Shouldn't this be handled by setup_serial_modem ?
> >>
> >
> >Because the modem was seen through a serial/USB converter, my kernel sees it as a USB device. So I used this function.
> >Should this be coded in a more generic way ?
> >
>
> Yes, you should use the setup_serial_modem bits and the UDEV rule outlined
> in patch 3 should take care of the detection.
>
All right, I'll change the detection and setup in next patch set.
> Regards,
> -Denis
Regards
Clem
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 3/3] sim800: adding documentation to inform about the udev rule the user must add
2018-09-18 20:36 [PATCH 1/3] sim800: adding support for Simcom SIM800 modem =?unknown-8bit?q?Cl=C3=A9mentViel?=
2018-09-18 20:36 ` [PATCH 2/3] sim800: add udev detection =?unknown-8bit?q?Cl=C3=A9mentViel?=
@ 2018-09-18 20:36 ` =?unknown-8bit?q?Cl=C3=A9mentViel?=
2018-09-19 16:43 ` Denis Kenzior
2018-09-20 22:05 ` Pavel Machek
2018-09-19 16:51 ` [PATCH 1/3] sim800: adding support for Simcom SIM800 modem Denis Kenzior
2 siblings, 2 replies; 19+ messages in thread
From: =?unknown-8bit?q?Cl=C3=A9mentViel?= @ 2018-09-18 20:36 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 557 bytes --]
From: clem <vielclement@gmail.com>
---
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..6731879
--- /dev/null
+++ b/doc/sim800-modem.txt
@@ -0,0 +1,7 @@
+SIM800 modem usage
+===================
+
+To enable SIM900 module support you need to put the following
+udev rule into appropriate file in /{etc,lib}/udev/rules.d:
+
+KERNEL=="ttyUSB0", ENV{OFONO_DRIVER}="sim800"
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 3/3] sim800: adding documentation to inform about the udev rule the user must add
2018-09-18 20:36 ` [PATCH 3/3] sim800: adding documentation to inform about the udev rule the user must add =?unknown-8bit?q?Cl=C3=A9mentViel?=
@ 2018-09-19 16:43 ` Denis Kenzior
2018-09-19 19:25 ` Clement Viel
2018-09-20 22:05 ` Pavel Machek
1 sibling, 1 reply; 19+ messages in thread
From: Denis Kenzior @ 2018-09-19 16:43 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 868 bytes --]
Hi,
On 09/18/2018 03:36 PM, ClémentViel wrote:
> From: clem <vielclement@gmail.com>
Can you fix your Author information (see AUTHORS for an example).
Otherwise I cannot take these...
>
> ---
> 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..6731879
> --- /dev/null
> +++ b/doc/sim800-modem.txt
> @@ -0,0 +1,7 @@
> +SIM800 modem usage
> +===================
> +
> +To enable SIM900 module support you need to put the following
> +udev rule into appropriate file in /{etc,lib}/udev/rules.d:
> +
> +KERNEL=="ttyUSB0", ENV{OFONO_DRIVER}="sim800"
>
So sim800 is a serial device right? I take it you're running it from a
serial converter here?
Regards,
-Denis
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 3/3] sim800: adding documentation to inform about the udev rule the user must add
2018-09-19 16:43 ` Denis Kenzior
@ 2018-09-19 19:25 ` Clement Viel
2018-09-19 19:27 ` =?unknown-8bit?q?Pi=C4=8Dugins?= Arsenijs
0 siblings, 1 reply; 19+ messages in thread
From: Clement Viel @ 2018-09-19 19:25 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]
On Wed, Sep 19, 2018 at 11:43:49AM -0500, Denis Kenzior wrote:
> Hi,
>
> On 09/18/2018 03:36 PM, ClémentViel wrote:
> >From: clem <vielclement@gmail.com>
>
> Can you fix your Author information (see AUTHORS for an example). Otherwise
> I cannot take these...
Yep, I'll do it the right way
>
> >
> >---
> > 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..6731879
> >--- /dev/null
> >+++ b/doc/sim800-modem.txt
> >@@ -0,0 +1,7 @@
> >+SIM800 modem usage
> >+===================
> >+
> >+To enable SIM900 module support you need to put the following
> >+udev rule into appropriate file in /{etc,lib}/udev/rules.d:
> >+
> >+KERNEL=="ttyUSB0", ENV{OFONO_DRIVER}="sim800"
> >
>
> So sim800 is a serial device right? I take it you're running it from a
> serial converter here?
Yes, there is a converter. I let "ttyUSB0" as the starter kit provided by SimCom comes with a converter.
>
> Regards,
> -Denis
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 3/3] sim800: adding documentation to inform about the udev rule the user must add
2018-09-18 20:36 ` [PATCH 3/3] sim800: adding documentation to inform about the udev rule the user must add =?unknown-8bit?q?Cl=C3=A9mentViel?=
2018-09-19 16:43 ` Denis Kenzior
@ 2018-09-20 22:05 ` Pavel Machek
1 sibling, 0 replies; 19+ messages in thread
From: Pavel Machek @ 2018-09-20 22:05 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 701 bytes --]
On Tue 2018-09-18 22:36:27, ClémentViel wrote:
> From: clem <vielclement@gmail.com>
>
> ---
> 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..6731879
> --- /dev/null
> +++ b/doc/sim800-modem.txt
> @@ -0,0 +1,7 @@
> +SIM800 modem usage
> +===================
> +
> +To enable SIM900 module support you need to put the following
Is there typo and this should say SIM800?
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] sim800: adding support for Simcom SIM800 modem
2018-09-18 20:36 [PATCH 1/3] sim800: adding support for Simcom SIM800 modem =?unknown-8bit?q?Cl=C3=A9mentViel?=
2018-09-18 20:36 ` [PATCH 2/3] sim800: add udev detection =?unknown-8bit?q?Cl=C3=A9mentViel?=
2018-09-18 20:36 ` [PATCH 3/3] sim800: adding documentation to inform about the udev rule the user must add =?unknown-8bit?q?Cl=C3=A9mentViel?=
@ 2018-09-19 16:51 ` Denis Kenzior
2018-09-19 17:03 ` =?unknown-8bit?q?Pi=C4=8Dugins?= Arsenijs
2018-09-19 19:45 ` Clement Viel
2 siblings, 2 replies; 19+ messages in thread
From: Denis Kenzior @ 2018-09-19 16:51 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]
Hi,
On 09/18/2018 03:36 PM, ClémentViel wrote:
> From: clem <vielclement@gmail.com>
>
You might want to describe how this is different from sim900 that it
warrants a fully separate driver?
If there are only minor differences, then this can be handled via UDEV
attributes or querying +CGMM, etc.
> ---
> Makefile.am | 4 +
> plugins/sim800.c | 424 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 428 insertions(+)
> create mode 100644 plugins/sim800.c
>
<snip>
> +
> +static void sim800_post_sim(struct ofono_modem *modem)
> +{
> + struct sim800_data *data = ofono_modem_get_data(modem);
> + struct ofono_gprs *gprs;
> + struct ofono_gprs_context *gc;
> +
> + DBG("%p", modem);
> +
> + /* Dirty Hack : give some time to sim800 for multiplexing
> + * to be effective and avoid VOICE_DLC to be
> + * flooded thus leading to a "famine" situation
> + */
> +
> + sleep(2);
No sleeps inside plugins. That blocks the entire daemon and we can't
have that. How does this help you anyway? GAtChat is a queue, so only
1 command is outstanding at a time.
> + 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);
> +}
> +
Regards,
-Denis
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/3] sim800: adding support for Simcom SIM800 modem
2018-09-19 16:51 ` [PATCH 1/3] sim800: adding support for Simcom SIM800 modem Denis Kenzior
@ 2018-09-19 17:03 ` =?unknown-8bit?q?Pi=C4=8Dugins?= Arsenijs
2018-09-19 19:52 ` Clement Viel
2018-09-19 19:45 ` Clement Viel
1 sibling, 1 reply; 19+ messages in thread
From: =?unknown-8bit?q?Pi=C4=8Dugins?= Arsenijs @ 2018-09-19 17:03 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2: attachment.html --]
[-- Type: text/html, Size: 3530 bytes --]
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] sim800: adding support for Simcom SIM800 modem
2018-09-19 17:03 ` =?unknown-8bit?q?Pi=C4=8Dugins?= Arsenijs
@ 2018-09-19 19:52 ` Clement Viel
0 siblings, 0 replies; 19+ messages in thread
From: Clement Viel @ 2018-09-19 19:52 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 3932 bytes --]
On Wed, Sep 19, 2018 at 08:03:27PM +0300, Pičugins Arsenijs wrote:
Hi,
I guess I submit it in a hurry and forgot to add the firmware version.
BTW, your problem may be related to this. Are you sure your CMUX command is not returning an error ? Thus preventing ofono from enabling the modem ? I had this problem and Simcom sent me an updated firmware that supports the CMUX command.
> <div>Hello!</div><div> </div><div>> You might want to describe how this is different from sim900 that it<br />> warrants a fully separate driver?</div><div> </div><div>I've tried to apply this patch yesterday evening (since I too am</div><div>interested in SIM800 integration) and tried to apply it, patch 2</div><div>doesn't apply on current master for some reason. Here's a diff</div><div>between current SIM900 and this SIM800:</div><div><a href="https://matrix.org/_matrix/media/v1/download/matrix.org/KqDxbcuYLxrcceUfRorxYgus" data-external="true">https://matrix.org/_matrix/media/v1/download/matrix.org/KqDxbcuYLxrcceUfRorxYgus</a></div><div> </div><div>tl;dr: apart from</div><div> </div><div>- adding phonebook support (two lines, I don't understand it quite yet)</div><div>- the sleep() hack</div><div>- a comment clarifying a problem with some unknown version of firmware</div><div>- DBG() log calls</div><div>- sim900 => sim800 renaming</div><div> </div><div>it's no different than current SIM900 driver. Unfortunately, that</div><div>doesn't solve the problems I'm personally experiencing (hang on</div><div>CFUN, for one).</div><div> </div><div>Cheers!<br />Arsenijs</div><div> </div><div>19.09.2018, 19:51, "Denis Kenzior" <denkenz(a)gmail.com>:</div><blockquote type="cite"><p>Hi,<br /><br />On 09/18/2018 03:36 PM, ClémentViel wrote:</p><blockquote> From: clem <<a data-external="true" href="mailto:vielclement@gmail.com">vielclement(a)gmail.com</a>><br /> </blockquote><p><br />You might want to describe how this is different from sim900 that it<br />warrants a fully separate driver?<br /><br />If there are only minor differences, then this can be handled via UDEV<br />attributes or querying +CGMM, etc.<br /> </p><blockquote> ---<br /> Makefile.am | 4 +<br /> plugins/sim800.c | 424 +++++++++++++++++++++++++++++++++++++++++++++++++++++++<br /> 2 files changed, 428 insertions(+)<br /> create mode 100644 plugins/sim800.c<br /> </blockquote><p><br /><snip><br /> </p><blockquote> +<br /> +static void sim800_post_sim(struct ofono_modem *modem)<br /> +{<br /> + struct sim800_data *data = ofono_modem_get_data(modem);<br /> + struct ofono_gprs *gprs;<br /> + struct ofono_gprs_context *gc;<br /> +<br /> + DBG("%p", modem);<br /> +<br /> + /* Dirty Hack : give some time to sim800 for multiplexing<br /> + * to be effective and avoid VOICE_DLC to be<br /> + * flooded thus leading to a "famine" situation<br /> + */<br /> +<br /> + sleep(2);</blockquote><p><br />No sleeps inside plugins. That blocks the entire daemon and we can't<br />have that. How does this help you anyway? GAtChat is a queue, so only<br />1 command is outstanding at a time.<br /> </p><blockquote> + ofono_sms_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",<br /> + data->dlcs[SMS_DLC]);<br /> +<br /> +<br /> + gprs = ofono_gprs_create(modem, 0, "atmodem", data->dlcs[GPRS_DLC]);<br /> + if (gprs == NULL)<br /> + return;<br /> +<br /> + gc = ofono_gprs_context_create(modem, OFONO_VENDOR_SIMCOM,<br /> + "atmodem", data->dlcs[GPRS_DLC]);<br /> + if (gc)<br /> + ofono_gprs_add_context(gprs, gc);<br /> +}<br /> +</blockquote><p><br />Regards,<br />-Denis<br />_______________________________________________<br />ofono mailing list<br /><a data-external="true" href="mailto:ofono@ofono.org">ofono(a)ofono.org</a><br /><a href="https://lists.ofono.org/mailman/listinfo/ofono">https://lists.ofono.org/mailman/listinfo/ofono</a></p></blockquote>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 1/3] sim800: adding support for Simcom SIM800 modem
2018-09-19 16:51 ` [PATCH 1/3] sim800: adding support for Simcom SIM800 modem Denis Kenzior
2018-09-19 17:03 ` =?unknown-8bit?q?Pi=C4=8Dugins?= Arsenijs
@ 2018-09-19 19:45 ` Clement Viel
1 sibling, 0 replies; 19+ messages in thread
From: Clement Viel @ 2018-09-19 19:45 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 2171 bytes --]
On Wed, Sep 19, 2018 at 11:51:26AM -0500, Denis Kenzior wrote:
Hi,
> Hi,
>
> On 09/18/2018 03:36 PM, ClémentViel wrote:
> >From: clem <vielclement@gmail.com>
> >
>
> You might want to describe how this is different from sim900 that it
> warrants a fully separate driver?
>
> If there are only minor differences, then this can be handled via UDEV
> attributes or querying +CGMM, etc.
>
> >---
> > Makefile.am | 4 +
> > plugins/sim800.c | 424 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 428 insertions(+)
> > create mode 100644 plugins/sim800.c
> >
I guess I went straightforward and because sim800 wasn't supported I wanted to add it as a plugin.
Sim800 is really close from sim900, in fact theey are compatible. But because sim800 series are newer.
sim800 have more AT commands than sim900 so maybe the compatibility won't be complete if the two drivers are merged...
>
> <snip>
>
> >+
> >+static void sim800_post_sim(struct ofono_modem *modem)
> >+{
> >+ struct sim800_data *data = ofono_modem_get_data(modem);
> >+ struct ofono_gprs *gprs;
> >+ struct ofono_gprs_context *gc;
> >+
> >+ DBG("%p", modem);
> >+
> >+ /* Dirty Hack : give some time to sim800 for multiplexing
> >+ * to be effective and avoid VOICE_DLC to be
> >+ * flooded thus leading to a "famine" situation
> >+ */
> >+
> >+ sleep(2);
>
> No sleeps inside plugins. That blocks the entire daemon and we can't have
> that. How does this help you anyway? GAtChat is a queue, so only 1 command
> is outstanding at a time.
Yep that's why the "Dirty Hack" comment, I'll try another trick, there is an URC that we can trigger on.
>
> >+ 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);
> >+}
> >+
>
> Regards,
> -Denis
Regards
Clement
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/3] sim800: adding support for SimCom SIM800 modem
@ 2018-09-24 22:43 Clement Viel
2018-09-25 16:48 ` Denis Kenzior
0 siblings, 1 reply; 19+ messages in thread
From: Clement Viel @ 2018-09-24 22:43 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 12560 bytes --]
From: clem <vielclement@gmail.com>
---
Makefile.am | 4 +
plugins/sim800.c | 463 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 467 insertions(+)
create mode 100644 plugins/sim800.c
diff --git a/Makefile.am b/Makefile.am
index 6dee4ce..f4d03b6 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -496,6 +496,10 @@ builtin_sources += plugins/speedupcdma.c
builtin_modules += samsung
builtin_sources += plugins/samsung.c
+builtin_modules += sim800
+builtin_sources += plugins/sim800.c
+
+
builtin_modules += sim900
builtin_sources += plugins/sim900.c
diff --git a/plugins/sim800.c b/plugins/sim800.c
new file mode 100644
index 0000000..c9285c1
--- /dev/null
+++ b/plugins/sim800.c
@@ -0,0 +1,463 @@
+/*
+ *
+ * oFono - Open Source Telephony
+ *
+ * Copyright (C) 2008-2011 Intel Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <errno.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <glib.h>
+#include <gatchat.h>
+#include <gattty.h>
+#include <gatmux.h>
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include <ofono/plugin.h>
+#include <ofono/modem.h>
+#include <ofono/devinfo.h>
+#include <ofono/netreg.h>
+#include <ofono/sim.h>
+#include <ofono/sms.h>
+#include <ofono/ussd.h>
+#include <ofono/gprs.h>
+#include <ofono/gprs-context.h>
+#include <ofono/phonebook.h>
+#include <ofono/history.h>
+#include <ofono/log.h>
+#include <ofono/voicecall.h>
+#include <ofono/call-volume.h>
+#include <drivers/atmodem/vendor.h>
+
+#define NUM_DLC 5
+
+#define VOICE_DLC 0
+#define NETREG_DLC 1
+#define SMS_DLC 2
+#define GPRS_DLC 3
+#define SETUP_DLC 4
+
+static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
+ "GPRS: " , "Setup: "};
+
+static const char *none_prefix[] = { NULL };
+
+struct sim800_data {
+ GIOChannel *device;
+ GAtMux *mux;
+ GAtChat * dlcs[NUM_DLC];
+ guint frame_size;
+ int mux_not_supported;
+};
+
+
+static void mux_ready_notify(GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim800_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);
+ }
+ notified = 1;
+
+}
+
+
+static int sim800_probe(struct ofono_modem *modem)
+{
+ struct sim800_data *data;
+
+ DBG("%p", modem);
+
+ data = g_try_new0(struct sim800_data, 1);
+ if (data == NULL)
+ return -ENOMEM;
+
+ ofono_modem_set_data(modem, data);
+
+ return 0;
+}
+
+static void sim800_remove(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("%p", modem);
+
+ ofono_modem_set_data(modem, NULL);
+
+ g_free(data);
+}
+
+static void sim800_debug(const char *str, void *user_data)
+{
+ const char *prefix = user_data;
+
+ ofono_info("%s%s", prefix, str);
+}
+
+static GAtChat *open_device(struct ofono_modem *modem,
+ const char *key, char *debug)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+ const char *device;
+ GAtSyntax *syntax;
+ GIOChannel *channel;
+ GAtChat *chat;
+ GHashTable *options;
+
+ device = ofono_modem_get_string(modem, key);
+ if (device == NULL) {
+ DBG("couldn't get string %s",key);
+ return NULL;
+ }
+
+ DBG("%s %s", key, device);
+
+ options = g_hash_table_new(g_str_hash, g_str_equal);
+ if (options == NULL) {
+ DBG("options is null");
+ return NULL;
+ }
+
+ g_hash_table_insert(options, "Baud", "115200");
+ g_hash_table_insert(options, "Parity", "none");
+ g_hash_table_insert(options, "StopBits", "1");
+ g_hash_table_insert(options, "DataBits", "8");
+ g_hash_table_insert(options, "XonXoff", "off");
+ g_hash_table_insert(options, "Local", "off");
+ g_hash_table_insert(options, "RtsCts", "off");
+ g_hash_table_insert(options, "Read", "on");
+
+ channel = g_at_tty_open(device, options);
+ g_hash_table_destroy(options);
+
+ if (channel == NULL){
+ DBG("serial channel couldn't be opened");
+ return NULL;
+ }
+
+ data->device = channel;
+ syntax = g_at_syntax_new_gsm_permissive();
+ chat = g_at_chat_new(channel, syntax);
+ g_at_syntax_unref(syntax);
+
+ if (chat == NULL) {
+ g_io_channel_unref(data->device);
+ data->device = NULL;
+ DBG("at channel couldn't be opened");
+
+ return NULL;
+ }
+
+ if (getenv("OFONO_AT_DEBUG"))
+ g_at_chat_set_debug(chat, sim800_debug, debug);
+
+ return chat;
+}
+
+static GAtChat *create_chat(GIOChannel *channel, struct ofono_modem *modem,
+ char *debug)
+{
+ GAtSyntax *syntax;
+ GAtChat *chat;
+
+ if (channel == NULL)
+ return NULL;
+
+ syntax = g_at_syntax_new_gsmv1();
+ chat = g_at_chat_new(channel, syntax);
+ g_at_syntax_unref(syntax);
+ g_io_channel_unref(channel);
+
+ if (chat == NULL)
+ return NULL;
+
+ if (getenv("OFONO_AT_DEBUG")) {
+ DBG("AT DEBUG enabled");
+ g_at_chat_set_debug(chat, sim800_debug, debug);
+ }
+
+ return chat;
+}
+
+static void shutdown_device(struct sim800_data *data)
+{
+ int i;
+
+ DBG("");
+
+ for (i = 0; i < NUM_DLC; i++) {
+ if (data->dlcs[i] == NULL)
+ continue;
+
+ g_at_chat_unref(data->dlcs[i]);
+ data->dlcs[i] = NULL;
+ }
+
+ if (data->mux) {
+ g_at_mux_shutdown(data->mux);
+ g_at_mux_unref(data->mux);
+ data->mux = NULL;
+ }
+
+ g_io_channel_unref(data->device);
+ data->device = NULL;
+}
+
+static void setup_internal_mux(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+ int i;
+
+ DBG("");
+
+ data->frame_size = 128;
+
+ data->mux = g_at_mux_new_gsm0710_basic(data->device,
+ data->frame_size);
+ if (data->mux == NULL)
+ goto error;
+
+ if (getenv("OFONO_MUX_DEBUG"))
+ g_at_mux_set_debug(data->mux, sim800_debug, "MUX: ");
+
+ if (!g_at_mux_start(data->mux)) {
+ g_at_mux_shutdown(data->mux);
+ g_at_mux_unref(data->mux);
+ goto error;
+ }
+
+ for (i = 0; i < NUM_DLC; i++) {
+ GIOChannel *channel = g_at_mux_create_channel(data->mux);
+
+ data->dlcs[i] = create_chat(channel, modem, dlc_prefixes[i]);
+ if (data->dlcs[i] == NULL) {
+ ofono_error("Failed to create channel");
+ goto error;
+ }
+ }
+ 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);
+ return;
+
+error:
+ shutdown_device(data);
+ ofono_modem_set_powered(modem, FALSE);
+}
+
+static void mux_setup_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("");
+
+ g_at_chat_unref(data->dlcs[SETUP_DLC]);
+ data->dlcs[SETUP_DLC] = NULL;
+
+ if (!ok)
+ goto error;
+
+ data->mux_not_supported = 0;
+ setup_internal_mux(modem);
+ return;
+
+error:
+ DBG("If you are on a SIM800H modem with firmware version is \
+ 1308B02SIM800H32_BT or lower, CMUX command is not supported thus \
+ preventing GPRS, Voice and SMS managers \
+ to be executed simultaneously.");
+ /*
+ * If your use of SIM800 does not need simultaneous use of Voice, SMS
+ * and GPRS, you can ignore the error, set mux_not_supported=1 and comment
+ * setup_internal_mux function, refactor this code to use only one DLC.
+ * Else, you must contact SIMCOM to get a firmware update.
+ */
+ shutdown_device(data);
+ ofono_modem_set_powered(modem, FALSE);
+}
+
+static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("");
+
+ if (!ok) {
+ g_at_chat_unref(data->dlcs[SETUP_DLC]);
+ data->dlcs[SETUP_DLC] = NULL;
+ ofono_modem_set_powered(modem, FALSE);
+ return;
+ }
+
+ g_at_chat_send(data->dlcs[SETUP_DLC],"AT+CMUX=0,0,5,128,10,3,30,10,2", NULL,mux_setup_cb, modem, NULL);
+
+}
+
+static int sim800_enable(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("%p", modem);
+
+ data->dlcs[SETUP_DLC] = open_device(modem, "Device", "Setup: ");
+ if (data->dlcs[SETUP_DLC] == NULL){
+ DBG("couldn't open device");
+ return -EINVAL;
+ }
+
+ g_at_chat_send(data->dlcs[SETUP_DLC], "ATE0", NULL, NULL, NULL, NULL);
+ g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CFUN=1", none_prefix,
+ cfun_enable, modem, NULL);
+
+ return -EINPROGRESS;
+}
+
+static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("");
+
+ shutdown_device(data);
+
+ if (ok)
+ ofono_modem_set_powered(modem, FALSE);
+}
+
+static int sim800_disable(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("%p", modem);
+
+ g_at_chat_send(data->dlcs[SETUP_DLC], "AT+CFUN=4", none_prefix,
+ cfun_disable, modem, NULL);
+
+ return -EINPROGRESS;
+}
+
+static void sim800_pre_sim(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+ struct ofono_sim *sim;
+
+ DBG("%p %x", modem, data);
+
+ ofono_devinfo_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ DBG("devinfo created");
+ sim = ofono_sim_create(modem, OFONO_VENDOR_SIMCOM, "atmodem",
+ data->dlcs[VOICE_DLC]);
+ DBG("sim created created");
+
+ if (sim)
+ ofono_sim_inserted_notify(sim, TRUE);
+
+ DBG("pre sim finished");
+}
+
+static void sim800_post_sim(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+ struct ofono_gprs *gprs = NULL;
+ struct ofono_gprs_context *gc;
+
+ DBG("%p", modem);
+
+ /* If CMUX command is not supported, we can go through
+ * the normal way but with only one DLC.*/
+
+ if (data->mux_not_supported){
+
+ 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 sim800_post_online(struct ofono_modem *modem)
+{
+ struct sim800_data *data = ofono_modem_get_data(modem);
+
+ DBG("%p", modem);
+
+ ofono_phonebook_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_netreg_create(modem, OFONO_VENDOR_SIMCOM,
+ "atmodem", data->dlcs[NETREG_DLC]);
+ ofono_ussd_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_voicecall_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+ ofono_call_volume_create(modem, 0, "atmodem", data->dlcs[VOICE_DLC]);
+}
+
+static struct ofono_modem_driver sim800_driver = {
+ .name = "sim800",
+ .probe = sim800_probe,
+ .remove = sim800_remove,
+ .enable = sim800_enable,
+ .disable = sim800_disable,
+ .pre_sim = sim800_pre_sim,
+ .post_sim = sim800_post_sim,
+ .post_online = sim800_post_online,
+};
+
+static int sim800_init(void)
+{
+ return ofono_modem_driver_register(&sim800_driver);
+}
+
+static void sim800_exit(void)
+{
+ ofono_modem_driver_unregister(&sim800_driver);
+}
+
+OFONO_PLUGIN_DEFINE(sim800, "SIM800 modem driver", VERSION,
+ OFONO_PLUGIN_PRIORITY_DEFAULT, sim800_init, sim800_exit)
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH 1/3] sim800: adding support for SimCom SIM800 modem
2018-09-24 22:43 [PATCH 1/3] sim800: adding support for SimCom " Clement Viel
@ 2018-09-25 16:48 ` Denis Kenzior
2018-09-25 18:08 ` Clement Viel
0 siblings, 1 reply; 19+ messages in thread
From: Denis Kenzior @ 2018-09-25 16:48 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 5953 bytes --]
Hi Clement,
On 09/24/2018 05:43 PM, Clement Viel wrote:
> From: clem <vielclement@gmail.com>
Your author information is still broken for the actual commits. Please
fix that.
>
> ---
> Makefile.am | 4 +
> plugins/sim800.c | 463 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 467 insertions(+)
> create mode 100644 plugins/sim800.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 6dee4ce..f4d03b6 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -496,6 +496,10 @@ builtin_sources += plugins/speedupcdma.c
> builtin_modules += samsung
> builtin_sources += plugins/samsung.c
>
> +builtin_modules += sim800
> +builtin_sources += plugins/sim800.c
> +
> +
One empty line please
> builtin_modules += sim900
> builtin_sources += plugins/sim900.c
>
> diff --git a/plugins/sim800.c b/plugins/sim800.c
> new file mode 100644
> index 0000000..c9285c1
> --- /dev/null
> +++ b/plugins/sim800.c
> @@ -0,0 +1,463 @@
> +/*
> + *
> + * oFono - Open Source Telephony
> + *
> + * Copyright (C) 2008-2011 Intel Corporation. All rights reserved.
Not updating the copyright?
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <errno.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <glib.h>
> +#include <gatchat.h>
> +#include <gattty.h>
> +#include <gatmux.h>
> +
> +#define OFONO_API_SUBJECT_TO_CHANGE
> +#include <ofono/plugin.h>
> +#include <ofono/modem.h>
> +#include <ofono/devinfo.h>
> +#include <ofono/netreg.h>
> +#include <ofono/sim.h>
> +#include <ofono/sms.h>
> +#include <ofono/ussd.h>
> +#include <ofono/gprs.h>
> +#include <ofono/gprs-context.h>
> +#include <ofono/phonebook.h>
> +#include <ofono/history.h>
> +#include <ofono/log.h>
> +#include <ofono/voicecall.h>
> +#include <ofono/call-volume.h>
> +#include <drivers/atmodem/vendor.h>
> +
> +#define NUM_DLC 5
> +
> +#define VOICE_DLC 0
> +#define NETREG_DLC 1
> +#define SMS_DLC 2
> +#define GPRS_DLC 3
> +#define SETUP_DLC 4
> +
> +static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
> + "GPRS: " , "Setup: "};
> +
> +static const char *none_prefix[] = { NULL };
> +
> +struct sim800_data {
> + GIOChannel *device;
> + GAtMux *mux;
> + GAtChat * dlcs[NUM_DLC];
> + guint frame_size;
> + int mux_not_supported;
bool ?
> +};
> +
> +
No double empty lines please
> +static void mux_ready_notify(GAtResult *result, gpointer user_data)
> +{
> + struct ofono_modem *modem = user_data;
> + struct sim800_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]);
> +
> +
Wrong indentation & coding style...
> + 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);
> + }
> + notified = 1;
> +
> +}
> +
> +
<snip>
> +static void mux_setup_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> + struct ofono_modem *modem = user_data;
> + struct sim800_data *data = ofono_modem_get_data(modem);
> +
> + DBG("");
> +
> + g_at_chat_unref(data->dlcs[SETUP_DLC]);
> + data->dlcs[SETUP_DLC] = NULL;
> +
> + if (!ok)
> + goto error;
> +
> + data->mux_not_supported = 0;
What does this variable do?
> + setup_internal_mux(modem);
> + return;
> +
> +error:
> + DBG("If you are on a SIM800H modem with firmware version is \
> + 1308B02SIM800H32_BT or lower, CMUX command is not supported thus \
> + preventing GPRS, Voice and SMS managers \
> + to be executed simultaneously.");
> + /*
> + * If your use of SIM800 does not need simultaneous use of Voice, SMS
> + * and GPRS, you can ignore the error, set mux_not_supported=1 and comment
> + * setup_internal_mux function, refactor this code to use only one DLC.
> + * Else, you must contact SIMCOM to get a firmware update.
I would drop all these hacks and document the required firmware version
in doc/sim800-modem.txt.
> + */
> + shutdown_device(data);
> + ofono_modem_set_powered(modem, FALSE);
> +}
> +
> +static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> + struct ofono_modem *modem = user_data;
> + struct sim800_data *data = ofono_modem_get_data(modem);
> +
> + DBG("");
> +
> + if (!ok) {
> + g_at_chat_unref(data->dlcs[SETUP_DLC]);
> + data->dlcs[SETUP_DLC] = NULL;
> + ofono_modem_set_powered(modem, FALSE);
> + return;
> + }
> +
> + g_at_chat_send(data->dlcs[SETUP_DLC],"AT+CMUX=0,0,5,128,10,3,30,10,2", NULL,mux_setup_cb, modem, NULL);
Not our coding style.
> +
> +}
> +
<snip>
I ran a quick diff between this and sim900 driver and there's really not
enough differences to warrant a completely separate plugin. Can't we
just simply query the model version and act accordingly?
Regards,
-Denis
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/3] sim800: adding support for SimCom SIM800 modem
2018-09-25 16:48 ` Denis Kenzior
@ 2018-09-25 18:08 ` Clement Viel
2018-09-25 18:41 ` Denis Kenzior
0 siblings, 1 reply; 19+ messages in thread
From: Clement Viel @ 2018-09-25 18:08 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 6849 bytes --]
Hi Denis,
On Tue, Sep 25, 2018 at 11:48:39AM -0500, Denis Kenzior wrote:
> Hi Clement,
>
> On 09/24/2018 05:43 PM, Clement Viel wrote:
> >From: clem <vielclement@gmail.com>
>
> Your author information is still broken for the actual commits. Please fix
> that.
>
> >
> >---
> > Makefile.am | 4 +
> > plugins/sim800.c | 463 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 467 insertions(+)
> > create mode 100644 plugins/sim800.c
> >
> >diff --git a/Makefile.am b/Makefile.am
> >index 6dee4ce..f4d03b6 100644
> >--- a/Makefile.am
> >+++ b/Makefile.am
> >@@ -496,6 +496,10 @@ builtin_sources += plugins/speedupcdma.c
> > builtin_modules += samsung
> > builtin_sources += plugins/samsung.c
> >+builtin_modules += sim800
> >+builtin_sources += plugins/sim800.c
> >+
> >+
>
> One empty line please
>
> > builtin_modules += sim900
> > builtin_sources += plugins/sim900.c
> >diff --git a/plugins/sim800.c b/plugins/sim800.c
> >new file mode 100644
> >index 0000000..c9285c1
> >--- /dev/null
> >+++ b/plugins/sim800.c
> >@@ -0,0 +1,463 @@
> >+/*
> >+ *
> >+ * oFono - Open Source Telephony
> >+ *
> >+ * Copyright (C) 2008-2011 Intel Corporation. All rights reserved.
>
> Not updating the copyright?
>
> >+ *
> >+ * This program is free software; you can redistribute it and/or modify
> >+ * it under the terms of the GNU General Public License version 2 as
> >+ * published by the Free Software Foundation.
> >+ *
> >+ * This program is distributed in the hope that it will be useful,
> >+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >+ * GNU General Public License for more details.
> >+ *
> >+ * You should have received a copy of the GNU General Public License
> >+ * along with this program; if not, write to the Free Software
> >+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> >+ *
> >+ */
> >+
> >+#ifdef HAVE_CONFIG_H
> >+#include <config.h>
> >+#endif
> >+
> >+#include <errno.h>
> >+#include <stdlib.h>
> >+#include <unistd.h>
> >+#include <glib.h>
> >+#include <gatchat.h>
> >+#include <gattty.h>
> >+#include <gatmux.h>
> >+
> >+#define OFONO_API_SUBJECT_TO_CHANGE
> >+#include <ofono/plugin.h>
> >+#include <ofono/modem.h>
> >+#include <ofono/devinfo.h>
> >+#include <ofono/netreg.h>
> >+#include <ofono/sim.h>
> >+#include <ofono/sms.h>
> >+#include <ofono/ussd.h>
> >+#include <ofono/gprs.h>
> >+#include <ofono/gprs-context.h>
> >+#include <ofono/phonebook.h>
> >+#include <ofono/history.h>
> >+#include <ofono/log.h>
> >+#include <ofono/voicecall.h>
> >+#include <ofono/call-volume.h>
> >+#include <drivers/atmodem/vendor.h>
> >+
> >+#define NUM_DLC 5
> >+
> >+#define VOICE_DLC 0
> >+#define NETREG_DLC 1
> >+#define SMS_DLC 2
> >+#define GPRS_DLC 3
> >+#define SETUP_DLC 4
> >+
> >+static char *dlc_prefixes[NUM_DLC] = { "Voice: ", "Net: ", "SMS: ",
> >+ "GPRS: " , "Setup: "};
> >+
> >+static const char *none_prefix[] = { NULL };
> >+
> >+struct sim800_data {
> >+ GIOChannel *device;
> >+ GAtMux *mux;
> >+ GAtChat * dlcs[NUM_DLC];
> >+ guint frame_size;
> >+ int mux_not_supported;
>
> bool ?
>
> >+};
> >+
> >+
>
> No double empty lines please
>
> >+static void mux_ready_notify(GAtResult *result, gpointer user_data)
> >+{
> >+ struct ofono_modem *modem = user_data;
> >+ struct sim800_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]);
> >+
> >+
>
> Wrong indentation & coding style...
>
> >+ 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);
> >+ }
> >+ notified = 1;
> >+
> >+}
> >+
> >+
>
> <snip>
>
> >+static void mux_setup_cb(gboolean ok, GAtResult *result, gpointer user_data)
> >+{
> >+ struct ofono_modem *modem = user_data;
> >+ struct sim800_data *data = ofono_modem_get_data(modem);
> >+
> >+ DBG("");
> >+
> >+ g_at_chat_unref(data->dlcs[SETUP_DLC]);
> >+ data->dlcs[SETUP_DLC] = NULL;
> >+
> >+ if (!ok)
> >+ goto error;
> >+
> >+ data->mux_not_supported = 0;
>
> What does this variable do?
>
> >+ setup_internal_mux(modem);
> >+ return;
> >+
> >+error:
> >+ DBG("If you are on a SIM800H modem with firmware version is \
> >+ 1308B02SIM800H32_BT or lower, CMUX command is not supported thus \
> >+ preventing GPRS, Voice and SMS managers \
> >+ to be executed simultaneously.");
> >+ /*
> >+ * If your use of SIM800 does not need simultaneous use of Voice, SMS
> >+ * and GPRS, you can ignore the error, set mux_not_supported=1 and comment
> >+ * setup_internal_mux function, refactor this code to use only one DLC.
> >+ * Else, you must contact SIMCOM to get a firmware update.
>
> I would drop all these hacks and document the required firmware version in
> doc/sim800-modem.txt.
>
> >+ */
> >+ shutdown_device(data);
> >+ ofono_modem_set_powered(modem, FALSE);
> >+}
> >+
> >+static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
> >+{
> >+ struct ofono_modem *modem = user_data;
> >+ struct sim800_data *data = ofono_modem_get_data(modem);
> >+
> >+ DBG("");
> >+
> >+ if (!ok) {
> >+ g_at_chat_unref(data->dlcs[SETUP_DLC]);
> >+ data->dlcs[SETUP_DLC] = NULL;
> >+ ofono_modem_set_powered(modem, FALSE);
> >+ return;
> >+ }
> >+
> >+ g_at_chat_send(data->dlcs[SETUP_DLC],"AT+CMUX=0,0,5,128,10,3,30,10,2", NULL,mux_setup_cb, modem, NULL);
>
> Not our coding style.
>
> >+
> >+}
> >+
>
>
> <snip>
>
> I ran a quick diff between this and sim900 driver and there's really not
> enough differences to warrant a completely separate plugin. Can't we just
> simply query the model version and act accordingly?
My opinion is that the sim800 driver is much to be needed as Simcom considers the sim800 as the new sim900.
I "fear" that merging the two drivers will end up in a big file having a lot of "if...else" for every different feature
between those two drivers. Whereas having the two separate for some time will allow to address the use of sim800's new features
and support legacy sim900.
But as you are the maintainer, I think the decision is yours. Tell me and i'll modify my patches accordingly then preventing too much noisy commits :)
>
> Regards,
> -Denis
Regards
Clement
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH 1/3] sim800: adding support for SimCom SIM800 modem
2018-09-25 18:08 ` Clement Viel
@ 2018-09-25 18:41 ` Denis Kenzior
0 siblings, 0 replies; 19+ messages in thread
From: Denis Kenzior @ 2018-09-25 18:41 UTC (permalink / raw)
To: ofono
[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]
Hi Clement,
>>
>> I ran a quick diff between this and sim900 driver and there's really not
>> enough differences to warrant a completely separate plugin. Can't we just
>> simply query the model version and act accordingly?
>
> My opinion is that the sim800 driver is much to be needed as Simcom considers the sim800 as the new sim900.
> I "fear" that merging the two drivers will end up in a big file having a lot of "if...else" for every different feature
> between those two drivers. Whereas having the two separate for some time will allow to address the use of sim800's new features
> and support legacy sim900.
It might not be as bad as you think. Besides, if we determine at some
future date, that sim800 and sim900 are different enough to warrant
different drivers, then we have the option of splitting them out into
separate drivers at that time.
>
> But as you are the maintainer, I think the decision is yours. Tell me and i'll modify my patches accordingly then preventing too much noisy commits :)
>
My feeling is that we should merge these. The only difference I see
right now is a VENDOR flag tweak and an SMS Ready notification handling.
These don't amount to much and could be easily handled by the existing
driver.
Regards,
-Denis
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2018-09-25 18:41 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-18 20:36 [PATCH 1/3] sim800: adding support for Simcom SIM800 modem =?unknown-8bit?q?Cl=C3=A9mentViel?=
2018-09-18 20:36 ` [PATCH 2/3] sim800: add udev detection =?unknown-8bit?q?Cl=C3=A9mentViel?=
2018-09-19 16:46 ` Denis Kenzior
2018-09-19 20:01 ` Clement Viel
2018-09-19 20:03 ` Denis Kenzior
2018-09-19 20:10 ` Clement Viel
2018-09-18 20:36 ` [PATCH 3/3] sim800: adding documentation to inform about the udev rule the user must add =?unknown-8bit?q?Cl=C3=A9mentViel?=
2018-09-19 16:43 ` Denis Kenzior
2018-09-19 19:25 ` Clement Viel
2018-09-19 19:27 ` =?unknown-8bit?q?Pi=C4=8Dugins?= Arsenijs
2018-09-20 22:05 ` Pavel Machek
2018-09-19 16:51 ` [PATCH 1/3] sim800: adding support for Simcom SIM800 modem Denis Kenzior
2018-09-19 17:03 ` =?unknown-8bit?q?Pi=C4=8Dugins?= Arsenijs
2018-09-19 19:52 ` Clement Viel
2018-09-19 19:45 ` Clement Viel
-- strict thread matches above, loose matches on Subject: below --
2018-09-24 22:43 [PATCH 1/3] sim800: adding support for SimCom " Clement Viel
2018-09-25 16:48 ` Denis Kenzior
2018-09-25 18:08 ` Clement Viel
2018-09-25 18:41 ` Denis Kenzior
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.