* [RFC 0/3] STE-plugin: Plugin for ST-Ericsson modems @ 2010-01-17 17:28 sjur.brandeland 2010-01-17 17:28 ` [RFC 1/3] STE-plugin: Add vendor STE sjur.brandeland 0 siblings, 1 reply; 10+ messages in thread From: sjur.brandeland @ 2010-01-17 17:28 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 1527 bytes --] From: Sjur Brandeland <sjur.brandeland@stericsson.com> We are happy to announce ST-Ericsson's participation in the oFono community and contribution of an oFono plug-in for ST-Ericsson modems. This patch set adds a plug-in for use with ST-Ericsson modems. It is implemented using AT-commands over a CAIF link to communicate with the modem. CAIF is the ST-Ericsson IPC mechanism used between host and modem. Feedback on this patch set is welcome! This patch-set is based on commit: e433ddc100bda3a437fb81805ea44348f22e9fcb Sjur Brandeland (3): STE-plugin: Add vendor STE STE-plugin: Mechanism for inheritance STE-plugin: Add STE plugin Makefile.am | 11 + drivers/atmodem/gprs.c | 15 +- drivers/atmodem/vendor.h | 5 + drivers/stemodem/gprs-context.c | 612 ++++++++++++++++++++++++++++++ drivers/stemodem/network-registration.c | 225 +++++++++++ drivers/stemodem/stemodem.c | 109 ++++++ drivers/stemodem/stemodem.h | 33 ++ drivers/stemodem/voicecall.c | 615 +++++++++++++++++++++++++++++++ include/netreg.h | 5 + include/voicecall.h | 7 +- plugins/modemconf.c | 5 + plugins/ste.c | 246 ++++++++++++ src/network.c | 22 ++ src/voicecall.c | 22 ++ 14 files changed, 1929 insertions(+), 3 deletions(-) ^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC 1/3] STE-plugin: Add vendor STE 2010-01-17 17:28 [RFC 0/3] STE-plugin: Plugin for ST-Ericsson modems sjur.brandeland @ 2010-01-17 17:28 ` sjur.brandeland 2010-01-17 17:28 ` [RFC 2/3] STE-plugin: Mechanism for inheritance sjur.brandeland 2010-01-17 21:10 ` [RFC 1/3] STE-plugin: Add vendor STE Marcel Holtmann 0 siblings, 2 replies; 10+ messages in thread From: sjur.brandeland @ 2010-01-17 17:28 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 3599 bytes --] From: Sjur Brandeland <sjur.brandeland@stericsson.com> This patch add STE as vendor, and a few adjustments needed in "atmodem" for ST-Ericsson modem. --- drivers/atmodem/gprs.c | 15 +++++++++++++-- drivers/atmodem/vendor.h | 5 +++++ plugins/modemconf.c | 5 +++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/drivers/atmodem/gprs.c b/drivers/atmodem/gprs.c index 76085d9..305f22f 100644 --- a/drivers/atmodem/gprs.c +++ b/drivers/atmodem/gprs.c @@ -17,6 +17,10 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA * + * Copyright (C) 2010 ST-Ericsson AB. + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. + * STE specific implementation. + * */ #ifdef HAVE_CONFIG_H @@ -38,6 +42,7 @@ #include "gatresult.h" #include "atmodem.h" +#include "vendor.h" static const char *cgreg_prefix[] = { "+CGREG:", NULL }; static const char *cgdcont_prefix[] = { "+CGDCONT:", NULL }; @@ -45,6 +50,7 @@ static const char *none_prefix[] = { NULL }; struct gprs_data { GAtChat *chat; + unsigned int vendor; }; static void at_cgatt_cb(gboolean ok, GAtResult *result, gpointer user_data) @@ -216,7 +222,12 @@ static void at_cgreg_test_cb(gboolean ok, GAtResult *result, g_at_chat_send(gd->chat, cmd, none_prefix, NULL, NULL, NULL); g_at_chat_send(gd->chat, "AT+CGAUTO=0", none_prefix, NULL, NULL, NULL); - g_at_chat_send(gd->chat, "AT+CGEREP=2,1", none_prefix, + + if (gd->vendor == OFONO_VENDOR_STE) + g_at_chat_send(gd->chat, "AT+CGEREP=1,0", none_prefix, + gprs_initialized, gprs, NULL); + else + g_at_chat_send(gd->chat, "AT+CGEREP=2,1", none_prefix, gprs_initialized, gprs, NULL); return; @@ -289,7 +300,7 @@ static int at_gprs_probe(struct ofono_gprs *gprs, gd = g_new0(struct gprs_data, 1); gd->chat = chat; - + gd->vendor = vendor; ofono_gprs_set_data(gprs, gd); g_at_chat_send(chat, "AT+CGDCONT=?", cgdcont_prefix, diff --git a/drivers/atmodem/vendor.h b/drivers/atmodem/vendor.h index 8d9ed47..d7b5210 100644 --- a/drivers/atmodem/vendor.h +++ b/drivers/atmodem/vendor.h @@ -17,11 +17,16 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA * + * Copyright (C) 2010 ST-Ericsson AB. + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. + * STE specific implementation. + * */ enum ofono_vendor { OFONO_VENDOR_GENERIC = 0, OFONO_VENDOR_CALYPSO, + OFONO_VENDOR_STE, OFONO_VENDOR_QUALCOMM_MSM, OFONO_VENDOR_OPTION_HSO, }; diff --git a/plugins/modemconf.c b/plugins/modemconf.c index c8c261f..41c7428 100644 --- a/plugins/modemconf.c +++ b/plugins/modemconf.c @@ -17,6 +17,10 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA * + * Copyright (C) 2010 ST-Ericsson AB. + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. + * STE specific implementation. + * */ #ifdef HAVE_CONFIG_H @@ -128,6 +132,7 @@ static struct ofono_modem *create_modem(GKeyFile *keyfile, const char *group) set_address(modem, keyfile, group); if (!g_strcmp0(driver, "atgen") || !g_strcmp0(driver, "g1") || + !g_strcmp0(driver, "ste") || !g_strcmp0(driver, "calypso") || !g_strcmp0(driver, "hfp") || !g_strcmp0(driver, "palmpre")) -- 1.6.0.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC 2/3] STE-plugin: Mechanism for inheritance 2010-01-17 17:28 ` [RFC 1/3] STE-plugin: Add vendor STE sjur.brandeland @ 2010-01-17 17:28 ` sjur.brandeland 2010-01-17 17:28 ` [RFC 3/3] STE-plugin: Adding STE plugin sjur.brandeland 2010-01-17 21:10 ` [RFC 1/3] STE-plugin: Add vendor STE Marcel Holtmann 1 sibling, 1 reply; 10+ messages in thread From: sjur.brandeland @ 2010-01-17 17:28 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 4940 bytes --] From: Sjur Brandeland <sjur.brandeland@stericsson.com> This patch adds a mechanism to inherit/specialize from the standard "atmodem". This is used in the initialization of voice call driver and the network registration driver in order to re-use functions from "atmodems" whenever possible. We realize this is not the standard way of doing it, so I would appreciate your comments on this approach. --- include/netreg.h | 5 +++++ include/voicecall.h | 7 ++++++- src/network.c | 22 ++++++++++++++++++++++ src/voicecall.c | 22 ++++++++++++++++++++++ 4 files changed, 55 insertions(+), 1 deletions(-) diff --git a/include/netreg.h b/include/netreg.h old mode 100644 new mode 100755 index 0079477..ee0b201 --- a/include/netreg.h +++ b/include/netreg.h @@ -17,6 +17,10 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA * + * Copyright (C) 2010 ST-Ericsson AB. + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. + * STE specific implementation. + * */ #ifndef __OFONO_NETREG_H @@ -96,6 +100,7 @@ void ofono_netreg_status_notify(struct ofono_netreg *netreg, int status, int ofono_netreg_driver_register(const struct ofono_netreg_driver *d); void ofono_netreg_driver_unregister(const struct ofono_netreg_driver *d); +struct ofono_netreg_driver *ofono_netreg_driver_get(const char *driver); struct ofono_netreg *ofono_netreg_create(struct ofono_modem *modem, unsigned int vendor, diff --git a/include/voicecall.h b/include/voicecall.h index 6ceb3d8..ca43c91 100644 --- a/include/voicecall.h +++ b/include/voicecall.h @@ -17,6 +17,10 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA * + * Copyright (C) 2010 ST-Ericsson AB. + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. + * STE specific implementation. + * */ #ifndef __OFONO_VOICECALL_H @@ -29,7 +33,7 @@ extern "C" { #include <ofono/types.h> struct ofono_voicecall; - +struct ofono_modem; typedef void (*ofono_voicecall_cb_t)(const struct ofono_error *error, void *data); @@ -102,6 +106,7 @@ void ofono_voicecall_disconnected(struct ofono_voicecall *vc, int id, int ofono_voicecall_driver_register(const struct ofono_voicecall_driver *d); void ofono_voicecall_driver_unregister(const struct ofono_voicecall_driver *d); +struct ofono_voicecall_driver *ofono_voicecall_driver_get(const char *driver); struct ofono_voicecall *ofono_voicecall_create(struct ofono_modem *modem, unsigned int vendor, diff --git a/src/network.c b/src/network.c index 8b4eb09..75ee98b 100644 --- a/src/network.c +++ b/src/network.c @@ -17,6 +17,10 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA * + * Copyright (C) 2010 ST-Ericsson AB. + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. + * STE specific implementation. + * */ #ifdef HAVE_CONFIG_H @@ -1596,6 +1600,24 @@ void ofono_netreg_driver_unregister(const struct ofono_netreg_driver *d) g_drivers = g_slist_remove(g_drivers, (void *)d); } +struct ofono_netreg_driver *ofono_netreg_driver_get(const char *driver) +{ + GSList *l; + + if (driver == NULL) + return NULL; + + for (l = g_drivers; l; l = l->next) { + struct ofono_netreg_driver *drv = l->data; + + if (g_strcmp0(drv->name, driver)) + continue; + else + return drv; + } + return NULL; +} + static void netreg_unregister(struct ofono_atom *atom) { struct ofono_netreg *netreg = __ofono_atom_get_data(atom); diff --git a/src/voicecall.c b/src/voicecall.c index 73de35f..1cd5bd7 100644 --- a/src/voicecall.c +++ b/src/voicecall.c @@ -17,6 +17,10 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA * + * Copyright (C) 2010 ST-Ericsson AB. + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. + * STE specific implementation. + * */ #ifdef HAVE_CONFIG_H @@ -1751,6 +1755,24 @@ void ofono_voicecall_driver_unregister(const struct ofono_voicecall_driver *d) g_drivers = g_slist_remove(g_drivers, (void *)d); } +struct ofono_voicecall_driver *ofono_voicecall_driver_get(const char *driver) +{ + GSList *l; + + if (driver == NULL) + return NULL; + + for (l = g_drivers; l; l = l->next) { + struct ofono_voicecall_driver *drv = l->data; + + if (g_strcmp0(drv->name, driver)) + continue; + else + return drv; + } + return NULL; +} + static void voicecall_unregister(struct ofono_atom *atom) { DBusConnection *conn = ofono_dbus_get_connection(); -- 1.6.0.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC 3/3] STE-plugin: Adding STE plugin 2010-01-17 17:28 ` [RFC 2/3] STE-plugin: Mechanism for inheritance sjur.brandeland @ 2010-01-17 17:28 ` sjur.brandeland 2010-01-17 21:40 ` Marcel Holtmann 2010-01-17 22:50 ` Denis Kenzior 0 siblings, 2 replies; 10+ messages in thread From: sjur.brandeland @ 2010-01-17 17:28 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 51585 bytes --] From: Sjur Brandeland <sjur.brandeland@stericsson.com> Added implementation for STE modem; STE modem driver, and STE specific drivers for gprs, network registration and voice call. This patch uses CAIF sockets. CAIF patch for net-next-2.6 will be contributed on netdev(a)vger.kernel.org soon. --- Makefile.am | 11 + drivers/stemodem/gprs-context.c | 612 ++++++++++++++++++++++++++++++ drivers/stemodem/network-registration.c | 225 +++++++++++ drivers/stemodem/stemodem.c | 109 ++++++ drivers/stemodem/stemodem.h | 33 ++ drivers/stemodem/voicecall.c | 615 +++++++++++++++++++++++++++++++ plugins/ste.c | 246 ++++++++++++ 7 files changed, 1851 insertions(+), 0 deletions(-) diff --git a/Makefile.am b/Makefile.am index 276b478..bae8dcf 100644 --- a/Makefile.am +++ b/Makefile.am @@ -129,6 +129,14 @@ builtin_sources += drivers/atmodem/atutil.h \ drivers/calypsomodem/calypsomodem.c \ drivers/calypsomodem/voicecall.c +builtin_modules += stemodem +builtin_sources += drivers/atmodem/atutil.h \ + drivers/stemodem/stemodem.h \ + drivers/stemodem/stemodem.c \ + drivers/stemodem/voicecall.c \ + drivers/stemodem/network-registration.c \ + drivers/stemodem/gprs-context.c + builtin_modules += hfpmodem builtin_sources += drivers/atmodem/atutil.h \ drivers/hfpmodem/hfpmodem.h \ @@ -168,6 +176,9 @@ builtin_sources += plugins/g1.c builtin_modules += calypso builtin_sources += plugins/calypso.c +builtin_modules += ste +builtin_sources += plugins/ste.c + builtin_modules += mbm builtin_sources += plugins/mbm.c diff --git a/drivers/stemodem/gprs-context.c b/drivers/stemodem/gprs-context.c new file mode 100644 index 0000000..1d5d8db --- /dev/null +++ b/drivers/stemodem/gprs-context.c @@ -0,0 +1,612 @@ +/* + * + * oFono - Open Source Telephony + * + * Copyright (C) 2008-2009 Intel Corporation. All rights reserved. + * Copyright (C) 2010 ST-Ericsson AB. + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. + * + * 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 + +#define _GNU_SOURCE +#include <string.h> +#include <stdlib.h> +#include <stdio.h> + +#include <glib.h> + +#include <ofono/log.h> +#include <ofono/modem.h> +#include <ofono/gprs-context.h> +#include <ofono/gprs.h> + +#include <linux/types.h> +#include <net/if.h> +#include <sys/ioctl.h> +#include <arpa/inet.h> +#include <linux/caif/caif_socket.h> +#include <linux/caif/if_caif.h> + +#include "gatchat.h" +#include "gatresult.h" +#include "stemodem.h" + +#define MAX_CAIF_DEVICES 7 +#define MAX_DNS 5 +#define AUTH_BUF_LENGTH (OFONO_GPRS_MAX_USERNAME_LENGTH + \ + OFONO_GPRS_MAX_PASSWORD_LENGTH + 128) + +static const char *cgact_prefix[] = { "+CGACT:", NULL }; +static const char *none_prefix[] = { NULL }; + +static GSList *g_caif_devices; + +struct gprs_context_data { + GAtChat *chat; + unsigned int active_context; + char *username; + char *password; +}; + +struct conn_info { + unsigned int cid; + unsigned int device; + unsigned int channel_id; + char *interface; +}; + +static gint conn_compare_by_cid(gconstpointer a, gconstpointer b) +{ + const struct conn_info *conn = a; + unsigned int used = GPOINTER_TO_UINT(b); + + if (used != conn->cid) + return 1; + + return 0; +} + +/* TODO: should parse_xml function to be moved to e.g. atutil? */ +static char *parse_xml(char * xml, char* tag) +{ + char *begin; + char *end; + int len; + char *res = NULL; + char *start = (char *)g_malloc(strlen(tag)+3); + char *stop = (char *)g_malloc(strlen(tag)+4); + + sprintf(start, "<%s>", tag); + sprintf(stop, "</%s>", tag); + + begin = strstr(xml, start); + if (begin == NULL) + goto error; + + end = strstr(begin, stop); + if (end == NULL) + goto error; + + begin += strlen(start); + len = end - begin; + res = (char *)g_malloc(len+1); + strncpy(res, begin, len); + res[len] = 0; + +error: + free(start); + free(stop); + return res; +} + +static struct conn_info *conn_info_create( + unsigned int device, + unsigned int channel_id) +{ + struct conn_info *connection = g_try_new0(struct conn_info, 1); + + if (!connection) + return NULL; + + connection->cid = 0; + connection->device = device; + connection->channel_id = channel_id; + + return connection; +} + +/* Creates a new or removes an existing IP interface for CAIF. + * Sets the ip address, subnet mask and MTU size at creation. + * +*/ +static gboolean caif_if_create_remove(const char *interface, + const char *ip, const char *mtu, + const char *mask, unsigned int connid, + gboolean create) +{ + int s; + struct sockaddr_in sin; + static struct ifcaif_param param; + static struct ifreq ifr; + + param.ipv4_connid = connid; + ifr.ifr_data = (void *) ¶m; + strcpy(ifr.ifr_name, interface); + + s = socket(AF_CAIF, SOCK_SEQPACKET, CAIFPROTO_AT); + if (s < 0) { + ofono_debug("Failed to create socket for CAIF interface"); + goto error; + } + + if (create) { + if (ioctl(s, SIOCCAIFNETNEW, &ifr) < 0) { + ofono_debug("Failed to create IP interface for CAIF"); + goto error; + } + + s = socket(PF_INET, SOCK_DGRAM, 0); + if (s < 0) { + ofono_debug("Failed to create socket."); + goto error; + } + + /* Set IP address */ + memset(&sin, 0, sizeof(struct sockaddr)); + sin.sin_family = AF_INET; + + if (inet_pton(AF_INET, ip, &sin.sin_addr) <= 0) { + ofono_debug("inet_pton failed, will not be" + "able to set the IP address"); + goto error; + } + memcpy(&ifr.ifr_addr, &sin, sizeof(struct sockaddr)); + + if (ioctl(s, SIOCSIFADDR, &ifr) < 0) { + ofono_debug("Failed to set IP address for" + " interface: %s", ifr.ifr_name); + goto error; + } + + /* Set subnet mask */ + memset(&sin, 0, sizeof(struct sockaddr)); + sin.sin_family = AF_INET; + + if (inet_pton(AF_INET, mask, &sin.sin_addr) <= 0) { + ofono_debug("inet_pton failed, will not be" + "able to set the subnet mask"); + } else { + memcpy(&ifr.ifr_addr, &sin, sizeof(struct sockaddr)); + + if (ioctl(s, SIOCSIFNETMASK, &ifr) < 0) + ofono_debug("Failed to set subnet mask for" + "interface: %s", ifr.ifr_name); + } + + /* Set MTU */ + ifr.ifr_mtu = atoi(mtu); + + if (ioctl(s, SIOCSIFMTU, &ifr) < 0) + ofono_debug("Failed to set MTU for interface: %s", + ifr.ifr_name); + } else { + if (ioctl(s, SIOCGIFINDEX, &ifr) == 0) { + if (ioctl(s, SIOCCAIFNETREMOVE, &ifr) < 0) { + ofono_debug("Failed to remove IP interface" + "for CAIF"); + goto error; + } + } else { + ofono_debug("Did not find interface (%s) to remove", + interface); + goto error; + } + } + + return TRUE; + +error: + return FALSE; +} + +static void ste_eppsd_down_cb(gboolean ok, + GAtResult *result, gpointer user_data) +{ + struct cb_data *cbd = user_data; + ofono_gprs_context_cb_t cb = cbd->cb; + struct ofono_gprs_context *gc = cbd->user; + struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc); + struct ofono_error error; + struct conn_info *conn; + GSList *l; + + dump_response("ste_eppsd_down_cb", ok, result); + + if (!ok) + goto error; + + l = g_slist_find_custom(g_caif_devices, + GUINT_TO_POINTER(gcd->active_context), + conn_compare_by_cid); + + if (!l) { + ofono_debug("Did not find data (used caif device) for" + "connection with cid; %d", + gcd->active_context); + goto error; + } + conn = l->data; + if (!caif_if_create_remove(conn->interface, NULL, + NULL, NULL, conn->channel_id, FALSE)) { + ofono_debug("Failed to remove caif interface %s.", + conn->interface); + } + conn->cid = 0; + + decode_at_error(&error, g_at_result_final_response(result)); + cb(&error, cbd->data); + return; + +error: + CALLBACK_WITH_FAILURE(cb, cbd->data); +} + +static void ste_eppsd_up_cb(gboolean ok, GAtResult *result, gpointer user_data) +{ + struct cb_data *cbd = user_data; + ofono_gprs_context_up_cb_t cb = cbd->cb; + struct ofono_gprs_context *gc = cbd->user; + struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc); + struct conn_info *conn = NULL; + GAtResultIter iter; + GSList *l; + int i; + int numdns = 0; + char *res_string; + const char *ip = NULL; + const char *netmask = NULL; + const char *gateway = NULL; + const char *mtu = NULL; + const char *dns[MAX_DNS + 1]; + const char *str; + + dump_response("ste_eppsd_up_cb", ok, result); + + l = g_slist_find_custom(g_caif_devices, + GUINT_TO_POINTER(gcd->active_context), + conn_compare_by_cid); + + if (!l) { + ofono_debug("Did not find data (device and channel id)" + "for connection with cid; %d", + gcd->active_context); + goto error; + } + conn = l->data; + + if (!ok) + goto error; + + g_at_result_iter_init(&iter, result); + for (i = 0; i < g_at_result_num_response_lines(result); i++) { + g_at_result_iter_next(&iter, NULL); + res_string = strdup(g_at_result_iter_raw_line(&iter)); + + if (strstr(res_string, "ip_address")) { + ip = g_strdup(parse_xml(res_string, + "ip_address")); + } else if ((strstr(res_string, "subnet_mask"))) { + netmask = g_strdup(parse_xml(res_string, + "subnet_mask")); + } else if ((strstr(res_string, "mtu"))) { + mtu = g_strdup(parse_xml(res_string, + "mtu")); + } else if ((strstr(res_string, "default_gateway"))) { + gateway = g_strdup(parse_xml(res_string, + "default_gateway")); + } else if ((strstr(res_string, "dns_server"))) { + str = g_strdup(parse_xml(res_string, + "dns_server")); + + if (numdns < MAX_DNS) + dns[numdns++] = str; + } + } + dns[numdns] = NULL; + + conn->interface = g_malloc(10); + if (!conn->interface) + goto error; + + sprintf(conn->interface, "caif%u", conn->device); + + if (!caif_if_create_remove(conn->interface, ip, + mtu, netmask, conn->channel_id, TRUE)) { + ofono_error("Failed to create caif interface %s.", + conn->interface); + conn->interface = NULL; + } + + /* Set back to strict syntax check */ + g_at_chat_set_syntax(gcd->chat, g_at_syntax_new_gsmv1()); + + CALLBACK_WITH_SUCCESS(cb, conn->interface, + FALSE, ip, netmask, gateway, dns, cbd->data); + return; + +error: + ofono_debug("ste_eppsd_up_cb error"); + + if (conn) + conn->cid = 0; + + CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, cbd->data); +} + +static void ste_cgdcont_cb(gboolean ok, GAtResult *result, gpointer user_data) +{ + struct cb_data *cbd = user_data; + ofono_gprs_context_up_cb_t cb = cbd->cb; + struct ofono_gprs_context *gc = cbd->user; + struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc); + struct cb_data *ncbd = NULL; + struct conn_info *conn; + char buf[AUTH_BUF_LENGTH]; + GSList *l; + + dump_response("cgdcont_cb", ok, result); + + if (!ok) { + struct ofono_error error; + + gcd->active_context = 0; + + decode_at_error(&error, g_at_result_final_response(result)); + cb(&error, NULL, 0, NULL, NULL, NULL, NULL, cbd->data); + return; + } + + /* Set username and password */ + sprintf(buf, "AT*EIAAUW=%d,1,\"%s\",\"%s\"", gcd->active_context, + gcd->username, gcd->password); + + if (g_at_chat_send(gcd->chat, buf, none_prefix, + NULL, NULL, NULL) == 0) + goto error; + + ncbd = g_memdup(cbd, sizeof(struct cb_data)); + + /* Need to change to gsm_permissive syntax in order to + * parse the response from EPPSD (xml) */ + g_at_chat_set_syntax(gcd->chat, g_at_syntax_new_gsm_permissive()); + + l = g_slist_find_custom(g_caif_devices, GUINT_TO_POINTER(0), + conn_compare_by_cid); + + if (!l) { + ofono_debug("at_cgdcont_cb, no more available devices"); + goto error; + } + conn = l->data; + conn->cid = gcd->active_context; + sprintf(buf, "AT*EPPSD=1,%u,%u", conn->channel_id, conn->cid); + + if (g_at_chat_send(gcd->chat, buf, NULL, + ste_eppsd_up_cb, ncbd, g_free) > 0) + return; +error: + if (ncbd) + g_free(ncbd); + + gcd->active_context = 0; + + CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, + NULL, NULL, cbd->data); +} + +static void ste_gprs_activate_primary(struct ofono_gprs_context *gc, + const struct ofono_gprs_primary_context *ctx, + ofono_gprs_context_up_cb_t cb, void *data) +{ + struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc); + struct cb_data *cbd = cb_data_new(cb, data); + char buf[OFONO_GPRS_MAX_APN_LENGTH + 128]; + int len; + + if (!cbd) + goto error; + + gcd->active_context = ctx->cid; + gcd->username = g_strdup(ctx->username); + gcd->password = g_strdup(ctx->password); + cbd->user = gc; + + len = sprintf(buf, "AT+CGDCONT=%u,\"IP\"", ctx->cid); + + if (ctx->apn) + snprintf(buf + len, sizeof(buf) - len - 3, ",\"%s\"", + ctx->apn); + + if (g_at_chat_send(gcd->chat, buf, none_prefix, + ste_cgdcont_cb, cbd, g_free) > 0) + return; +error: + if (cbd) + g_free(cbd); + + CALLBACK_WITH_FAILURE(cb, NULL, 0, NULL, NULL, NULL, NULL, data); +} + +static void ste_gprs_deactivate_primary(struct ofono_gprs_context *gc, + unsigned int id, + ofono_gprs_context_cb_t cb, void *data) +{ + struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc); + struct cb_data *cbd = cb_data_new(cb, data); + struct conn_info *conn; + char buf[64]; + GSList *l; + + if (!cbd) + goto error; + + gcd->active_context = id; + cbd->user = gc; + + l = g_slist_find_custom(g_caif_devices, GUINT_TO_POINTER(id), + conn_compare_by_cid); + + if (!l) { + ofono_debug("at_gprs_deactivate_primary, did not find" + "data (channel id) for connection with cid; %d", id); + goto error; + } + conn = l->data; + + sprintf(buf, "AT*EPPSD=0,%u,%u", conn->channel_id, id); + + if (g_at_chat_send(gcd->chat, buf, none_prefix, + ste_eppsd_down_cb, cbd, g_free) > 0) + return; + +error: + if (cbd) + g_free(cbd); + + CALLBACK_WITH_FAILURE(cb, data); +} + +static void ste_cgact_read_cb(gboolean ok, GAtResult *result, + gpointer user_data) +{ + struct ofono_gprs_context *gc = user_data; + struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc); + gint cid, state; + GAtResultIter iter; + + dump_response("cgact_read_cb", ok, result); + + if (!ok) + return; + g_at_result_iter_init(&iter, result); + + while (g_at_result_iter_next(&iter, "+CGACT:")) { + + if (!g_at_result_iter_next_number(&iter, &cid)) + continue; + + if ((unsigned int) cid != gcd->active_context) + continue; + + if (!g_at_result_iter_next_number(&iter, &state)) + continue; + + if (state == 1) + continue; + + ofono_gprs_context_deactivated(gc, gcd->active_context); + gcd->active_context = 0; + + break; + } +} + +static void cgev_notify(GAtResult *result, gpointer user_data) +{ + struct ofono_gprs_context *gc = user_data; + struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc); + GAtResultIter iter; + const char *event; + + dump_response("cgev_notify", TRUE, result); + + g_at_result_iter_init(&iter, result); + + if (!g_at_result_iter_next(&iter, "+CGEV:")) + return; + + if (!g_at_result_iter_next_unquoted_string(&iter, &event)) + return; + + if (g_str_has_prefix(event, "NW REACT ") || + g_str_has_prefix(event, "NW DEACT ") || + g_str_has_prefix(event, "ME DEACT ")) { + /* Ask what primary contexts are active now */ + + g_at_chat_send(gcd->chat, "AT+CGACT?", cgact_prefix, + ste_cgact_read_cb, gc, NULL); + + return; + } +} + +static int ste_gprs_context_probe(struct ofono_gprs_context *gc, + unsigned int vendor, void *data) +{ + GAtChat *chat = data; + struct gprs_context_data *gcd; + struct conn_info *ci; + int i; + + gcd = g_new0(struct gprs_context_data, 1); + gcd->chat = chat; + + g_at_chat_register(gcd->chat, "+CGEV:", cgev_notify, FALSE, gc, NULL); + + ofono_gprs_context_set_data(gc, gcd); + + for (i = 0; i < MAX_CAIF_DEVICES; i++) { + ci = conn_info_create(i, i+1); + if (ci) + g_caif_devices = g_slist_append(g_caif_devices, ci); + } + + return 0; +} + +static void ste_gprs_context_remove(struct ofono_gprs_context *gc) +{ + struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc); + + g_slist_foreach(g_caif_devices, (GFunc) g_free, NULL); + g_slist_free(g_caif_devices); + g_caif_devices = NULL; + + ofono_gprs_context_set_data(gc, NULL); + g_free(gcd); +} + +static struct ofono_gprs_context_driver driver = { + .name = "stemodem", + .probe = ste_gprs_context_probe, + .remove = ste_gprs_context_remove, + .activate_primary = ste_gprs_activate_primary, + .deactivate_primary = ste_gprs_deactivate_primary, +}; + +void ste_gprs_context_init() +{ + ofono_gprs_context_driver_register(&driver); +} + +void ste_gprs_context_exit() +{ + ofono_gprs_context_driver_unregister(&driver); +} diff --git a/drivers/stemodem/network-registration.c b/drivers/stemodem/network-registration.c new file mode 100644 index 0000000..4eeb239 --- /dev/null +++ b/drivers/stemodem/network-registration.c @@ -0,0 +1,225 @@ +/* + * + * oFono - Open Source Telephony + * + * Copyright (C) 2008-2009 Intel Corporation. All rights reserved. + * Copyright (C) 2010 ST-Ericsson AB. + * Author: Marit Henriksen, marit.xx.henriksen@stericsson.com. + * + * 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 + +#define _GNU_SOURCE +#include <string.h> +#include <stdlib.h> +#include <stdio.h> + +#include <glib.h> + +#include <ofono/log.h> +#include <ofono/modem.h> +#include <ofono/netreg.h> + +#include "gatchat.h" +#include "gatresult.h" +#include "stemodem.h" + +static const char *cind_prefix[] = { "+CIND:", NULL }; + +struct netreg_data { + GAtChat *chat; + char mcc[OFONO_MAX_MCC_LENGTH + 1]; + char mnc[OFONO_MAX_MNC_LENGTH + 1]; +}; + +static void ciev_notify(GAtResult *result, gpointer user_data) +{ + struct ofono_netreg *netreg = user_data; + int strength, ind; + GAtResultIter iter; + + dump_response("ciev_notify", TRUE, result); + + g_at_result_iter_init(&iter, result); + + if (!g_at_result_iter_next(&iter, "+CIEV:")) + return; + + if (!g_at_result_iter_next_number(&iter, &ind)) + return; + + if (ind == 2) { /* signal strength indication */ + if (!g_at_result_iter_next_number(&iter, &strength)) + return; + + strength = (strength * 100) / 5; + ofono_netreg_strength_notify(netreg, strength); + } +} + +static void cind_cb(gboolean ok, GAtResult *result, gpointer user_data) +{ + struct cb_data *cbd = user_data; + ofono_netreg_strength_cb_t cb = cbd->cb; + int strength; + GAtResultIter iter; + struct ofono_error error; + + dump_response("cind_cb", ok, result); + decode_at_error(&error, g_at_result_final_response(result)); + + if (!ok) { + cb(&error, -1, cbd->data); + return; + } + + g_at_result_iter_init(&iter, result); + + if (!g_at_result_iter_next(&iter, "+CIND:")) { + CALLBACK_WITH_FAILURE(cb, -1, cbd->data); + return; + } + + /* Skip battery charge level, which is the first reported */ + g_at_result_iter_skip_next(&iter); + + g_at_result_iter_next_number(&iter, &strength); + + strength = (strength * 100) / 5; + + cb(&error, strength, cbd->data); +} + +static void ste_signal_strength(struct ofono_netreg *netreg, + ofono_netreg_strength_cb_t cb, void *data) +{ + struct netreg_data *nd = ofono_netreg_get_data(netreg); + struct cb_data *cbd = cb_data_new(cb, data); + + if (!cbd) + goto error; + + if (g_at_chat_send(nd->chat, "AT+CIND?", cind_prefix, + cind_cb, cbd, g_free) > 0) + return; + +error: + if (cbd) + g_free(cbd); + + CALLBACK_WITH_FAILURE(cb, -1, data); +} + +static void creg_notify(GAtResult *result, gpointer user_data) +{ + struct ofono_netreg *netreg = user_data; + GAtResultIter iter; + int status; + int lac = -1, ci = -1, tech = -1; + const char *str; + + dump_response("creg_notify", TRUE, result); + + g_at_result_iter_init(&iter, result); + + if (!g_at_result_iter_next(&iter, "+CREG:")) + return; + + g_at_result_iter_next_number(&iter, &status); + + if (g_at_result_iter_next_string(&iter, &str) == TRUE) + lac = strtol(str, NULL, 16); + else + goto out; + + if (g_at_result_iter_next_string(&iter, &str) == TRUE) + ci = strtol(str, NULL, 16); + else + goto out; + + g_at_result_iter_next_number(&iter, &tech); + +out: + ofono_debug("creg_notify: %d, %d, %d, %d", status, lac, ci, tech); + + ofono_netreg_status_notify(netreg, status, lac, ci, tech); +} + +static void ste_network_registration_initialized(gboolean ok, GAtResult *result, + gpointer user_data) +{ + struct ofono_netreg *netreg = user_data; + struct netreg_data *nd = ofono_netreg_get_data(netreg); + + if (!ok) { + ofono_error("Unable to initialize Network Registration"); + ofono_netreg_remove(netreg); + return; + } + + g_at_chat_register(nd->chat, "+CREG:", + creg_notify, FALSE, netreg, NULL); + + g_at_chat_register(nd->chat, "+CIEV:", + ciev_notify, FALSE, netreg, NULL); + + ofono_netreg_register(netreg); +} + +static int ste_netreg_probe(struct ofono_netreg *netreg, unsigned int vendor, + void *data) +{ + GAtChat *chat = data; + struct netreg_data *nd; + + nd = g_new0(struct netreg_data, 1); + + nd->chat = chat; + ofono_netreg_set_data(netreg, nd); + + g_at_chat_send(chat, "AT+CMER=3,0,0,1", NULL, NULL, NULL, NULL); + + g_at_chat_send(chat, "AT+CREG=2", NULL, + ste_network_registration_initialized, + netreg, NULL); + return 0; +} + +static struct ofono_netreg_driver driver = { + .name = "stemodem", + .probe = ste_netreg_probe, + .remove = NULL, + .registration_status = NULL, + .current_operator = NULL, + .list_operators = NULL, + .register_auto = NULL, + .register_manual = NULL, + .deregister = NULL, + .strength = ste_signal_strength, +}; + +void ste_netreg_init() +{ + ofono_netreg_driver_register(&driver); +} + +void ste_netreg_exit() +{ + ofono_netreg_driver_unregister(&driver); +} diff --git a/drivers/stemodem/stemodem.c b/drivers/stemodem/stemodem.c new file mode 100644 index 0000000..0d840ea --- /dev/null +++ b/drivers/stemodem/stemodem.c @@ -0,0 +1,109 @@ +/* + * + * oFono - Open Source Telephony + * + * Copyright (C) 2008-2009 Intel Corporation. All rights reserved. + * Copyright (C) 2010 ST-Ericsson AB. + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. + * + * 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 <glib.h> +#include <gatchat.h> +#include <ofono/log.h> + +#define OFONO_API_SUBJECT_TO_CHANGE +#include <ofono/plugin.h> +#include <ofono/types.h> +#include <ofono/voicecall.h> +#include <ofono/netreg.h> + +#include "stemodem.h" + +static int stemodem_init(void) +{ + /* Initialize voicecall driver */ + struct ofono_voicecall_driver *at_vcdrv; + struct ofono_voicecall_driver *ste_vcdrv; + struct ofono_netreg_driver *at_netdrv; + struct ofono_netreg_driver *ste_netdrv; + + ste_voicecall_init(); + + at_vcdrv = ofono_voicecall_driver_get("atmodem"); + ste_vcdrv = ofono_voicecall_driver_get("stemodem"); + + if (at_vcdrv && ste_vcdrv) { + ste_vcdrv->remove = at_vcdrv->remove; + ste_vcdrv->swap_without_accept = at_vcdrv->swap_without_accept; + ste_vcdrv->send_tones = at_vcdrv->send_tones; + } else { + if (!ste_vcdrv) + ofono_debug("Could not get ofono_voicecall_driver" + "from stemodem"); + if (!at_vcdrv) + ofono_debug("Could not get ofono_voicecall_driver" + "from atmodem"); + } + + /* Initialize netreg driver */ + ste_netreg_init(); + + at_netdrv = ofono_netreg_driver_get("atmodem"); + ste_netdrv = ofono_netreg_driver_get("stemodem"); + + if (at_netdrv && ste_netdrv) { + ste_netdrv->remove = at_netdrv->remove; + ste_netdrv->registration_status = + at_netdrv->registration_status; + ste_netdrv->current_operator = at_netdrv->current_operator; + ste_netdrv->list_operators = at_netdrv->list_operators; + ste_netdrv->register_auto = at_netdrv->register_auto; + ste_netdrv->register_manual = at_netdrv->register_manual; + } else { + if (!ste_netdrv) + ofono_debug("Could not get ofono_netreg_driver driver" + "from stemodem"); + if (!at_netdrv) + ofono_debug("Could not get ofono_netreg_driver driver" + "from atemodem"); + } + + /* Initialize gprs driver */ + ste_gprs_context_init(); + + return 0; +} + +static void stemodem_exit(void) +{ + ste_voicecall_exit(); + ste_netreg_exit(); + ste_gprs_context_exit(); +} + +/* atmodem must be registered before stemodem (for the ofono_netreg_driver_get + * to return correct information). This is achieved by setting priority low + * for stemodem; OFONO_PLUGIN_PRIORITY_LOW */ + +OFONO_PLUGIN_DEFINE(stemodem, "STE modem driver", VERSION, + OFONO_PLUGIN_PRIORITY_LOW, + stemodem_init, stemodem_exit) diff --git a/drivers/stemodem/stemodem.h b/drivers/stemodem/stemodem.h new file mode 100644 index 0000000..83bdcaa --- /dev/null +++ b/drivers/stemodem/stemodem.h @@ -0,0 +1,33 @@ +/* + * + * oFono - Open Source Telephony + * + * Copyright (C) 2008-2009 Intel Corporation. All rights reserved. + * Copyright (C) 2010 ST-Ericsson AB. + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. + * + * 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 + * + */ + +#include <drivers/atmodem/atutil.h> + +extern void ste_voicecall_init(); +extern void ste_voicecall_exit(); + +extern void ste_netreg_init(); +extern void ste_netreg_exit(); + +extern void ste_gprs_context_init(); +extern void ste_gprs_context_exit(); diff --git a/drivers/stemodem/voicecall.c b/drivers/stemodem/voicecall.c new file mode 100644 index 0000000..c00fc61 --- /dev/null +++ b/drivers/stemodem/voicecall.c @@ -0,0 +1,615 @@ +/* + * + * oFono - Open Source Telephony + * + * Copyright (C) 2008-2009 Intel Corporation. All rights reserved. + * Copyright (C) 2010 ST-Ericsson AB. + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. + * + * 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 + +#define _GNU_SOURCE +#include <string.h> +#include <stdlib.h> +#include <stdio.h> + +#include <glib.h> + +#include <ofono/log.h> +#include <ofono/modem.h> +#include <ofono/voicecall.h> + +#include "gatchat.h" +#include "gatresult.h" +#include "common.h" + +#include "stemodem.h" + +enum call_status_ste { + STE_CALL_STATUS_IDLE = 0, + STE_CALL_STATUS_CALLING = 1, + STE_CALL_STATUS_CONNECTING = 2, + STE_CALL_STATUS_ACTIVE = 3, + STE_CALL_STATUS_HOLD = 4, + STE_CALL_STATUS_WAITING = 5, + STE_CALL_STATUS_ALERTING = 6, + STE_CALL_STATUS_BUSY = 7 +}; + +static const char *none_prefix[] = { NULL }; + +struct voicecall_data { + GSList *calls; + unsigned int local_release; + GAtChat *chat; +}; + +struct release_id_req { + struct ofono_voicecall *vc; + ofono_voicecall_cb_t cb; + void *data; + int id; +}; + +struct change_state_req { + struct ofono_voicecall *vc; + ofono_voicecall_cb_t cb; + void *data; + int affected_types; +}; + +/* Translate from the ECAV-based STE-status to CLCC based status */ +static int call_status_ste_to_ofono(int status) +{ + switch (status) { + case STE_CALL_STATUS_IDLE: + return CALL_STATUS_DISCONNECTED; + case STE_CALL_STATUS_CALLING: + return CALL_STATUS_DIALING; + case STE_CALL_STATUS_CONNECTING: + return CALL_STATUS_ALERTING; + case STE_CALL_STATUS_ACTIVE: + return CALL_STATUS_ACTIVE; + case STE_CALL_STATUS_HOLD: + return CALL_STATUS_HELD; + case STE_CALL_STATUS_WAITING: + return CALL_STATUS_WAITING; + case STE_CALL_STATUS_ALERTING: + return CALL_STATUS_INCOMING; + case STE_CALL_STATUS_BUSY: + return CALL_STATUS_DISCONNECTED; + default: + return CALL_STATUS_DISCONNECTED; + } +} + +static gint call_compare(gconstpointer a, gconstpointer b) +{ + const struct ofono_call *ca = a; + const struct ofono_call *cb = b; + + if (ca->id < cb->id) + return -1; + + if (ca->id > cb->id) + return 1; + + return 0; +} + +static gint call_compare_by_id(gconstpointer a, gconstpointer b) +{ + const struct ofono_call *call = a; + unsigned int id = GPOINTER_TO_UINT(b); + + if (id < call->id) + return -1; + + if (id > call->id) + return 1; + + return 0; +} + +static struct ofono_call *create_call(struct ofono_voicecall *vc, int type, + int direction, int status, + const char *num, int num_type, int clip) +{ + struct voicecall_data *d = ofono_voicecall_get_data(vc); + struct ofono_call *call; + + /* Generate a call structure for the waiting call */ + call = g_try_new0(struct ofono_call, 1); + if (!call) + return NULL; + + call->type = type; + call->direction = direction; + call->status = status; + + if (clip != 2) { + strncpy(call->phone_number.number, num, + OFONO_MAX_PHONE_NUMBER_LENGTH); + call->phone_number.type = num_type; + } + + call->clip_validity = clip; + + d->calls = g_slist_insert_sorted(d->calls, call, call_compare); + + return call; +} + +static void ste_generic_cb(gboolean ok, GAtResult *result, gpointer user_data) +{ + struct change_state_req *req = user_data; + struct voicecall_data *vd = ofono_voicecall_get_data(req->vc); + struct ofono_error error; + + dump_response("ste_generic_cb", ok, result); + decode_at_error(&error, g_at_result_final_response(result)); + + if (ok && req->affected_types) { + GSList *l; + struct ofono_call *call; + + for (l = vd->calls; l; l = l->next) { + call = l->data; + + if (req->affected_types & (0x1 << call->status)) + vd->local_release |= (0x1 << call->id); + } + } + + /* We have to callback after we schedule a poll if required */ + req->cb(&error, req->data); +} + +static void release_id_cb(gboolean ok, GAtResult *result, + gpointer user_data) +{ + struct release_id_req *req = user_data; + struct voicecall_data *vd = ofono_voicecall_get_data(req->vc); + struct ofono_error error; + + dump_response("release_id_cb", ok, result); + decode_at_error(&error, g_at_result_final_response(result)); + + if (ok) + vd->local_release = 0x1 << req->id; + + /* We have to callback after we schedule a poll if required */ + req->cb(&error, req->data); +} + +static void atd_cb(gboolean ok, GAtResult *result, gpointer user_data) +{ + struct cb_data *cbd = user_data; + struct ofono_error error; + ofono_voicecall_cb_t cb = cbd->cb; + + dump_response("atd_cb", ok, result); + + decode_at_error(&error, g_at_result_final_response(result)); + + if (!ok) + goto out; + +out: + cb(&error, cbd->data); +} + +static void ste_dial(struct ofono_voicecall *vc, + const struct ofono_phone_number *ph, + enum ofono_clir_option clir, enum ofono_cug_option cug, + ofono_voicecall_cb_t cb, void *data) +{ + struct voicecall_data *vd = ofono_voicecall_get_data(vc); + struct cb_data *cbd = cb_data_new(cb, data); + char buf[256]; + + if (!cbd) + goto error; + + cbd->user = vc; + + if (ph->type == 145) + sprintf(buf, "ATD+%s", ph->number); + else + sprintf(buf, "ATD%s", ph->number); + + switch (clir) { + case OFONO_CLIR_OPTION_INVOCATION: + strcat(buf, "I"); + break; + case OFONO_CLIR_OPTION_SUPPRESSION: + strcat(buf, "i"); + break; + default: + break; + } + + switch (cug) { + case OFONO_CUG_OPTION_INVOCATION: + strcat(buf, "G"); + break; + default: + break; + } + + strcat(buf, ";"); + + if (g_at_chat_send(vd->chat, buf, none_prefix, + atd_cb, cbd, g_free) > 0) + return; + +error: + if (cbd) + g_free(cbd); + + CALLBACK_WITH_FAILURE(cb, data); +} + +static void ste_template(const char *cmd, struct ofono_voicecall *vc, + GAtResultFunc result_cb, unsigned int affected_types, + ofono_voicecall_cb_t cb, void *data) +{ + struct voicecall_data *vd = ofono_voicecall_get_data(vc); + struct change_state_req *req = g_try_new0(struct change_state_req, 1); + + if (!req) + goto error; + + req->vc = vc; + req->cb = cb; + req->data = data; + req->affected_types = affected_types; + + if (g_at_chat_send(vd->chat, cmd, none_prefix, + result_cb, req, g_free) > 0) + return; + +error: + if (req) + g_free(req); + + CALLBACK_WITH_FAILURE(cb, data); +} + +static void ste_answer(struct ofono_voicecall *vc, + ofono_voicecall_cb_t cb, void *data) +{ + ste_template("ATA", vc, ste_generic_cb, 0, cb, data); +} + +static void ste_hangup(struct ofono_voicecall *vc, + ofono_voicecall_cb_t cb, void *data) +{ + /* Hangup all calls */ + ste_template("AT+CHUP", vc, ste_generic_cb, 0x3f, cb, data); +} + +static void ste_hold_all_active(struct ofono_voicecall *vc, + ofono_voicecall_cb_t cb, void *data) +{ + ste_template("AT+CHLD=2", vc, ste_generic_cb, 0, cb, data); +} + +static void ste_release_all_held(struct ofono_voicecall *vc, + ofono_voicecall_cb_t cb, void *data) +{ + unsigned int held_status = 0x1 << 1; + ste_template("AT+CHLD=0", vc, ste_generic_cb, held_status, cb, data); +} + +static void ste_set_udub(struct ofono_voicecall *vc, + ofono_voicecall_cb_t cb, void *data) +{ + unsigned int incoming_or_waiting = (0x1 << 4) | (0x1 << 5); + ste_template("AT+CHLD=0", vc, ste_generic_cb, incoming_or_waiting, + cb, data); +} + +static void ste_release_all_active(struct ofono_voicecall *vc, + ofono_voicecall_cb_t cb, void *data) +{ + ste_template("AT+CHLD=1", vc, ste_generic_cb, 0x1, cb, data); +} + +static void ste_release_specific(struct ofono_voicecall *vc, int id, + ofono_voicecall_cb_t cb, void *data) +{ + struct voicecall_data *vd = ofono_voicecall_get_data(vc); + struct release_id_req *req = g_try_new0(struct release_id_req, 1); + char buf[32]; + struct ofono_call *call; + GSList *l; + int ac = 0; + + if (!req) + goto error; + + req->vc = vc; + req->cb = cb; + req->data = data; + req->id = id; + + /* Count active calls. If there is more than one active call + * we cannot use ATH, as it will terminate all calls. + * The reason for using ATH and not CHLD is that + * emergency calls can not be terminated with AT+CHLD. + */ + for (l = vd->calls; l; l = l->next) { + call = l->data; + + if (call->status == CALL_STATUS_ACTIVE) + ac++; + } + + l = g_slist_find_custom(vd->calls, GUINT_TO_POINTER(id), + call_compare_by_id); + if (l == NULL) { + ofono_error("Hangup request for unknow call"); + goto error; + } + call = l->data; + /* Check the state of the call, as AT+CHLD does not terminate + * calls in state Dialing, Alerting and Incoming */ + if (call->status == CALL_STATUS_DIALING || + call->status == CALL_STATUS_ALERTING || + call->status == CALL_STATUS_INCOMING) + sprintf(buf, "ATH"); + + /* Waiting call can not be terminated by at+chld=1x, + * have to use at+chld = 0, but that will also terminate + * other held calls. Bug in STE's AT module. + */ + else if (call->status == CALL_STATUS_WAITING) + sprintf(buf, "AT+CHLD=0"); + + else { + /* A held call can not be released by ATH, need to use CHLD */ + if (ac > 1 || call->status == CALL_STATUS_HELD) + sprintf(buf, "AT+CHLD=1%d", id); + else /* This is the last call, ok to use ATH. */ + sprintf(buf, "ATH"); + } + + if (g_at_chat_send(vd->chat, buf, none_prefix, + release_id_cb, req, g_free) > 0) + return; + +error: + if (req) + g_free(req); + + CALLBACK_WITH_FAILURE(cb, data); +} + +static void ste_private_chat(struct ofono_voicecall *vc, int id, + ofono_voicecall_cb_t cb, void *data) +{ + char buf[32]; + + sprintf(buf, "AT+CHLD=2%d", id); + ste_template(buf, vc, ste_generic_cb, 0, cb, data); +} + +static void ste_create_multiparty(struct ofono_voicecall *vc, + ofono_voicecall_cb_t cb, void *data) +{ + ste_template("AT+CHLD=3", vc, ste_generic_cb, 0, cb, data); +} + +static void ste_transfer(struct ofono_voicecall *vc, + ofono_voicecall_cb_t cb, void *data) +{ + /* Held & Active */ + unsigned int transfer = 0x1 | 0x2; + + /* Transfer can puts held & active calls together and disconnects + * from both. However, some networks support transfering of + * dialing/ringing calls as well. + */ + transfer |= 0x4 | 0x8; + + ste_template("AT+CHLD=4", vc, ste_generic_cb, transfer, cb, data); +} + +static void ste_deflect(struct ofono_voicecall *vc, + const struct ofono_phone_number *ph, + ofono_voicecall_cb_t cb, void *data) +{ + char buf[128]; + unsigned int incoming_or_waiting = (0x1 << 4) | (0x1 << 5); + + sprintf(buf, "AT+CTFR=\"%s\",%d", ph->number, ph->type); + ste_template(buf, vc, ste_generic_cb, incoming_or_waiting, cb, data); +} + +static void ecav_notify(GAtResult *result, gpointer user_data) +{ + struct ofono_voicecall *vc = user_data; + struct voicecall_data *vd = ofono_voicecall_get_data(vc); + GAtResultIter iter; + const char *num; + int id; + int status; + int call_type; + int num_type; + struct ofono_call *new_call; + struct ofono_call *existing_call = NULL; + GSList *l; + + /* Parse ECAV */ + dump_response("ecav_notify", TRUE, result); + + g_at_result_iter_init(&iter, result); + + if (!g_at_result_iter_next(&iter, "*ECAV:")) + return; + + if (!g_at_result_iter_next_number(&iter, &id)) + return; + + if (!g_at_result_iter_next_number(&iter, &status)) + return; + + if (!g_at_result_iter_next_number(&iter, &call_type)) + return; + + /* Skip process id and exit cause */ + g_at_result_iter_skip_next(&iter); + g_at_result_iter_skip_next(&iter); + + status = call_status_ste_to_ofono(status); + if (status == CALL_STATUS_DIALING || + status == CALL_STATUS_WAITING || + status == CALL_STATUS_INCOMING) { + if (!g_at_result_iter_next_string(&iter, &num)) + return; + + if (!g_at_result_iter_next_number(&iter, &num_type)) + return; + } + + /* Handle the call according to the status. + * If it doesn't exists we make a new one + */ + l = g_slist_find_custom(vd->calls, GUINT_TO_POINTER(id), + call_compare_by_id); + if (l) + existing_call = l->data; + + if (l == NULL && status != CALL_STATUS_DIALING && + status != CALL_STATUS_WAITING && + status != CALL_STATUS_INCOMING) { + ofono_error("ECAV notification for unknow call. " + "id: %d, status: %d", id, status); + return; + } + + switch (status) { + case CALL_STATUS_DISCONNECTED: { + enum ofono_disconnect_reason reason; + existing_call->status = status; + + if (vd->local_release & (0x1 << existing_call->id)) + reason = OFONO_DISCONNECT_REASON_LOCAL_HANGUP; + else + reason = OFONO_DISCONNECT_REASON_REMOTE_HANGUP; + + if (call_type == BEARER_CLASS_VOICE) + ofono_voicecall_disconnected(vc, + existing_call->id, reason, NULL); + vd->calls = g_slist_remove(vd->calls, l->data); + break; + } + case CALL_STATUS_DIALING: + case CALL_STATUS_WAITING: + case CALL_STATUS_INCOMING: { + int clip_validity; + int direction; + + if (status == CALL_STATUS_DIALING) + direction = CALL_DIRECTION_MOBILE_ORIGINATED; + else + direction = CALL_DIRECTION_MOBILE_TERMINATED; + + if ((strlen(num)) > 0) + clip_validity = CLIP_VALIDITY_VALID; + else + clip_validity = CLIP_VALIDITY_NOT_AVAILABLE; + + new_call = create_call(vc, call_type, direction, + status, num, num_type, + clip_validity); + + if (!new_call) { + ofono_error("Unable to malloc." + "Call management is fubar"); + return; + } + new_call->id = id; + + if (call_type == BEARER_CLASS_VOICE) + ofono_voicecall_notify(vc, new_call); + break; + } + case CALL_STATUS_ALERTING: + case CALL_STATUS_ACTIVE: + case CALL_STATUS_HELD: + existing_call->status = status; + + if (call_type == BEARER_CLASS_VOICE) + ofono_voicecall_notify(vc, existing_call); + break; + default: + break; + } +} + +static int ste_voicecall_probe(struct ofono_voicecall *vc, unsigned int vendor, + void *data) +{ + GAtChat *chat = data; + struct voicecall_data *vd; + + vd = g_new0(struct voicecall_data, 1); + vd->chat = chat; + + ofono_voicecall_set_data(vc, vd); + + g_at_chat_send(chat, "AT*ECAM=1", NULL, NULL, NULL, NULL); + g_at_chat_register(chat, "*ECAV:", ecav_notify, FALSE, vc, NULL); + ofono_voicecall_register(vc); + + return 0; +} + +static struct ofono_voicecall_driver driver = { + .name = "stemodem", + .probe = ste_voicecall_probe, + .remove = NULL, + .dial = ste_dial, + .answer = ste_answer, + .hangup = ste_hangup, + .hold_all_active = ste_hold_all_active, + .release_all_held = ste_release_all_held, + .set_udub = ste_set_udub, + .release_all_active = ste_release_all_active, + .release_specific = ste_release_specific, + .private_chat = ste_private_chat, + .create_multiparty = ste_create_multiparty, + .transfer = ste_transfer, + .deflect = ste_deflect, + .swap_without_accept = NULL, + .send_tones = NULL +}; + +void ste_voicecall_init() +{ + ofono_voicecall_driver_register(&driver); +} + +void ste_voicecall_exit() +{ + ofono_voicecall_driver_unregister(&driver); +} diff --git a/plugins/ste.c b/plugins/ste.c new file mode 100644 index 0000000..72477a5 --- /dev/null +++ b/plugins/ste.c @@ -0,0 +1,246 @@ +/* + * + * oFono - Open Source Telephony + * + * Copyright (C) 2008-2009 Intel Corporation. All rights reserved. + * Copyright (C) 2010 ST-Ericsson AB. + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. + * + * 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 <glib.h> +#include <gatchat.h> +#include <gattty.h> + +#define OFONO_API_SUBJECT_TO_CHANGE +#include <ofono/plugin.h> +#include <ofono/log.h> +#include <ofono/modem.h> +#include <ofono/call-barring.h> +#include <ofono/call-forwarding.h> +#include <ofono/call-meter.h> +#include <ofono/call-settings.h> +#include <ofono/devinfo.h> +#include <ofono/message-waiting.h> +#include <ofono/netreg.h> +#include <ofono/phonebook.h> +#include <ofono/sim.h> +#include <ofono/sms.h> +#include <ofono/ssn.h> +#include <ofono/ussd.h> +#include <ofono/call-volume.h> +#include <ofono/voicecall.h> +#include <drivers/atmodem/vendor.h> +#include <ofono/gprs.h> +#include <ofono/gprs-context.h> +#include <linux/caif/caif_socket.h> +#include <unistd.h> + +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; + 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); + + /* Connect to the AT Service at the modem */ + connect(fd, (struct sockaddr *) &addr, sizeof(addr)); + 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_gsmv1(); + + 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, "AT+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(data->chat); + g_at_chat_send(data->chat, "AT+CFUN=4", NULL, + cfun_disable, modem, NULL); + + return -EINPROGRESS; +} + +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); +} + +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, 0, "stemodem", 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); + + if (gprs && gc) + ofono_gprs_add_context(gprs, gc); + + mw = ofono_message_waiting_create(modem); + + if (mw) + ofono_message_waiting_register(mw); +} + +static struct ofono_modem_driver ste_driver = { + .name = "ste", + .probe = ste_probe, + .remove = ste_remove, + .enable = ste_enable, + .disable = ste_disable, + .pre_sim = ste_pre_sim, + .post_sim = ste_post_sim, +}; + +static int ste_init(void) +{ + return ofono_modem_driver_register(&ste_driver); +} + +static void ste_exit(void) +{ + ofono_modem_driver_unregister(&ste_driver); +} + +OFONO_PLUGIN_DEFINE(ste, "ST-Ericsson modem driver", VERSION, + OFONO_PLUGIN_PRIORITY_DEFAULT, ste_init, ste_exit) -- 1.6.0.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC 3/3] STE-plugin: Adding STE plugin 2010-01-17 17:28 ` [RFC 3/3] STE-plugin: Adding STE plugin sjur.brandeland @ 2010-01-17 21:40 ` Marcel Holtmann 2010-01-18 18:22 ` Sjur =?unknown-8bit?q?Br=C3=A6ndeland?= 2010-01-17 22:50 ` Denis Kenzior 1 sibling, 1 reply; 10+ messages in thread From: Marcel Holtmann @ 2010-01-17 21:40 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 9401 bytes --] Hi Sjur, > Added implementation for STE modem; STE modem driver, and STE specific > drivers for gprs, network registration and voice call. > > This patch uses CAIF sockets. CAIF patch for net-next-2.6 will be > contributed on netdev(a)vger.kernel.org soon. can you please split these into smaller patches so they are easier to review. I am thinking of something the like this; one per network registration, one per voice call, one per GPRS. You are making some core changes and we need to have a close look at them and what the potential impact would be. This is not a complete review, but I making some comments on obvious things. > index 0000000..1d5d8db > --- /dev/null > +++ b/drivers/stemodem/gprs-context.c > @@ -0,0 +1,612 @@ > +/* > + * > + * oFono - Open Source Telephony > + * > + * Copyright (C) 2008-2009 Intel Corporation. All rights reserved. > + * Copyright (C) 2010 ST-Ericsson AB. > + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. As mentioned before, we track authorship via GIT. So ensure that the GIT commits are properly done. > +#include <linux/types.h> > +#include <net/if.h> > +#include <sys/ioctl.h> > +#include <arpa/inet.h> > +#include <linux/caif/caif_socket.h> > +#include <linux/caif/if_caif.h> This is dangerous until the CAIF subsystem is actually merged and present everywhere. Please consider adding an option to enable stemodem driver (like we do for atmodem and isimodem). It might make sense to have a local copy of the required structure and constants to allow an easier complication. Of course this depends on having CAIF at least in net-next-2.6 tree. > +/* TODO: should parse_xml function to be moved to e.g. atutil? */ First question. Why not use the XML parse that comes with GLib. > +static char *parse_xml(char * xml, char* tag) > +{ Please use consistent coding style. It is "char *xml". And it is always like this. No extra space after * or char*. > + char *begin; > + char *end; > + int len; > + char *res = NULL; > + char *start = (char *)g_malloc(strlen(tag)+3); > + char *stop = (char *)g_malloc(strlen(tag)+4); No casting of malloc function. It is not needed. And extra spaces between operation. Meaning malloc(strlen(tag) + 3). This applies to all code. > + if (create) { > + if (ioctl(s, SIOCCAIFNETNEW, &ifr) < 0) { > + ofono_debug("Failed to create IP interface for CAIF"); > + goto error; > + } > + > + s = socket(PF_INET, SOCK_DGRAM, 0); > + if (s < 0) { > + ofono_debug("Failed to create socket."); > + goto error; > + } > + > + /* Set IP address */ > + memset(&sin, 0, sizeof(struct sockaddr)); > + sin.sin_family = AF_INET; > + > + if (inet_pton(AF_INET, ip, &sin.sin_addr) <= 0) { > + ofono_debug("inet_pton failed, will not be" > + "able to set the IP address"); > + goto error; > + } > + memcpy(&ifr.ifr_addr, &sin, sizeof(struct sockaddr)); > + > + if (ioctl(s, SIOCSIFADDR, &ifr) < 0) { > + ofono_debug("Failed to set IP address for" > + " interface: %s", ifr.ifr_name); > + goto error; > + } oFono is never setting the IP address, netmask or any other configuration option. The only thing that oFono does is bringing the interface up. Systems like ConnMan do the IP configuration. Please see the comments in the documentation on how we expose IP interfaces. Check with ConnMan and how we configure them. > + if (ioctl(s, SIOCSIFMTU, &ifr) < 0) > + ofono_debug("Failed to set MTU for interface: %s", > + ifr.ifr_name); > + } else { > + if (ioctl(s, SIOCGIFINDEX, &ifr) == 0) { > + if (ioctl(s, SIOCCAIFNETREMOVE, &ifr) < 0) { > + ofono_debug("Failed to remove IP interface" > + "for CAIF"); > + goto error; > + } > + } else { > + ofono_debug("Did not find interface (%s) to remove", > + interface); > + goto error; > + } > + } Having create and remove in the same function seems hard to read. Why not have one for creating the interface and one for removing it. From what I see so far, it is not much more code. And makes it a lot easier to read and understand for other people. > + g_at_result_iter_init(&iter, result); > + for (i = 0; i < g_at_result_num_response_lines(result); i++) { > + g_at_result_iter_next(&iter, NULL); > + res_string = strdup(g_at_result_iter_raw_line(&iter)); > + > + if (strstr(res_string, "ip_address")) { > + ip = g_strdup(parse_xml(res_string, > + "ip_address")); > + } else if ((strstr(res_string, "subnet_mask"))) { > + netmask = g_strdup(parse_xml(res_string, > + "subnet_mask")); > + } else if ((strstr(res_string, "mtu"))) { > + mtu = g_strdup(parse_xml(res_string, > + "mtu")); > + } else if ((strstr(res_string, "default_gateway"))) { > + gateway = g_strdup(parse_xml(res_string, > + "default_gateway")); > + } else if ((strstr(res_string, "dns_server"))) { > + str = g_strdup(parse_xml(res_string, > + "dns_server")); > + > + if (numdns < MAX_DNS) > + dns[numdns++] = str; > + } I would prefer if you do the parsing in one function that goes through the XML ones. Just give it pointers to the fields you wanna have it extract or a special combined structure. This looks highly inefficient to me. > + conn->interface = g_malloc(10); > + if (!conn->interface) > + goto error; Please double check with the GLib documentation on memory allocation. The g_malloc failure would result in halt of the program. What you want here is g_try_malloc. Also for 10 bytes, please don't bother with an allocation. Just include a char array in the conn structure. > + > + sprintf(conn->interface, "caif%u", conn->device); > + > + if (!caif_if_create_remove(conn->interface, ip, > + mtu, netmask, conn->channel_id, TRUE)) { > + ofono_error("Failed to create caif interface %s.", > + conn->interface); > + conn->interface = NULL; > + } Yeah, I really prefer caif_if_create() and caif_if_remove(). This is not really intuitive at all. > + /* Need to change to gsm_permissive syntax in order to > + * parse the response from EPPSD (xml) */ > + g_at_chat_set_syntax(gcd->chat, g_at_syntax_new_gsm_permissive()); Is this an issue with your modem firmware or an issue in the v1 parser. If it is the modem firmware, then just use the permissive parser all the time and switch to E0. > +static void cgev_notify(GAtResult *result, gpointer user_data) > +{ > + struct ofono_gprs_context *gc = user_data; > + struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc); > + GAtResultIter iter; > + const char *event; > + > + dump_response("cgev_notify", TRUE, result); > + > + g_at_result_iter_init(&iter, result); > + > + if (!g_at_result_iter_next(&iter, "+CGEV:")) > + return; > + > + if (!g_at_result_iter_next_unquoted_string(&iter, &event)) > + return; > + > + if (g_str_has_prefix(event, "NW REACT ") || > + g_str_has_prefix(event, "NW DEACT ") || > + g_str_has_prefix(event, "ME DEACT ")) { > + /* Ask what primary contexts are active now */ > + > + g_at_chat_send(gcd->chat, "AT+CGACT?", cgact_prefix, > + ste_cgact_read_cb, gc, NULL); > + > + return; > + } > +} The return statement in the if clause is kinda pointless. Seems like either your code tried to be more complex or you are missing something. > +static int stemodem_init(void) > +{ > + /* Initialize voicecall driver */ > + struct ofono_voicecall_driver *at_vcdrv; > + struct ofono_voicecall_driver *ste_vcdrv; > + struct ofono_netreg_driver *at_netdrv; > + struct ofono_netreg_driver *ste_netdrv; > + > + ste_voicecall_init(); > + > + at_vcdrv = ofono_voicecall_driver_get("atmodem"); > + ste_vcdrv = ofono_voicecall_driver_get("stemodem"); > + > + if (at_vcdrv && ste_vcdrv) { > + ste_vcdrv->remove = at_vcdrv->remove; > + ste_vcdrv->swap_without_accept = at_vcdrv->swap_without_accept; > + ste_vcdrv->send_tones = at_vcdrv->send_tones; > + } else { > + if (!ste_vcdrv) > + ofono_debug("Could not get ofono_voicecall_driver" > + "from stemodem"); > + if (!at_vcdrv) > + ofono_debug("Could not get ofono_voicecall_driver" > + "from atmodem"); > + } > + > + /* Initialize netreg driver */ > + ste_netreg_init(); > + > + at_netdrv = ofono_netreg_driver_get("atmodem"); > + ste_netdrv = ofono_netreg_driver_get("stemodem"); > + > + if (at_netdrv && ste_netdrv) { > + ste_netdrv->remove = at_netdrv->remove; > + ste_netdrv->registration_status = > + at_netdrv->registration_status; > + ste_netdrv->current_operator = at_netdrv->current_operator; > + ste_netdrv->list_operators = at_netdrv->list_operators; > + ste_netdrv->register_auto = at_netdrv->register_auto; > + ste_netdrv->register_manual = at_netdrv->register_manual; So far this, I really like to see a description on what's the difference in the network registration and voice call atoms. We do need to understand what is actually needed. > + /* Create a CAIF socket for AT Service */ > + fd = socket(AF_CAIF, SOCK_SEQPACKET, CAIFPROTO_AT); > + > + /* Connect to the AT Service at the modem */ > + connect(fd, (struct sockaddr *) &addr, sizeof(addr)); > + channel = g_io_channel_unix_new(fd); You need to check the results of socket() and connect() calls. Regards Marcel ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [RFC 3/3] STE-plugin: Adding STE plugin 2010-01-17 21:40 ` Marcel Holtmann @ 2010-01-18 18:22 ` Sjur =?unknown-8bit?q?Br=C3=A6ndeland?= 2010-01-18 21:27 ` Marcel Holtmann 0 siblings, 1 reply; 10+ messages in thread From: Sjur =?unknown-8bit?q?Br=C3=A6ndeland?= @ 2010-01-18 18:22 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 7711 bytes --] Hi Marcel. Thank you for your feedback. We hope to get new patch-set out tomorrow with most of your comments fixed. Marcel Holtmann wrote: > Hi Sjur, > > review. I am thinking of something the like this; one per network > registration, one per voice call, one per GPRS. > > You are making some core changes and we need to have a close look at > them and what the potential impact would be. Sure, will split up better next time. >> +#include <arpa/inet.h> >> +#include <linux/caif/caif_socket.h> >> +#include <linux/caif/if_caif.h> > > This is dangerous until the CAIF subsystem is actually merged and > present everywhere. Please consider adding an option to enable > stemodem driver (like we do for atmodem and isimodem). OK, we'll put this on our todo-list. > It might make sense to have a local copy of the required structure > and constants to allow an easier complication. Of course this depends > on having CAIF at least in net-next-2.6 tree. OK, agree. I'll supply the CAIF specific patches next time. Should we put the CAIF header files into ofono/include/caif? > >> +/* TODO: should parse_xml function to be moved to e.g. atutil? */ > > First question. Why not use the XML parse that comes with GLib. OK. Is it "Simple XML Subset Parser" you refer to? If you have any pointer to sample code using this XML parser we would appreciate it. > oFono is never setting the IP address, netmask or any other > configuration option. The only thing that oFono does is bringing the > interface up. Systems like ConnMan do the IP configuration. > > Please see the comments in the documentation on how we expose IP > interfaces. Check with ConnMan and how we configure them. OK, we're returning the relevant parameters from activate_primary so that all PDP Context parameters including interface name, ip-address, netmask, dns, etc are available in PrimaryDataContext. And leave the interface created but not configured. Does this sound ok? > > Having create and remove in the same function seems hard to read. Why > not have one for creating the interface and one for removing it. From > what I see so far, it is not much more code. And makes it a lot > easier to read and understand for other people. > OK, we'll split this up. > I would prefer if you do the parsing in one function that goes > through the XML ones. Just give it pointers to the fields you wanna > have it extract or a special combined structure. This looks highly > inefficient to me. OK, see comments on XML above. > >> + conn->interface = g_malloc(10); >> + if (!conn->interface) >> + goto error; > > Please double check with the GLib documentation on memory allocation. > The g_malloc failure would result in halt of the program. What you > want here is g_try_malloc. > > Also for 10 bytes, please don't bother with an allocation. Just > include a char array in the conn structure. Yes I agree, we will fix this. > > Yeah, I really prefer caif_if_create() and caif_if_remove(). This is > not really intuitive at all. OK, sure. > >> + /* Need to change to gsm_permissive syntax in order to >> + * parse the response from EPPSD (xml) */ >> + g_at_chat_set_syntax(gcd->chat, g_at_syntax_new_gsm_permissive()); > > Is this an issue with your modem firmware or an issue in the v1 > parser. > If it is the modem firmware, then just use the permissive parser all > the time and switch to E0. Setting permissive mode was done in order to be able to parse the XML response from EPPSD (Propriatery Activate PDP context). But we can try if we can run with this mode default if you think this is better. > >> +static void cgev_notify(GAtResult *result, gpointer user_data) { >> + struct ofono_gprs_context *gc = user_data; >> + struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc); >> + GAtResultIter iter; + const char *event; >> + >> + dump_response("cgev_notify", TRUE, result); >> + >> + g_at_result_iter_init(&iter, result); >> + >> + if (!g_at_result_iter_next(&iter, "+CGEV:")) >> + return; >> + >> + if (!g_at_result_iter_next_unquoted_string(&iter, &event)) >> + return; + >> + if (g_str_has_prefix(event, "NW REACT ") || >> + g_str_has_prefix(event, "NW DEACT ") || >> + g_str_has_prefix(event, "ME DEACT ")) { >> + /* Ask what primary contexts are active now */ >> + >> + g_at_chat_send(gcd->chat, "AT+CGACT?", cgact_prefix, >> + ste_cgact_read_cb, gc, NULL); >> + >> + return; >> + } >> +} > > The return statement in the if clause is kinda pointless. Seems like > either your code tried to be more complex or you are missing > something. Sure we can fix this, but this is actually just copied from gprs_context in "atmodem". BTW, the iter_init seems to be missing in atmodem's implementation, this is probably a bug in "atmodem". > >> +static int stemodem_init(void) >> +{ >> + /* Initialize voicecall driver */ >> + struct ofono_voicecall_driver *at_vcdrv; >> + struct ofono_voicecall_driver *ste_vcdrv; >> + struct ofono_netreg_driver *at_netdrv; >> + struct ofono_netreg_driver *ste_netdrv; >> + >> + ste_voicecall_init(); >> + >> + at_vcdrv = ofono_voicecall_driver_get("atmodem"); >> + ste_vcdrv = ofono_voicecall_driver_get("stemodem"); + >> + if (at_vcdrv && ste_vcdrv) { >> + ste_vcdrv->remove = at_vcdrv->remove; >> + ste_vcdrv->swap_without_accept = at_vcdrv->swap_without_accept; >> + ste_vcdrv->send_tones = at_vcdrv->send_tones; >> + } else { >> + if (!ste_vcdrv) >> + ofono_debug("Could not get ofono_voicecall_driver" + "from >> stemodem"); + if (!at_vcdrv) >> + ofono_debug("Could not get ofono_voicecall_driver" + "from >> atmodem"); + } >> + >> + /* Initialize netreg driver */ >> + ste_netreg_init(); >> + >> + at_netdrv = ofono_netreg_driver_get("atmodem"); >> + ste_netdrv = ofono_netreg_driver_get("stemodem"); + >> + if (at_netdrv && ste_netdrv) { >> + ste_netdrv->remove = at_netdrv->remove; >> + ste_netdrv->registration_status = >> + at_netdrv->registration_status; >> + ste_netdrv->current_operator = at_netdrv->current_operator; >> + ste_netdrv->list_operators = at_netdrv->list_operators; >> + ste_netdrv->register_auto = at_netdrv->register_auto; >> + ste_netdrv->register_manual = at_netdrv->register_manual; > > So far this, I really like to see a description on what's the > difference in the network registration and voice call atoms. > > We do need to understand what is actually needed. > For Voice call the main difference is that we're not doing polling with CLCC, but are using our proprietary call events. This impacts most functions. The functions reusable from "atmodem" are "remove", "swap_without_accept", and "send_tones". I guess we could copy the three common functions from "atmodem" into "stemodem" voicecall.c if you prefer this. For Network Registration we have almost identical implementation, the difference is that for signal strength reporting we are using "AT+CIND" instead of "CSQ" and "CIEV". "CSQ" is not working for WCDMA in our case. I see that in the initialization function there is a switch on vendor. I guess we could go for this approach if you think this is better. > >> + /* Create a CAIF socket for AT Service */ >> + fd = socket(AF_CAIF, SOCK_SEQPACKET, CAIFPROTO_AT); + >> + /* Connect to the AT Service at the modem */ >> + connect(fd, (struct sockaddr *) &addr, sizeof(addr)); >> + channel = g_io_channel_unix_new(fd); > > You need to check the results of socket() and connect() calls. Sure, will fix this. Regards Sjur Brændeland ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [RFC 3/3] STE-plugin: Adding STE plugin 2010-01-18 18:22 ` Sjur =?unknown-8bit?q?Br=C3=A6ndeland?= @ 2010-01-18 21:27 ` Marcel Holtmann 2010-01-20 18:24 ` Sjur =?unknown-8bit?q?Br=C3=A6ndeland?= 0 siblings, 1 reply; 10+ messages in thread From: Marcel Holtmann @ 2010-01-18 21:27 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 8306 bytes --] Hi Sjur, > Thank you for your feedback. We hope to get new patch-set out tomorrow > with most of your comments fixed. sounds great. > > It might make sense to have a local copy of the required structure > > and constants to allow an easier complication. Of course this depends > > on having CAIF at least in net-next-2.6 tree. > > OK, agree. I'll supply the CAIF specific patches next time. > Should we put the CAIF header files into ofono/include/caif? Have a look at how we do it for the Phonet/ISI modem. Also we only need a few constants (AF_CAIF, proto etc.) and mainly only the socket structure for CAIF. This should be easy to fix actually. More important is that we can disable stemodem support via configure so people with an old kernel can compile it. We are in a chicken and egg problem with the AF_CAIF constant anyway. Until the CAIF subsystem is in net-next-2.6, the actually number for it is not assigned. > >> +/* TODO: should parse_xml function to be moved to e.g. atutil? */ > > > > First question. Why not use the XML parse that comes with GLib. > > OK. Is it "Simple XML Subset Parser" you refer to? If you have any pointer > to sample code using this XML parser we would appreciate it. Yes, I am talking about that one. It is similar to expact and the only example, I have is in the BlueZ source code. I think Google code search will reveal more useful examples. > > oFono is never setting the IP address, netmask or any other > > configuration option. The only thing that oFono does is bringing the > > interface up. Systems like ConnMan do the IP configuration. > > > > Please see the comments in the documentation on how we expose IP > > interfaces. Check with ConnMan and how we configure them. > > OK, we're returning the relevant parameters from activate_primary so that > all PDP Context parameters including interface name, > ip-address, netmask, dns, etc are available in PrimaryDataContext. > And leave the interface created but not configured. > Does this sound ok? Yes, sounds good. Please have a look at how we do it for Ericsson MBM and Option HSO modems. We just send the D-Bus signal with all the details and that is it. I forgot to mention the reasoning behind. oFono is incapable of making any kind of good decision when it comes to handling IP collision since it only knows about its scope. Systems like ConnMan have the full insight in whole networking to make proper decisions. > > > >> + /* Need to change to gsm_permissive syntax in order to > >> + * parse the response from EPPSD (xml) */ > >> + g_at_chat_set_syntax(gcd->chat, g_at_syntax_new_gsm_permissive()); > > > > Is this an issue with your modem firmware or an issue in the v1 > > parser. > > If it is the modem firmware, then just use the permissive parser all > > the time and switch to E0. > > Setting permissive mode was done in order to be able to parse > the XML response from EPPSD (Propriatery Activate PDP context). > But we can try if we can run with this mode default if you think > this is better. Can you post an example (manually via cu or minicom and with ATE1) on how this looks like. My question here is if you are actually following the strict v1 syntax. If you don't then that is basically a bug on the modem side. I don't even know if v1 has any kind of capabilities to handle XML properly in the first place. The point here is if you can not use v1 parser, then just use permissive from the beginning and avoid switching at runtime. The capability of switching at runtime mainly only exists for testing purposes. With the permissive parser you just have to ensure ATE0 inside the modem plugin. See the other users for an example. > >> +static void cgev_notify(GAtResult *result, gpointer user_data) { > >> + struct ofono_gprs_context *gc = user_data; > >> + struct gprs_context_data *gcd = ofono_gprs_context_get_data(gc); > >> + GAtResultIter iter; + const char *event; > >> + > >> + dump_response("cgev_notify", TRUE, result); > >> + > >> + g_at_result_iter_init(&iter, result); > >> + > >> + if (!g_at_result_iter_next(&iter, "+CGEV:")) > >> + return; > >> + > >> + if (!g_at_result_iter_next_unquoted_string(&iter, &event)) > >> + return; + > >> + if (g_str_has_prefix(event, "NW REACT ") || > >> + g_str_has_prefix(event, "NW DEACT ") || > >> + g_str_has_prefix(event, "ME DEACT ")) { > >> + /* Ask what primary contexts are active now */ > >> + > >> + g_at_chat_send(gcd->chat, "AT+CGACT?", cgact_prefix, > >> + ste_cgact_read_cb, gc, NULL); > >> + > >> + return; > >> + } > >> +} > > > > The return statement in the if clause is kinda pointless. Seems like > > either your code tried to be more complex or you are missing > > something. > > Sure we can fix this, but this is actually just copied from gprs_context in "atmodem". > BTW, the iter_init seems to be missing in atmodem's implementation, this is probably > a bug in "atmodem". Sounds like a bug in atmodem then. Can you point me to the lines where you are seeing this. I will fix it if needed. And btw. the { for functions belongs on the line after. I will do a thorough review of your next patchset and look out for these :) > >> +static int stemodem_init(void) > >> +{ > >> + /* Initialize voicecall driver */ > >> + struct ofono_voicecall_driver *at_vcdrv; > >> + struct ofono_voicecall_driver *ste_vcdrv; > >> + struct ofono_netreg_driver *at_netdrv; > >> + struct ofono_netreg_driver *ste_netdrv; > >> + > >> + ste_voicecall_init(); > >> + > >> + at_vcdrv = ofono_voicecall_driver_get("atmodem"); > >> + ste_vcdrv = ofono_voicecall_driver_get("stemodem"); + > >> + if (at_vcdrv && ste_vcdrv) { > >> + ste_vcdrv->remove = at_vcdrv->remove; > >> + ste_vcdrv->swap_without_accept = at_vcdrv->swap_without_accept; > >> + ste_vcdrv->send_tones = at_vcdrv->send_tones; > >> + } else { > >> + if (!ste_vcdrv) > >> + ofono_debug("Could not get ofono_voicecall_driver" + "from > >> stemodem"); + if (!at_vcdrv) > >> + ofono_debug("Could not get ofono_voicecall_driver" + "from > >> atmodem"); + } > >> + > >> + /* Initialize netreg driver */ > >> + ste_netreg_init(); > >> + > >> + at_netdrv = ofono_netreg_driver_get("atmodem"); > >> + ste_netdrv = ofono_netreg_driver_get("stemodem"); + > >> + if (at_netdrv && ste_netdrv) { > >> + ste_netdrv->remove = at_netdrv->remove; > >> + ste_netdrv->registration_status = > >> + at_netdrv->registration_status; > >> + ste_netdrv->current_operator = at_netdrv->current_operator; > >> + ste_netdrv->list_operators = at_netdrv->list_operators; > >> + ste_netdrv->register_auto = at_netdrv->register_auto; > >> + ste_netdrv->register_manual = at_netdrv->register_manual; > > > > So far this, I really like to see a description on what's the > > difference in the network registration and voice call atoms. > > > > We do need to understand what is actually needed. > > > For Voice call the main difference is that we're not doing polling with > CLCC, but are using our proprietary call events. This impacts most functions. > The functions reusable from "atmodem" are "remove", "swap_without_accept", and > "send_tones". I guess we could copy the three common functions from "atmodem" into > "stemodem" voicecall.c if you prefer this. I actually prefer if you copy them. If useful, we can create some common helpers in drivers/atmodem/atutil.c. I leave this up to Denis to comment on that. > For Network Registration we have almost identical implementation, the difference > is that for signal strength reporting we are using "AT+CIND" instead of "CSQ" and "CIEV". > "CSQ" is not working for WCDMA in our case. > I see that in the initialization function there is a switch on vendor. > I guess we could go for this approach if you think this is better. We do exactly that for OFONO_VENDOR_CALYPSO and OFONO_VENDOR_OPTION_HSO and so I think it is best that you guys follow that approach for now. If this gets massively convoluted at some point, we will be thinking of something. Right now I prefer to just follow exactly what we are doing for other modems with the same problem. Regards Marcel ^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [RFC 3/3] STE-plugin: Adding STE plugin 2010-01-18 21:27 ` Marcel Holtmann @ 2010-01-20 18:24 ` Sjur =?unknown-8bit?q?Br=C3=A6ndeland?= 0 siblings, 0 replies; 10+ messages in thread From: Sjur =?unknown-8bit?q?Br=C3=A6ndeland?= @ 2010-01-20 18:24 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 3174 bytes --] Hi Marcel. Thank you for lots of feedback on patches. Marit who has done most of the implementation is gone for a few days so we'll have to wait until end of next week with the next oFono patch-set. Marcel Holtmann wrote: >>>> + /* Need to change to gsm_permissive syntax in order to >>>> + * parse the response from EPPSD (xml) */ >>>> + g_at_chat_set_syntax(gcd->chat, >>>> +g_at_syntax_new_gsm_permissive()); >>> >>> Is this an issue with your modem firmware or an issue in the v1 >>> parser. If it is the modem firmware, then just use the permissive >>> parser all the time and switch to E0. >> >> Setting permissive mode was done in order to be able to parse the XML >> response from EPPSD (Propriatery Activate PDP context). >> But we can try if we can run with this mode default if you think this >> is better. > > Can you post an example (manually via cu or minicom and with ATE1) on > how this looks like. My question here is if you are actually > following the strict v1 syntax. If you don't then that is basically a > bug on the modem side. I don't even know if v1 has any kind of > capabilities to handle XML properly in the first place. > > The point here is if you can not use v1 parser, then just use > permissive from the beginning and avoid switching at runtime. The > capability of switching at runtime mainly only exists for testing > purposes. With the permissive parser you just have to ensure ATE0 > inside the modem plugin. > See the other users for an example. Here is an example of the output from AT*EPPSD. ofonod[26268]: < \r\nOK\r\n ofonod[26268]: > AT*EPPSD=1,1,1\r ofonod[26268]: < AT*EPPSD=1,1,1\r ofonod[26268]: < \r\n<?xml version="1.0"?>\r\n ofonod[26268]: < <connection_parameters>\r\n <ip_address>10.64.151.14</ip_address>\r\n <subnet_mask>255.255.255.255</subnet_mask>\r\n <mtu>1500</mtu>\r\n <dns_server>10.64.148.4</dns_server>\r\n <dns_server>10.64.148.5</dns_server>\r\n</connection_parameters>\r\n ofonod[26268]: < \r\nOK\r\n ofonod[26268]: ste_eppsd_up_cb got result: 1 ofonod[26268]: Final response: OK ofonod[26268]: Response line: <?xml version="1.0"?> ofonod[26268]: Response line: <connection_parameters> ofonod[26268]: Response line: <ip_address>10.64.151.14</ip_address> ofonod[26268]: Response line: <subnet_mask>255.255.255.255</subnet_mask> ofonod[26268]: Response line: <mtu>1500</mtu> ofonod[26268]: Response line: <dns_server>10.64.148.4</dns_server> ofonod[26268]: Response line: <dns_server>10.64.148.5</dns_server> ofonod[26268]: Response line: </connection_parameters> ofonod[26268]: src/gprs.c:pri_activate_callback() 0x9bf22f8 caif0 >> Sure we can fix this, but this is actually just copied from >> gprs_context in "atmodem". >> BTW, the iter_init seems to be missing in atmodem's implementation, >> this is probably a bug in "atmodem". > > Sounds like a bug in atmodem then. Can you point me to the lines > where you are seeing this. I will fix it if needed. cgev_notify() is on line 208 in drivers/atmodem/gprs-context.c line missing call to g_at_result_iter_init(&iter, result); BR/Sjur ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 3/3] STE-plugin: Adding STE plugin 2010-01-17 17:28 ` [RFC 3/3] STE-plugin: Adding STE plugin sjur.brandeland 2010-01-17 21:40 ` Marcel Holtmann @ 2010-01-17 22:50 ` Denis Kenzior 1 sibling, 0 replies; 10+ messages in thread From: Denis Kenzior @ 2010-01-17 22:50 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 7768 bytes --] Hi Sjur, > diff --git a/drivers/stemodem/network-registration.c > b/drivers/stemodem/network-registration.c new file mode 100644 > index 0000000..4eeb239 > --- /dev/null > +++ b/drivers/stemodem/network-registration.c > +static void ciev_notify(GAtResult *result, gpointer user_data) > +{ > + struct ofono_netreg *netreg = user_data; > + int strength, ind; > + GAtResultIter iter; > + > + dump_response("ciev_notify", TRUE, result); > + > + g_at_result_iter_init(&iter, result); > + > + if (!g_at_result_iter_next(&iter, "+CIEV:")) > + return; > + > + if (!g_at_result_iter_next_number(&iter, &ind)) > + return; > + > + if (ind == 2) { /* signal strength indication */ > + if (!g_at_result_iter_next_number(&iter, &strength)) > + return; > + > + strength = (strength * 100) / 5; > + ofono_netreg_strength_notify(netreg, strength); > + } > +} > + > +static void cind_cb(gboolean ok, GAtResult *result, gpointer user_data) > +{ > + struct cb_data *cbd = user_data; > + ofono_netreg_strength_cb_t cb = cbd->cb; > + int strength; > + GAtResultIter iter; > + struct ofono_error error; > + > + dump_response("cind_cb", ok, result); > + decode_at_error(&error, g_at_result_final_response(result)); > + > + if (!ok) { > + cb(&error, -1, cbd->data); > + return; > + } > + > + g_at_result_iter_init(&iter, result); > + > + if (!g_at_result_iter_next(&iter, "+CIND:")) { > + CALLBACK_WITH_FAILURE(cb, -1, cbd->data); > + return; > + } > + > + /* Skip battery charge level, which is the first reported */ > + g_at_result_iter_skip_next(&iter); > + > + g_at_result_iter_next_number(&iter, &strength); > + > + strength = (strength * 100) / 5; > + > + cb(&error, strength, cbd->data); > +} > + > +static void ste_signal_strength(struct ofono_netreg *netreg, > + ofono_netreg_strength_cb_t cb, void *data) > +{ > + struct netreg_data *nd = ofono_netreg_get_data(netreg); > + struct cb_data *cbd = cb_data_new(cb, data); > + > + if (!cbd) > + goto error; > + > + if (g_at_chat_send(nd->chat, "AT+CIND?", cind_prefix, > + cind_cb, cbd, g_free) > 0) > + return; > + > +error: > + if (cbd) > + g_free(cbd); > + > + CALLBACK_WITH_FAILURE(cb, -1, data); > +} I really prefer the above to be integrated into drivers/atmodem/network- registration.c and quirked. There are many modems that will need this (MBM modems in particular.) > + > +static void creg_notify(GAtResult *result, gpointer user_data) > +{ > + struct ofono_netreg *netreg = user_data; > + GAtResultIter iter; > + int status; > + int lac = -1, ci = -1, tech = -1; > + const char *str; > + > + dump_response("creg_notify", TRUE, result); > + > + g_at_result_iter_init(&iter, result); > + > + if (!g_at_result_iter_next(&iter, "+CREG:")) > + return; > + > + g_at_result_iter_next_number(&iter, &status); > + > + if (g_at_result_iter_next_string(&iter, &str) == TRUE) > + lac = strtol(str, NULL, 16); > + else > + goto out; > + > + if (g_at_result_iter_next_string(&iter, &str) == TRUE) > + ci = strtol(str, NULL, 16); > + else > + goto out; > + > + g_at_result_iter_next_number(&iter, &tech); > + > +out: > + ofono_debug("creg_notify: %d, %d, %d, %d", status, lac, ci, tech); > + > + ofono_netreg_status_notify(netreg, status, lac, ci, tech); > +} What is different about this function from the regular CREG unsolicited parser in drivers/atmodem/atutil.c at_util_parse_reg_unsolicited? > + > + g_at_chat_register(nd->chat, "+CIEV:", > + ciev_notify, FALSE, netreg, NULL); Move this to drivers/atmodem/network-registration.c and quirk it. > +static int ste_netreg_probe(struct ofono_netreg *netreg, unsigned int > vendor, + void *data) > +{ > + GAtChat *chat = data; > + struct netreg_data *nd; > + > + nd = g_new0(struct netreg_data, 1); > + > + nd->chat = chat; > + ofono_netreg_set_data(netreg, nd); > + > + g_at_chat_send(chat, "AT+CMER=3,0,0,1", NULL, NULL, NULL, NULL); Same as above. > + > + g_at_chat_send(chat, "AT+CREG=2", NULL, > + ste_network_registration_initialized, > + netreg, NULL); Assuming STE modems support AT+CREG=?, the atmodem netreg driver should take care of this properly as well. With these changes you no longer need a dedicated stemodem driver for netreg or the ofono_netreg_driver_get part. > +++ b/drivers/stemodem/voicecall.c > +static gint call_compare(gconstpointer a, gconstpointer b) > +{ > + const struct ofono_call *ca = a; > + const struct ofono_call *cb = b; > + > + if (ca->id < cb->id) > + return -1; > + > + if (ca->id > cb->id) > + return 1; > + > + return 0; > +} Please use at_util_call_compare > + > +static gint call_compare_by_id(gconstpointer a, gconstpointer b) > +{ > + const struct ofono_call *call = a; > + unsigned int id = GPOINTER_TO_UINT(b); > + > + if (id < call->id) > + return -1; > + > + if (id > call->id) > + return 1; > + > + return 0; > +} You might find that you simply don't need this function, but if you do, please add it to atutil.c > +static void ste_release_specific(struct ofono_voicecall *vc, int id, > + ofono_voicecall_cb_t cb, void *data) > +{ > + struct voicecall_data *vd = ofono_voicecall_get_data(vc); > + struct release_id_req *req = g_try_new0(struct release_id_req, 1); > + char buf[32]; > + struct ofono_call *call; > + GSList *l; > + int ac = 0; > + > + if (!req) > + goto error; > + > + req->vc = vc; > + req->cb = cb; > + req->data = data; > + req->id = id; > + > + /* Count active calls. If there is more than one active call > + * we cannot use ATH, as it will terminate all calls. > + * The reason for using ATH and not CHLD is that > + * emergency calls can not be terminated with AT+CHLD. > + */ > + for (l = vd->calls; l; l = l->next) { > + call = l->data; > + > + if (call->status == CALL_STATUS_ACTIVE) > + ac++; > + } > + > + l = g_slist_find_custom(vd->calls, GUINT_TO_POINTER(id), > + call_compare_by_id); > + if (l == NULL) { > + ofono_error("Hangup request for unknow call"); > + goto error; > + } > + call = l->data; > + /* Check the state of the call, as AT+CHLD does not terminate > + * calls in state Dialing, Alerting and Incoming */ > + if (call->status == CALL_STATUS_DIALING || > + call->status == CALL_STATUS_ALERTING || > + call->status == CALL_STATUS_INCOMING) > + sprintf(buf, "ATH"); > + > + /* Waiting call can not be terminated by at+chld=1x, > + * have to use at+chld = 0, but that will also terminate > + * other held calls. Bug in STE's AT module. > + */ > + else if (call->status == CALL_STATUS_WAITING) > + sprintf(buf, "AT+CHLD=0"); > + > + else { > + /* A held call can not be released by ATH, need to use CHLD */ > + if (ac > 1 || call->status == CALL_STATUS_HELD) > + sprintf(buf, "AT+CHLD=1%d", id); > + else /* This is the last call, ok to use ATH. */ > + sprintf(buf, "ATH"); > + } > + > + if (g_at_chat_send(vd->chat, buf, none_prefix, > + release_id_cb, req, g_free) > 0) > + return; > + > +error: > + if (req) > + g_free(req); > + > + CALLBACK_WITH_FAILURE(cb, data); > +} All of this logic needs to go away. The core already handles all of this, including selection of ATH/CHLD, waiting/held. Please review src/voicecall.c. If the core logic is not sufficient or does not properly handle a certain case, then lets see if we can fix the core first. Drivers should not concern themselves with this stuff. With this in mind, you might not need to track any state in this driver at all. See drivers/calypsomodem/voicecall.c for details. TI's CPI notifications are almost exactly the same as the STE ECAV. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC 1/3] STE-plugin: Add vendor STE 2010-01-17 17:28 ` [RFC 1/3] STE-plugin: Add vendor STE sjur.brandeland 2010-01-17 17:28 ` [RFC 2/3] STE-plugin: Mechanism for inheritance sjur.brandeland @ 2010-01-17 21:10 ` Marcel Holtmann 1 sibling, 0 replies; 10+ messages in thread From: Marcel Holtmann @ 2010-01-17 21:10 UTC (permalink / raw) To: ofono [-- Attachment #1: Type: text/plain, Size: 4376 bytes --] Hi Sjur, > This patch add STE as vendor, and a few adjustments needed in "atmodem" > for ST-Ericsson modem. > > --- > drivers/atmodem/gprs.c | 15 +++++++++++++-- > drivers/atmodem/vendor.h | 5 +++++ > plugins/modemconf.c | 5 +++++ > 3 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/atmodem/gprs.c b/drivers/atmodem/gprs.c > index 76085d9..305f22f 100644 > --- a/drivers/atmodem/gprs.c > +++ b/drivers/atmodem/gprs.c > @@ -17,6 +17,10 @@ > * along with this program; if not, write to the Free Software > * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > * > + * Copyright (C) 2010 ST-Ericsson AB. > + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. > + * STE specific implementation. > + * > */ please don't do that. Add the proper copyright to the header if the changes you are making are major. We track authorship via the GIT itself. > #ifdef HAVE_CONFIG_H > @@ -38,6 +42,7 @@ > #include "gatresult.h" > > #include "atmodem.h" > +#include "vendor.h" > > static const char *cgreg_prefix[] = { "+CGREG:", NULL }; > static const char *cgdcont_prefix[] = { "+CGDCONT:", NULL }; > @@ -45,6 +50,7 @@ static const char *none_prefix[] = { NULL }; > > struct gprs_data { > GAtChat *chat; > + unsigned int vendor; > }; > > static void at_cgatt_cb(gboolean ok, GAtResult *result, gpointer user_data) > @@ -216,7 +222,12 @@ static void at_cgreg_test_cb(gboolean ok, GAtResult *result, > > g_at_chat_send(gd->chat, cmd, none_prefix, NULL, NULL, NULL); > g_at_chat_send(gd->chat, "AT+CGAUTO=0", none_prefix, NULL, NULL, NULL); > - g_at_chat_send(gd->chat, "AT+CGEREP=2,1", none_prefix, > + > + if (gd->vendor == OFONO_VENDOR_STE) > + g_at_chat_send(gd->chat, "AT+CGEREP=1,0", none_prefix, > + gprs_initialized, gprs, NULL); > + else > + g_at_chat_send(gd->chat, "AT+CGEREP=2,1", none_prefix, > gprs_initialized, gprs, NULL); > return; Can you add some extra comment here explaining why this is needed. Otherwise it looks like some black magic. > @@ -289,7 +300,7 @@ static int at_gprs_probe(struct ofono_gprs *gprs, > > gd = g_new0(struct gprs_data, 1); > gd->chat = chat; > - > + gd->vendor = vendor; > ofono_gprs_set_data(gprs, gd); > > g_at_chat_send(chat, "AT+CGDCONT=?", cgdcont_prefix, > diff --git a/drivers/atmodem/vendor.h b/drivers/atmodem/vendor.h > index 8d9ed47..d7b5210 100644 > --- a/drivers/atmodem/vendor.h > +++ b/drivers/atmodem/vendor.h > @@ -17,11 +17,16 @@ > * along with this program; if not, write to the Free Software > * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > * > + * Copyright (C) 2010 ST-Ericsson AB. > + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. > + * STE specific implementation. > + * > */ To be honest, just adding a new entry in enum is not really a reason to add a copyright statement here. > enum ofono_vendor { > OFONO_VENDOR_GENERIC = 0, > OFONO_VENDOR_CALYPSO, > + OFONO_VENDOR_STE, > OFONO_VENDOR_QUALCOMM_MSM, > OFONO_VENDOR_OPTION_HSO, > }; > diff --git a/plugins/modemconf.c b/plugins/modemconf.c > index c8c261f..41c7428 100644 > --- a/plugins/modemconf.c > +++ b/plugins/modemconf.c > @@ -17,6 +17,10 @@ > * along with this program; if not, write to the Free Software > * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > * > + * Copyright (C) 2010 ST-Ericsson AB. > + * Author: Marit Henriksen, marit.xx.henriksen(a)stericsson.com. > + * STE specific implementation. > + * > */ > > #ifdef HAVE_CONFIG_H > @@ -128,6 +132,7 @@ static struct ofono_modem *create_modem(GKeyFile *keyfile, const char *group) > set_address(modem, keyfile, group); > > if (!g_strcmp0(driver, "atgen") || !g_strcmp0(driver, "g1") || > + !g_strcmp0(driver, "ste") || > !g_strcmp0(driver, "calypso") || > !g_strcmp0(driver, "hfp") || > !g_strcmp0(driver, "palmpre")) I am failing to see the reason for this. I makes no sense to me. The CAIF kernel code (as far as I understand it) exports AT command sockets and they are configured differently. So this seems like a change without any functionality. Regards Marcel ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-01-20 18:24 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-01-17 17:28 [RFC 0/3] STE-plugin: Plugin for ST-Ericsson modems sjur.brandeland 2010-01-17 17:28 ` [RFC 1/3] STE-plugin: Add vendor STE sjur.brandeland 2010-01-17 17:28 ` [RFC 2/3] STE-plugin: Mechanism for inheritance sjur.brandeland 2010-01-17 17:28 ` [RFC 3/3] STE-plugin: Adding STE plugin sjur.brandeland 2010-01-17 21:40 ` Marcel Holtmann 2010-01-18 18:22 ` Sjur =?unknown-8bit?q?Br=C3=A6ndeland?= 2010-01-18 21:27 ` Marcel Holtmann 2010-01-20 18:24 ` Sjur =?unknown-8bit?q?Br=C3=A6ndeland?= 2010-01-17 22:50 ` Denis Kenzior 2010-01-17 21:10 ` [RFC 1/3] STE-plugin: Add vendor STE Marcel Holtmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox