[RFC v2 4/7] STE-plugin: Add STE plugin

2010-01-19 Thread sjur . brandeland
Added check on return value of socket() and connect() calls.
Use gsm_permissive syntax all the time.
Added ATE0 in initialization.
---
 plugins/ste.c |  262 +
 1 files changed, 262 insertions(+), 0 deletions(-)
 create mode 100644 plugins/ste.c

diff --git a/plugins/ste.c b/plugins/ste.c
new file mode 100644
index 000..6127c71
--- /dev/null
+++ b/plugins/ste.c
@@ -0,0 +1,262 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2009  Intel Corporation. All rights reserved.
+ *  Copyright (C) 2010 ST-Ericsson AB.
+ *
+ *  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 
+#endif
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#define OFONO_API_SUBJECT_TO_CHANGE
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Use the local CAIF header files until is included in linux kernel */
+#ifdef PF_CAIF
+#include 
+#include 
+#else
+#include "gcaif/caif_socket.h"
+#include "gcaif/if_caif.h"
+#endif
+
+struct ste_data {
+   GAtChat *chat;
+};
+
+static int ste_probe(struct ofono_modem *modem)
+{
+   struct ste_data *data;
+
+   DBG("%p", modem);
+
+   data = g_try_new0(struct ste_data, 1);
+
+   if (!data)
+   return -ENOMEM;
+
+   ofono_modem_set_data(modem, data);
+   return 0;
+}
+
+static void ste_remove(struct ofono_modem *modem)
+{
+   struct ste_data *data = ofono_modem_get_data(modem);
+
+   DBG("%p", modem);
+
+   ofono_modem_set_data(modem, NULL);
+
+   g_at_chat_unref(data->chat);
+   g_free(data);
+}
+
+static void ste_debug(const char *str, void *user_data)
+{
+   ofono_info("%s", str);
+}
+
+static void cfun_enable(gboolean ok, GAtResult *result, gpointer user_data)
+{
+   struct ofono_modem *modem = user_data;
+
+   DBG("");
+
+   if (!ok)
+   ofono_modem_set_powered(modem, FALSE);
+
+   ofono_modem_set_powered(modem, TRUE);
+}
+
+static int ste_enable(struct ofono_modem *modem)
+{
+   struct ste_data *data = ofono_modem_get_data(modem);
+   GIOChannel *channel;
+   GAtSyntax *syntax;
+   int fd, err;
+   struct sockaddr_caif addr = {
+   .family = AF_CAIF,
+   .u.at.type = CAIF_ATTYPE_PLAIN
+   };
+
+   DBG("%p", modem);
+
+   /* Create a CAIF socket for AT Service */
+   fd = socket(AF_CAIF, SOCK_SEQPACKET, CAIFPROTO_AT);
+
+   if (fd < 0) {
+   ofono_debug("Failed to create CAIF socket for AT");
+   return -EIO;
+   }
+   /* Connect to the AT Service at the modem */
+   err = connect(fd, (struct sockaddr *) &addr, sizeof(addr));
+
+   if (err < 0) {
+   close(fd);
+   return err;
+   }
+   channel = g_io_channel_unix_new(fd);
+
+   if (!channel)  {
+   close(fd);
+   return -EIO;
+   }
+   g_io_channel_set_close_on_unref(channel, TRUE);
+
+   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)
+   return -ENOMEM;
+
+   if (getenv("OFONO_AT_DEBUG"))
+   g_at_chat_set_debug(data->chat, ste_debug, NULL);
+
+   g_at_chat_send(data->chat, "ATE0 +CMEE=1", NULL, NULL, NULL, NULL);
+   g_at_chat_send(data->chat, "AT+CFUN=1", NULL, cfun_enable, modem, NULL);
+
+   return -EINPROGRESS;
+}
+
+static void cfun_disable(gboolean ok, GAtResult *result, gpointer user_data)
+{
+   struct ofono_modem *modem = user_data;
+   struct ste_data *data = ofono_modem_get_data(modem);
+
+   DBG("");
+
+   g_at_chat_shutdown(data->chat);
+   g_at_chat_unref(data->chat);
+   data->chat = NULL;
+
+   if (ok)
+   ofono_modem_set_powered(modem, FALSE);
+}
+
+static int ste_disable(struct ofono_modem *modem)
+{
+   struct ste_data *data = ofono_modem_get_data(modem);
+
+   DBG("%p", modem);
+
+   if (!data->chat)
+   return 0;
+
+   g_at_chat_cancel_all(data->chat);
+   g_at_chat_unregister_all(

Re: [RFC v2 4/7] STE-plugin: Add STE plugin

2010-01-19 Thread Marcel Holtmann
Hi Sjur,

> Added check on return value of socket() and connect() calls.
> Use gsm_permissive syntax all the time.
> Added ATE0 in initialization.



> +#ifdef HAVE_CONFIG_H
> +#include 
> +#endif
> +
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#define OFONO_API_SUBJECT_TO_CHANGE
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

please, please only include the atom header files you really use.

> +/* Use the local CAIF header files until is included in linux kernel */
> +#ifdef PF_CAIF
> +#include 
> +#include 
> +#else
> +#include "gcaif/caif_socket.h"
> +#include "gcaif/if_caif.h"
> +#endif

This is not gonna work. To make this work you would need a GLibc that
has the PF_CAIF defined.

My current proposal to fix this would be to include caif_socket.h and
if_caif.h in the drivers/stemodem/ directory and just use them directly.

The gcaif/ directory is the wrong approach here. We have the g* prefixed
subdirectories for libraries that provide GLib mainloop integration. You
are not doing that, you just need to extra headers. So put them side by
side with the modem driver.

We can fix the upstream usage of CAIF and their header files later then
when that has been accepted into net-next-2.6 tree.

> + int fd, err;
> + struct sockaddr_caif addr = {
> + .family = AF_CAIF,
> + .u.at.type = CAIF_ATTYPE_PLAIN
> + };

I would prefer if you do this like this:

memset(&addr, 0, sizeof(addr));
addr.family = AF_CAIF;
addr.u.at.type = CAIF_ATTYPE_PLAIN;

And just before calling connect.

Also the more important question here is if you have two CAIF modems in
the system. How do you distinguish between them?

> +
> + DBG("%p", modem);
> +
> + /* Create a CAIF socket for AT Service */
> + fd = socket(AF_CAIF, SOCK_SEQPACKET, CAIFPROTO_AT);
> +
> + if (fd < 0) {

Skip that extra empty line here.

> + ofono_debug("Failed to create CAIF socket for AT");
> + return -EIO;
> + }
> + /* Connect to the AT Service at the modem */
> + err = connect(fd, (struct sockaddr *) &addr, sizeof(addr));
> +
> + if (err < 0) {

Same here.

> + close(fd);
> + return err;
> + }
> + channel = g_io_channel_unix_new(fd);
> +
> + if (!channel)  {

And here.

> +static void ste_pre_sim(struct ofono_modem *modem)
> +{
> + struct ste_data *data = ofono_modem_get_data(modem);
> + DBG("%p", modem);
> +
> + ofono_devinfo_create(modem, 0, "atmodem", data->chat);
> + ofono_sim_create(modem, 0, "atmodem", data->chat);
> + ofono_voicecall_create(modem, 0, "stemodem", data->chat);
> +}

It will not break the build, but the functionality for the stemodem
voiceall atom is not merged yet. So leave that out.

> +static void ste_post_sim(struct ofono_modem *modem)
> +{
> + struct ste_data *data = ofono_modem_get_data(modem);
> + struct ofono_message_waiting *mw;
> + struct ofono_gprs *gprs;
> + struct ofono_gprs_context *gc;
> +
> + DBG("%p", modem);
> + ofono_ussd_create(modem, 0, "atmodem", data->chat);
> + ofono_call_forwarding_create(modem, 0, "atmodem", data->chat);
> + ofono_call_settings_create(modem, 0, "atmodem", data->chat);
> + ofono_netreg_create(modem, OFONO_VENDOR_STE, "atmodem", data->chat);
> + ofono_call_meter_create(modem, 0, "atmodem", data->chat);
> + ofono_call_barring_create(modem, 0, "atmodem", data->chat);
> + ofono_ssn_create(modem, 0, "atmodem", data->chat);
> + ofono_sms_create(modem, 0, "atmodem", data->chat);
> + ofono_phonebook_create(modem, 0, "atmodem", data->chat);
> + ofono_call_volume_create(modem, 0, "atmodem", data->chat);
> +
> + gprs = ofono_gprs_create(modem,
> + OFONO_VENDOR_STE, "atmodem", data->chat);
> + gc = ofono_gprs_context_create(modem, 0, "stemodem", data->chat);

Same applies here. Please don't do this until all the dependencies are
merged.

You can just add these extra atom via the patch introducing the support
for it. That makes the git log way cleaner and we can follow the changes
in a lot simpler way at some later time.

Regards

Marcel


___
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono