Hi Guillaume,
I applied the patch and fixed up the last issues by myself. Please
review them closely.
Free speedupcdma_data while removing modem.
Version and changelog comments have to come after the ---. Otherwise
they end up in the commit log. And without a previous commit they make
no sense then.
If in doubt, please check your patch with git am and see how it applies.
---
Makefile.am | 3 +
plugins/speedupcdma.c | 210 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 213 insertions(+), 0 deletions(-)
create mode 100644 plugins/speedupcdma.c
diff --git a/Makefile.am b/Makefile.am
index a4e6c95..4b72091 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -336,6 +336,9 @@ builtin_sources += plugins/telit.c
builtin_modules += speedup
builtin_sources += plugins/speedup.c
+builtin_modules += speedupcdma
+builtin_sources += plugins/speedupcdma.c
+
if BLUETOOTH
builtin_modules += bluetooth
builtin_sources += plugins/bluetooth.c plugins/bluetooth.h
diff --git a/plugins/speedupcdma.c b/plugins/speedupcdma.c
new file mode 100644
index 0000000..65da2f6
--- /dev/null
+++ b/plugins/speedupcdma.c
@@ -0,0 +1,210 @@
+/*
+ *
+ * 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 <stdio.h>
+#include <errno.h>
+#include <stdlib.h>
+
+#include <glib.h>
+#include <gatchat.h>
+#include <gattty.h>
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include <ofono/plugin.h>
+#include <ofono/modem.h>
+#include <ofono/devinfo.h>
+#include <ofono/cdma-connman.h>
+#include <ofono/log.h>
+
+#include <drivers/atmodem/atutil.h>
+
+static const char *none_prefix[] = { NULL };
+
+struct speedupcdma_data {
+ GAtChat *chat;
+};
+
+static void speedupcdma_debug(const char *str, void *data)
+{
+ const char *prefix = data;
+
+ ofono_info("%s%s", prefix, str);
+}
+
+static int speedupcdma_probe(struct ofono_modem *modem)
+{
+ struct speedupcdma_data *data;
+
+ DBG("%p", modem);
+
+ data = g_try_new0(struct speedupcdma_data, 1);
+ if (data == NULL)
+ return -ENOMEM;
+
+ ofono_modem_set_data(modem, data);
+
+ return 0;
+}
+
+static void speedupcdma_remove(struct ofono_modem *modem)
+{
+ struct speedupcdma_data *data = ofono_modem_get_data(modem);
+
+ DBG("%p", modem);
+
+ g_free(data);
+ ofono_modem_set_data(modem, NULL);
+}
The general rule here is to first call ofono_modem_set_data() in a
remove callback to set it back to NULL and then free the data. This is
how all of the other plugins do it as well.
It does not make a difference here since we are single threaded with a
mainloop, but it is style that we should keep.
If really in doubt, the IFX plugin is the best example since we spent a
lot of time to make it clean and be able to have it as a reference.
+static void cfun_enable(gboolean ok, GAtResult *result, gpointer
user_data)
+{
+ struct ofono_modem *modem = user_data;
+
+ DBG("");
+
+ if (!ok) {
+ struct speedupcdma_data *data = ofono_modem_get_data(modem);
While this is fine, the general style is to keep it at the top.
+
+ g_at_chat_unref(data->chat);
+ data->chat == NULL;
And seriously? This even gives a compile error:
CC plugins/speedupcdma.o
plugins/speedupcdma.c: In function ‘cfun_enable’:
plugins/speedupcdma.c:91:3: error: statement with no effect [-Werror=unused-value]
cc1: all warnings being treated as errors
I know that typos like this happen. Even I overlooked it in the v3
review, but the compiler warning should have been a big indication that
something is wrong here. That is why I insist that developers use
bootstrap-configure to enable maintainer-mode and we can catch these
ones.
In your previous code with the g_at_chat_unref() inside the remove
callback, this would have caused a crash when enabling of a modem failed
and you stop ofonod after.
+ ofono_modem_set_powered(modem, FALSE);
+ return;
+ }
+
+ ofono_modem_set_powered(modem, TRUE);
+}
+
+static int speedupcdma_enable(struct ofono_modem *modem)
+{
+ struct speedupcdma_data *data = ofono_modem_get_data(modem);
+ GAtSyntax *syntax;
+ GIOChannel *channel;
+ const char *modem_path;
+
+ modem_path = ofono_modem_get_string(modem, "Modem");
+ if (modem_path == NULL)
+ return -EINVAL;
+
+ DBG("path is: %s", modem_path);
+
+ channel = g_at_tty_open(modem_path, NULL);
+ if (channel == NULL)
+ return -EIO;
+
+ syntax = g_at_syntax_new_gsm_permissive();
+ data->chat = g_at_chat_new(channel, syntax);
+ g_at_syntax_unref(syntax);
+
+ g_io_channel_unref(channel);
+
+ if (data->chat == NULL)
+ return -ENOMEM;
+
+ if (getenv("OFONO_AT_DEBUG"))
+ g_at_chat_set_debug(data->chat, speedupcdma_debug, "Modem: ");
+
+ g_at_chat_send(data->chat, "ATE0 +CMEE=1", none_prefix,
+ NULL, NULL, NULL);
+
+ g_at_chat_send(data->chat, "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 speedupcdma_data *data = ofono_modem_get_data(modem);
+
+ DBG("");
+
+ g_at_chat_unref(data->chat);
+ data->chat = NULL;
+
+ if (ok)
+ ofono_modem_set_powered(modem, FALSE);
+}
+
+static int speedupcdma_disable(struct ofono_modem *modem)
+{
+ struct speedupcdma_data *data = ofono_modem_get_data(modem);
+
+ DBG("%p", modem);
+
+ g_at_chat_send(data->chat, "AT+CFUN=0", none_prefix,
+ cfun_disable, modem, NULL);
+
+ return -EINPROGRESS;
+}
+
+static void speedupcdma_pre_sim(struct ofono_modem *modem)
+{
+ struct speedupcdma_data *data = ofono_modem_get_data(modem);
+
+ DBG("%p", modem);
+
+ ofono_devinfo_create(modem, 0, "cdmamodem", data->chat);
+}
+
+static void speedupcdma_post_sim(struct ofono_modem *modem)
+{
+ DBG("%p", modem);
+}
+
+static void speedupcdma_post_online(struct ofono_modem *modem)
+{
+ struct speedupcdma_data *data = ofono_modem_get_data(modem);
+
+ DBG("%p", modem);
+
+ ofono_cdma_connman_create(modem, 0, "cdmamodem", data->chat);
+}
+
+static struct ofono_modem_driver speedupcdma_driver = {
+ .name = "speedupcdma",
+ .probe = speedupcdma_probe,
+ .remove = speedupcdma_remove,
+ .enable = speedupcdma_enable,
+ .disable = speedupcdma_disable,
+ .pre_sim = speedupcdma_pre_sim,
+ .post_sim = speedupcdma_post_sim,
+ .post_online = speedupcdma_post_online,
+};
+
+static int speedupcdma_init(void)
+{
+ return ofono_modem_driver_register(&speedupcdma_driver);
+}
+
+static void speedupcdma_exit(void)
+{
+ ofono_modem_driver_unregister(&speedupcdma_driver);
+}
+
+OFONO_PLUGIN_DEFINE(speedupcdma, "Speedup CDMA modem driver", VERSION,
+ OFONO_PLUGIN_PRIORITY_DEFAULT,
+ speedupcdma_init, speedupcdma_exit)
I fixed up the name to Speed Up since that is how this company name
seems to be written. At least according to my Google search ;)
Regards
Marcel