All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH_v2 0/3] Create CDMA Network Registration atom
@ 2011-07-23  4:09 Bertrand Aygon
  2011-07-23  4:09 ` [PATCH_v2 1/3] cdmamodem: Create org.ofono.cdma.NetworkRegistration interface Bertrand Aygon
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Bertrand Aygon @ 2011-07-23  4:09 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]

Create an org.ofono.cdma.NetworkRegistraiton interface

Bertrand Aygon (3):
  cdmamodem: Create org.ofono.cdma.NetworkRegistration interface.
  cdmamodem: Create network registration atom.
  speedupcdma: register to cdma network registration atom.

 Makefile.am                              |    9 +-
 drivers/cdmamodem/cdmamodem.c            |    2 +
 drivers/cdmamodem/cdmamodem.h            |    5 +
 drivers/cdmamodem/network-registration.c |  139 ++++++++++++++++++
 include/cdma-netreg.h                    |   63 ++++++++
 include/dbus.h                           |    2 +
 plugins/speedupcdma.c                    |    3 +
 src/cdma-netreg.c                        |  233 ++++++++++++++++++++++++++++++
 src/ofono.h                              |    2 +
 9 files changed, 454 insertions(+), 4 deletions(-)
 create mode 100644 drivers/cdmamodem/network-registration.c
 create mode 100644 include/cdma-netreg.h
 create mode 100644 src/cdma-netreg.c

-- 
1.7.4.1

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH_v2 1/3] cdmamodem: Create org.ofono.cdma.NetworkRegistration interface.
  2011-07-23  4:09 [PATCH_v2 0/3] Create CDMA Network Registration atom Bertrand Aygon
@ 2011-07-23  4:09 ` Bertrand Aygon
  2011-07-23 14:52   ` Marcel Holtmann
  2011-07-23  4:09 ` [PATCH_v2 2/3] cdmamodem: Create network registration atom Bertrand Aygon
  2011-07-23  4:09 ` [PATCH_v2 3/3] speedupcdma: register to cdma " Bertrand Aygon
  2 siblings, 1 reply; 10+ messages in thread
From: Bertrand Aygon @ 2011-07-23  4:09 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 11747 bytes --]

---
 Makefile.am           |    6 +-
 include/cdma-netreg.h |   63 +++++++++++++
 include/dbus.h        |    2 +
 src/cdma-netreg.c     |  233 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/ofono.h           |    2 +
 5 files changed, 303 insertions(+), 3 deletions(-)
 create mode 100644 include/cdma-netreg.h
 create mode 100644 src/cdma-netreg.c

diff --git a/Makefile.am b/Makefile.am
index d44e62b..837d374 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -16,8 +16,8 @@ pkginclude_HEADERS = include/log.h include/plugin.h include/history.h \
 			include/cdma-sms.h include/sim-auth.h \
 			include/gprs-provision.h include/emulator.h \
 			include/location-reporting.h \
-			include/cdma-connman.h include/gnss.h \
-			include/private-network.h
+			include/cdma-connman.h include/cdma-netreg.h \
+			include/gnss.h include/private-network.h
 
 nodist_pkginclude_HEADERS = include/version.h
 
@@ -410,7 +410,7 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) src/ofono.ver \
 			src/cdma-connman.c src/gnss.c \
 			src/gnssagent.c src/gnssagent.h \
 			src/cdma-smsutil.h src/cdma-smsutil.c \
-			src/cdma-sms.c src/private-network.c
+			src/cdma-sms.c src/cdma-netreg.c src/private-network.c
 
 src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ @CAPNG_LIBS@ -ldl
 
diff --git a/include/cdma-netreg.h b/include/cdma-netreg.h
new file mode 100644
index 0000000..36f8593
--- /dev/null
+++ b/include/cdma-netreg.h
@@ -0,0 +1,63 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifndef __OFONO_CDMA_NETREG_H
+#define __OFONO_CDMA_NETREG_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <ofono/types.h>
+
+struct ofono_cdma_netreg;
+
+struct ofono_cdma_netreg_driver {
+	const char *name;
+	int (*probe)(struct ofono_cdma_netreg *cdma_netreg, unsigned int vendor,
+			void *data);
+	void (*remove)(struct ofono_cdma_netreg *cdma_netreg);
+};
+
+void ofono_cdma_netreg_status_notify(struct ofono_cdma_netreg *netreg,
+					int status);
+
+int ofono_cdma_netreg_driver_register(const struct ofono_cdma_netreg_driver *d);
+void ofono_cdma_netreg_driver_unregister(const struct ofono_cdma_netreg_driver *d);
+
+struct ofono_cdma_netreg *ofono_cdma_netreg_create(struct ofono_modem *modem,
+						unsigned int vendor,
+						const char *driver,
+						void *data);
+
+void ofono_cdma_netreg_register(struct ofono_cdma_netreg *cdma_netreg);
+void ofono_cdma_netreg_remove(struct ofono_cdma_netreg *cdma_netreg);
+
+void ofono_cdma_netreg_set_data(struct ofono_cdma_netreg *cdma_netreg, void *data);
+void *ofono_cdma_netreg_get_data(struct ofono_cdma_netreg *cdma_netreg);
+
+int ofono_cdma_netreg_get_status(struct ofono_cdma_netreg *netreg);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __OFONO_CDMA_NETREG_H */
diff --git a/include/dbus.h b/include/dbus.h
index 4dd9db5..65bda72 100644
--- a/include/dbus.h
+++ b/include/dbus.h
@@ -63,6 +63,8 @@ extern "C" {
 #define OFONO_CDMA_VOICECALL_MANAGER_INTERFACE "org.ofono.cdma.VoiceCallManager"
 #define OFONO_CDMA_MESSAGE_MANAGER_INTERFACE "org.ofono.cdma.MessageManager"
 #define OFONO_CDMA_CONNECTION_MANAGER_INTERFACE "org.ofono.cdma.ConnectionManager"
+#define OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE \
+					"org.ofono.cdma.NetworkRegistration"
 
 /* Essentially a{sv} */
 #define OFONO_PROPERTIES_ARRAY_SIGNATURE DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING \
diff --git a/src/cdma-netreg.c b/src/cdma-netreg.c
new file mode 100644
index 0000000..c19a861
--- /dev/null
+++ b/src/cdma-netreg.c
@@ -0,0 +1,233 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <string.h>
+#include <stdio.h>
+#include <errno.h>
+
+#include <glib.h>
+#include <gdbus.h>
+#include <sys/time.h>
+
+#include "ofono.h"
+#include "common.h"
+
+static GSList *g_drivers;
+
+struct ofono_cdma_netreg {
+	int status;
+	const struct ofono_cdma_netreg_driver *driver;
+	void *driver_data;
+	struct ofono_atom *atom;
+};
+
+static DBusMessage *network_get_properties(DBusConnection *conn,
+						DBusMessage *msg, void *data)
+{
+	struct ofono_cdma_netreg *netreg = data;
+	DBusMessage *reply;
+	DBusMessageIter iter;
+	DBusMessageIter dict;
+
+	const char *status = registration_status_to_string(netreg->status);
+
+	reply = dbus_message_new_method_return(msg);
+	if (reply == NULL)
+		return NULL;
+
+	dbus_message_iter_init_append(reply, &iter);
+
+	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
+					OFONO_PROPERTIES_ARRAY_SIGNATURE,
+					&dict);
+
+	ofono_dbus_dict_append(&dict, "Status", DBUS_TYPE_STRING, &status);
+
+	dbus_message_iter_close_container(&iter, &dict);
+
+	return reply;
+}
+
+static GDBusMethodTable cdma_netreg_manager_methods[] = {
+	/* TODO */
+	{ "GetProperties",  "",  "a{sv}",	network_get_properties },
+	{ }
+};
+
+static GDBusSignalTable cdma_netreg_manager_signals[] = {
+	/* TODO */
+	{ }
+};
+
+static void set_registration_status(struct ofono_cdma_netreg *netreg,
+						int status)
+{
+	const char *str_status = registration_status_to_string(status);
+	const char *path = __ofono_atom_get_path(netreg->atom);
+	DBusConnection *conn = ofono_dbus_get_connection();
+
+	netreg->status = status;
+
+	ofono_dbus_signal_property_changed(conn, path,
+				OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE,
+				"Status", DBUS_TYPE_STRING, &str_status);
+}
+
+void ofono_cdma_netreg_status_notify(struct ofono_cdma_netreg *netreg,
+					int status)
+{
+	if (netreg == NULL)
+		return;
+
+	if (netreg->status != status)
+		set_registration_status(netreg, status);
+}
+
+int ofono_cdma_netreg_get_status(struct ofono_cdma_netreg *netreg)
+{
+	if (netreg == NULL)
+		return -1;
+
+	return netreg->status;
+}
+
+int ofono_cdma_netreg_driver_register(const struct ofono_cdma_netreg_driver *d)
+{
+	DBG("driver: %p, name: %s", d, d->name);
+
+	if (d->probe == NULL)
+		return -EINVAL;
+
+	g_drivers = g_slist_prepend(g_drivers, (void *)d);
+
+	return 0;
+}
+
+void ofono_cdma_netreg_driver_unregister(const struct ofono_cdma_netreg_driver *d)
+{
+	DBG("driver: %p, name: %s", d, d->name);
+
+	g_drivers = g_slist_remove(g_drivers, (void *)d);
+}
+
+static void cdma_netreg_unregister(struct ofono_atom *atom)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(atom);
+	const char *path = __ofono_atom_get_path(atom);
+
+	g_dbus_unregister_interface(conn, path,
+				OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE);
+
+	ofono_modem_remove_interface(modem,
+				OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE);
+}
+
+static void cdma_netreg_remove(struct ofono_atom *atom)
+{
+	struct ofono_cdma_netreg *cdma_netreg = __ofono_atom_get_data(atom);
+
+	DBG("atom: %p", atom);
+
+	if (cdma_netreg == NULL)
+		return;
+
+	if (cdma_netreg->driver && cdma_netreg->driver->remove)
+		cdma_netreg->driver->remove(cdma_netreg);
+
+	g_free(cdma_netreg);
+}
+
+struct ofono_cdma_netreg *ofono_cdma_netreg_create(struct ofono_modem *modem,
+						unsigned int vendor,
+						const char *driver,
+						void *data)
+{
+	struct ofono_cdma_netreg *cdma_netreg;
+	GSList *l;
+
+	if (driver == NULL)
+		return NULL;
+
+	cdma_netreg = g_try_new0(struct ofono_cdma_netreg, 1);
+	if (cdma_netreg == NULL)
+		return NULL;
+
+	cdma_netreg->atom = __ofono_modem_add_atom(modem,
+					OFONO_ATOM_TYPE_CDMA_NETREG,
+					cdma_netreg_remove, cdma_netreg);
+
+	for (l = g_drivers; l; l = l->next) {
+		const struct ofono_cdma_netreg_driver *drv = l->data;
+
+		if (g_strcmp0(drv->name, driver))
+			continue;
+
+		if (drv->probe(cdma_netreg, vendor, data) < 0)
+			continue;
+
+		cdma_netreg->driver = drv;
+		break;
+	}
+
+	return cdma_netreg;
+}
+
+void ofono_cdma_netreg_register(struct ofono_cdma_netreg *cdma_netreg)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	struct ofono_modem *modem = __ofono_atom_get_modem(cdma_netreg->atom);
+	const char *path = __ofono_atom_get_path(cdma_netreg->atom);
+
+	if (!g_dbus_register_interface(conn, path,
+				OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE,
+				cdma_netreg_manager_methods,
+				cdma_netreg_manager_signals,
+				NULL, cdma_netreg, NULL)) {
+		ofono_error("Could not create %s interface",
+				OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE);
+		return;
+	}
+
+	ofono_modem_add_interface(modem,
+				OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE);
+
+	__ofono_atom_register(cdma_netreg->atom, cdma_netreg_unregister);
+}
+
+void ofono_cdma_netreg_remove(struct ofono_cdma_netreg *cdma_netreg)
+{
+	__ofono_atom_free(cdma_netreg->atom);
+}
+
+void ofono_cdma_netreg_set_data(struct ofono_cdma_netreg *cdma_netreg, void *data)
+{
+	cdma_netreg->driver_data = data;
+}
+
+void *ofono_cdma_netreg_get_data(struct ofono_cdma_netreg *cdma_netreg)
+{
+	return cdma_netreg->driver_data;
+}
diff --git a/src/ofono.h b/src/ofono.h
index 6524806..3de3052 100644
--- a/src/ofono.h
+++ b/src/ofono.h
@@ -135,6 +135,7 @@ enum ofono_atom_type {
 	OFONO_ATOM_TYPE_LOCATION_REPORTING,
 	OFONO_ATOM_TYPE_GNSS,
 	OFONO_ATOM_TYPE_CDMA_SMS,
+	OFONO_ATOM_TYPE_CDMA_NETREG
 };
 
 enum ofono_atom_watch_condition {
@@ -474,6 +475,7 @@ void __ofono_gprs_provision_free_settings(
 #include <ofono/emulator.h>
 #include <ofono/gnss.h>
 #include <ofono/cdma-sms.h>
+#include <ofono/cdma-netreg.h>
 #include <ofono/private-network.h>
 
 void __ofono_private_network_release(int id);
-- 
1.7.4.1

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH_v2 2/3] cdmamodem: Create network registration atom.
  2011-07-23  4:09 [PATCH_v2 0/3] Create CDMA Network Registration atom Bertrand Aygon
  2011-07-23  4:09 ` [PATCH_v2 1/3] cdmamodem: Create org.ofono.cdma.NetworkRegistration interface Bertrand Aygon
@ 2011-07-23  4:09 ` Bertrand Aygon
  2011-07-23 14:59   ` Marcel Holtmann
  2011-07-23  4:09 ` [PATCH_v2 3/3] speedupcdma: register to cdma " Bertrand Aygon
  2 siblings, 1 reply; 10+ messages in thread
From: Bertrand Aygon @ 2011-07-23  4:09 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 6090 bytes --]

---
 Makefile.am                              |    3 +-
 drivers/cdmamodem/cdmamodem.c            |    2 +
 drivers/cdmamodem/cdmamodem.h            |    5 +
 drivers/cdmamodem/network-registration.c |  139 ++++++++++++++++++++++++++++++
 4 files changed, 148 insertions(+), 1 deletions(-)
 create mode 100644 drivers/cdmamodem/network-registration.c

diff --git a/Makefile.am b/Makefile.am
index 837d374..e00f73b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -269,7 +269,8 @@ builtin_sources += drivers/cdmamodem/cdmamodem.h \
 			drivers/cdmamodem/cdmamodem.c \
 			drivers/cdmamodem/voicecall.c \
 			drivers/cdmamodem/devinfo.c \
-			drivers/cdmamodem/connman.c
+			drivers/cdmamodem/connman.c \
+			drivers/cdmamodem/network-registration.c
 endif
 
 builtin_modules += g1
diff --git a/drivers/cdmamodem/cdmamodem.c b/drivers/cdmamodem/cdmamodem.c
index 1b19a4a..60d5315 100644
--- a/drivers/cdmamodem/cdmamodem.c
+++ b/drivers/cdmamodem/cdmamodem.c
@@ -37,6 +37,7 @@ static int cdmamodem_init(void)
 	cdma_voicecall_init();
 	cdma_devinfo_init();
 	cdma_connman_init();
+	cdma_netreg_init();
 
 	return 0;
 }
@@ -46,6 +47,7 @@ static void cdmamodem_exit(void)
 	cdma_voicecall_exit();
 	cdma_devinfo_exit();
 	cdma_connman_exit();
+	cdma_netreg_exit();
 }
 
 OFONO_PLUGIN_DEFINE(cdmamodem, "CDMA AT modem driver", VERSION,
diff --git a/drivers/cdmamodem/cdmamodem.h b/drivers/cdmamodem/cdmamodem.h
index 90e2848..df69082 100644
--- a/drivers/cdmamodem/cdmamodem.h
+++ b/drivers/cdmamodem/cdmamodem.h
@@ -23,7 +23,12 @@
 
 extern void cdma_voicecall_init(void);
 extern void cdma_voicecall_exit(void);
+
 extern void cdma_devinfo_init(void);
 extern void cdma_devinfo_exit(void);
+
 extern void cdma_connman_init(void);
 extern void cdma_connman_exit(void);
+
+extern void cdma_netreg_init(void);
+extern void cdma_netreg_exit(void);
diff --git a/drivers/cdmamodem/network-registration.c b/drivers/cdmamodem/network-registration.c
new file mode 100644
index 0000000..7641e66
--- /dev/null
+++ b/drivers/cdmamodem/network-registration.c
@@ -0,0 +1,139 @@
+/*
+ *
+ *  oFono - Open Source Telephony
+ *
+ *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License version 2 as
+ *  published by the Free Software Foundation.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#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/cdma-netreg.h>
+
+#include "gatchat.h"
+#include "gatresult.h"
+
+#include "common.h"
+#include "cdmamodem.h"
+
+static const char *sysinfo_prefix[] = { "^SYSINFO:", NULL };
+
+struct netreg_data {
+	GAtChat *chat;
+	unsigned int vendor;
+};
+
+static void sysinfo_cb(gboolean ok, GAtResult *result, gpointer user_data)
+{
+	struct ofono_cdma_netreg *netreg = user_data;
+	gint service_state;
+	gint service_domain;
+	gint roaming_state;
+	int status;
+	GAtResultIter iter;
+
+	if (!ok)
+		return;
+
+	g_at_result_iter_init(&iter, result);
+
+	if (!g_at_result_iter_next(&iter, "^SYSINFO:"))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &service_state))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &service_domain))
+		return;
+
+	if (!g_at_result_iter_next_number(&iter, &roaming_state))
+		return;
+
+	switch (service_state) {
+	case 0:
+	case 4:
+		status = NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;
+		break;
+	case 2:
+		if (roaming_state)
+			status = NETWORK_REGISTRATION_STATUS_ROAMING;
+		else
+			status = NETWORK_REGISTRATION_STATUS_REGISTERED;
+		break;
+	default:
+		status = NETWORK_REGISTRATION_STATUS_UNKNOWN;
+		break;
+	}
+
+	ofono_cdma_netreg_register(netreg);
+
+	ofono_cdma_netreg_status_notify(netreg, status);
+}
+
+static int cdma_netreg_probe(struct ofono_cdma_netreg *netreg,
+				unsigned int vendor, void *data)
+{
+	GAtChat *chat = data;
+	struct netreg_data *nd;
+
+	nd = g_new0(struct netreg_data, 1);
+
+	nd->chat = g_at_chat_clone(chat);
+	nd->vendor = vendor;
+	ofono_cdma_netreg_set_data(netreg, nd);
+
+	g_at_chat_send(nd->chat, "AT^SYSINFO", sysinfo_prefix,
+			sysinfo_cb, netreg, NULL);
+
+	return 0;
+}
+
+static void cdma_netreg_remove(struct ofono_cdma_netreg *netreg)
+{
+	struct netreg_data *nd = ofono_cdma_netreg_get_data(netreg);
+
+	ofono_cdma_netreg_set_data(netreg, NULL);
+
+	g_at_chat_unref(nd->chat);
+	g_free(nd);
+}
+
+static struct ofono_cdma_netreg_driver driver = {
+	.name				= "cdmamodem",
+	.probe				= cdma_netreg_probe,
+	.remove				= cdma_netreg_remove,
+};
+
+void cdma_netreg_init(void)
+{
+	ofono_cdma_netreg_driver_register(&driver);
+}
+
+void cdma_netreg_exit(void)
+{
+	ofono_cdma_netreg_driver_unregister(&driver);
+}
-- 
1.7.4.1

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH_v2 3/3] speedupcdma: register to cdma network registration atom.
  2011-07-23  4:09 [PATCH_v2 0/3] Create CDMA Network Registration atom Bertrand Aygon
  2011-07-23  4:09 ` [PATCH_v2 1/3] cdmamodem: Create org.ofono.cdma.NetworkRegistration interface Bertrand Aygon
  2011-07-23  4:09 ` [PATCH_v2 2/3] cdmamodem: Create network registration atom Bertrand Aygon
@ 2011-07-23  4:09 ` Bertrand Aygon
  2011-07-23 15:00   ` Marcel Holtmann
  2 siblings, 1 reply; 10+ messages in thread
From: Bertrand Aygon @ 2011-07-23  4:09 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1284 bytes --]

---
 plugins/speedupcdma.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/plugins/speedupcdma.c b/plugins/speedupcdma.c
index b7e54a1..14189aa 100644
--- a/plugins/speedupcdma.c
+++ b/plugins/speedupcdma.c
@@ -36,6 +36,7 @@
 #include <ofono/modem.h>
 #include <ofono/devinfo.h>
 #include <ofono/cdma-connman.h>
+#include <ofono/cdma-netreg.h>
 #include <ofono/log.h>
 
 #include <drivers/atmodem/atutil.h>
@@ -181,6 +182,8 @@ static void speedupcdma_post_online(struct ofono_modem *modem)
 
 	DBG("%p", modem);
 
+	ofono_cdma_netreg_create(modem, 0, "cdmamodem", data->chat);
+
 	ofono_cdma_connman_create(modem, 0, "cdmamodem", data->chat);
 }
 
-- 
1.7.4.1

---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH_v2 1/3] cdmamodem: Create org.ofono.cdma.NetworkRegistration interface.
  2011-07-23  4:09 ` [PATCH_v2 1/3] cdmamodem: Create org.ofono.cdma.NetworkRegistration interface Bertrand Aygon
@ 2011-07-23 14:52   ` Marcel Holtmann
  0 siblings, 0 replies; 10+ messages in thread
From: Marcel Holtmann @ 2011-07-23 14:52 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 12648 bytes --]

Hi Bertrand,

>  Makefile.am           |    6 +-
>  include/cdma-netreg.h |   63 +++++++++++++
>  include/dbus.h        |    2 +
>  src/cdma-netreg.c     |  233 +++++++++++++++++++++++++++++++++++++++++++++++++
>  src/ofono.h           |    2 +
>  5 files changed, 303 insertions(+), 3 deletions(-)
>  create mode 100644 include/cdma-netreg.h
>  create mode 100644 src/cdma-netreg.c

I prefer to keep include/dbus.h changes in a separate patch.

> diff --git a/Makefile.am b/Makefile.am
> index d44e62b..837d374 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -16,8 +16,8 @@ pkginclude_HEADERS = include/log.h include/plugin.h include/history.h \
>  			include/cdma-sms.h include/sim-auth.h \
>  			include/gprs-provision.h include/emulator.h \
>  			include/location-reporting.h \
> -			include/cdma-connman.h include/gnss.h \
> -			include/private-network.h
> +			include/cdma-connman.h include/cdma-netreg.h \
> +			include/gnss.h include/private-network.h
>  
>  nodist_pkginclude_HEADERS = include/version.h
>  
> @@ -410,7 +410,7 @@ src_ofonod_SOURCES = $(gdbus_sources) $(builtin_sources) src/ofono.ver \
>  			src/cdma-connman.c src/gnss.c \
>  			src/gnssagent.c src/gnssagent.h \
>  			src/cdma-smsutil.h src/cdma-smsutil.c \
> -			src/cdma-sms.c src/private-network.c
> +			src/cdma-sms.c src/cdma-netreg.c src/private-network.c
>  
>  src_ofonod_LDADD = $(builtin_libadd) @GLIB_LIBS@ @DBUS_LIBS@ @CAPNG_LIBS@ -ldl
>  
> diff --git a/include/cdma-netreg.h b/include/cdma-netreg.h
> new file mode 100644
> index 0000000..36f8593
> --- /dev/null
> +++ b/include/cdma-netreg.h
> @@ -0,0 +1,63 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#ifndef __OFONO_CDMA_NETREG_H
> +#define __OFONO_CDMA_NETREG_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <ofono/types.h>
> +
> +struct ofono_cdma_netreg;
> +
> +struct ofono_cdma_netreg_driver {
> +	const char *name;
> +	int (*probe)(struct ofono_cdma_netreg *cdma_netreg, unsigned int vendor,
> +			void *data);
> +	void (*remove)(struct ofono_cdma_netreg *cdma_netreg);
> +};
> +
> +void ofono_cdma_netreg_status_notify(struct ofono_cdma_netreg *netreg,
> +					int status);
> +
> +int ofono_cdma_netreg_driver_register(const struct ofono_cdma_netreg_driver *d);
> +void ofono_cdma_netreg_driver_unregister(const struct ofono_cdma_netreg_driver *d);
> +
> +struct ofono_cdma_netreg *ofono_cdma_netreg_create(struct ofono_modem *modem,
> +						unsigned int vendor,
> +						const char *driver,
> +						void *data);
> +
> +void ofono_cdma_netreg_register(struct ofono_cdma_netreg *cdma_netreg);
> +void ofono_cdma_netreg_remove(struct ofono_cdma_netreg *cdma_netreg);
> +
> +void ofono_cdma_netreg_set_data(struct ofono_cdma_netreg *cdma_netreg, void *data);
> +void *ofono_cdma_netreg_get_data(struct ofono_cdma_netreg *cdma_netreg);
> +
> +int ofono_cdma_netreg_get_status(struct ofono_cdma_netreg *netreg);

If you wanna put in a basic skeleton first, then that is fine, but then
please leave details like status out of it.

> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* __OFONO_CDMA_NETREG_H */
> diff --git a/include/dbus.h b/include/dbus.h
> index 4dd9db5..65bda72 100644
> --- a/include/dbus.h
> +++ b/include/dbus.h
> @@ -63,6 +63,8 @@ extern "C" {
>  #define OFONO_CDMA_VOICECALL_MANAGER_INTERFACE "org.ofono.cdma.VoiceCallManager"
>  #define OFONO_CDMA_MESSAGE_MANAGER_INTERFACE "org.ofono.cdma.MessageManager"
>  #define OFONO_CDMA_CONNECTION_MANAGER_INTERFACE "org.ofono.cdma.ConnectionManager"
> +#define OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE \
> +					"org.ofono.cdma.NetworkRegistration"

As mentioned above, separate patch.

>  
>  /* Essentially a{sv} */
>  #define OFONO_PROPERTIES_ARRAY_SIGNATURE DBUS_DICT_ENTRY_BEGIN_CHAR_AS_STRING \
> diff --git a/src/cdma-netreg.c b/src/cdma-netreg.c
> new file mode 100644
> index 0000000..c19a861
> --- /dev/null
> +++ b/src/cdma-netreg.c
> @@ -0,0 +1,233 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <string.h>
> +#include <stdio.h>
> +#include <errno.h>
> +
> +#include <glib.h>
> +#include <gdbus.h>
> +#include <sys/time.h>

Please keep the includes to a minimum that is required.

> +
> +#include "ofono.h"
> +#include "common.h"

What is common.h doing here?

> +
> +static GSList *g_drivers;
> +
> +struct ofono_cdma_netreg {
> +	int status;

Leave status out for a skeleton.

> +	const struct ofono_cdma_netreg_driver *driver;
> +	void *driver_data;
> +	struct ofono_atom *atom;
> +};
> +
> +static DBusMessage *network_get_properties(DBusConnection *conn,
> +						DBusMessage *msg, void *data)
> +{
> +	struct ofono_cdma_netreg *netreg = data;
> +	DBusMessage *reply;
> +	DBusMessageIter iter;
> +	DBusMessageIter dict;
> +
> +	const char *status = registration_status_to_string(netreg->status);

I am not sure that using the GSM counterpart is such a good idea here.

> +
> +	reply = dbus_message_new_method_return(msg);
> +	if (reply == NULL)
> +		return NULL;
> +
> +	dbus_message_iter_init_append(reply, &iter);
> +
> +	dbus_message_iter_open_container(&iter, DBUS_TYPE_ARRAY,
> +					OFONO_PROPERTIES_ARRAY_SIGNATURE,
> +					&dict);
> +
> +	ofono_dbus_dict_append(&dict, "Status", DBUS_TYPE_STRING, &status);
> +
> +	dbus_message_iter_close_container(&iter, &dict);
> +
> +	return reply;
> +}
> +
> +static GDBusMethodTable cdma_netreg_manager_methods[] = {
> +	/* TODO */
> +	{ "GetProperties",  "",  "a{sv}",	network_get_properties },
> +	{ }
> +};
> +
> +static GDBusSignalTable cdma_netreg_manager_signals[] = {
> +	/* TODO */
> +	{ }
> +};

Both TODOs are a bad idea. I rather implement it and have it fail with
an error not implemented than having weird TODO entries here.

> +
> +static void set_registration_status(struct ofono_cdma_netreg *netreg,
> +						int status)
> +{
> +	const char *str_status = registration_status_to_string(status);
> +	const char *path = __ofono_atom_get_path(netreg->atom);
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +
> +	netreg->status = status;
> +
> +	ofono_dbus_signal_property_changed(conn, path,
> +				OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE,
> +				"Status", DBUS_TYPE_STRING, &str_status);
> +}
> +
> +void ofono_cdma_netreg_status_notify(struct ofono_cdma_netreg *netreg,
> +					int status)
> +{
> +	if (netreg == NULL)
> +		return;
> +
> +	if (netreg->status != status)
> +		set_registration_status(netreg, status);
> +}
> +
> +int ofono_cdma_netreg_get_status(struct ofono_cdma_netreg *netreg)
> +{
> +	if (netreg == NULL)
> +		return -1;
> +
> +	return netreg->status;
> +}
> +
> +int ofono_cdma_netreg_driver_register(const struct ofono_cdma_netreg_driver *d)
> +{
> +	DBG("driver: %p, name: %s", d, d->name);
> +
> +	if (d->probe == NULL)
> +		return -EINVAL;
> +
> +	g_drivers = g_slist_prepend(g_drivers, (void *)d);
> +
> +	return 0;
> +}
> +
> +void ofono_cdma_netreg_driver_unregister(const struct ofono_cdma_netreg_driver *d)
> +{
> +	DBG("driver: %p, name: %s", d, d->name);
> +
> +	g_drivers = g_slist_remove(g_drivers, (void *)d);
> +}
> +
> +static void cdma_netreg_unregister(struct ofono_atom *atom)
> +{
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +	struct ofono_modem *modem = __ofono_atom_get_modem(atom);
> +	const char *path = __ofono_atom_get_path(atom);
> +
> +	g_dbus_unregister_interface(conn, path,
> +				OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE);
> +
> +	ofono_modem_remove_interface(modem,
> +				OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE);
> +}
> +
> +static void cdma_netreg_remove(struct ofono_atom *atom)
> +{
> +	struct ofono_cdma_netreg *cdma_netreg = __ofono_atom_get_data(atom);
> +
> +	DBG("atom: %p", atom);
> +
> +	if (cdma_netreg == NULL)
> +		return;
> +
> +	if (cdma_netreg->driver && cdma_netreg->driver->remove)
> +		cdma_netreg->driver->remove(cdma_netreg);
> +
> +	g_free(cdma_netreg);
> +}
> +
> +struct ofono_cdma_netreg *ofono_cdma_netreg_create(struct ofono_modem *modem,
> +						unsigned int vendor,
> +						const char *driver,
> +						void *data)
> +{
> +	struct ofono_cdma_netreg *cdma_netreg;
> +	GSList *l;
> +
> +	if (driver == NULL)
> +		return NULL;
> +
> +	cdma_netreg = g_try_new0(struct ofono_cdma_netreg, 1);
> +	if (cdma_netreg == NULL)
> +		return NULL;
> +
> +	cdma_netreg->atom = __ofono_modem_add_atom(modem,
> +					OFONO_ATOM_TYPE_CDMA_NETREG,
> +					cdma_netreg_remove, cdma_netreg);
> +
> +	for (l = g_drivers; l; l = l->next) {
> +		const struct ofono_cdma_netreg_driver *drv = l->data;
> +
> +		if (g_strcmp0(drv->name, driver))
> +			continue;
> +
> +		if (drv->probe(cdma_netreg, vendor, data) < 0)
> +			continue;
> +
> +		cdma_netreg->driver = drv;
> +		break;
> +	}
> +
> +	return cdma_netreg;
> +}
> +
> +void ofono_cdma_netreg_register(struct ofono_cdma_netreg *cdma_netreg)
> +{
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +	struct ofono_modem *modem = __ofono_atom_get_modem(cdma_netreg->atom);
> +	const char *path = __ofono_atom_get_path(cdma_netreg->atom);
> +
> +	if (!g_dbus_register_interface(conn, path,
> +				OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE,
> +				cdma_netreg_manager_methods,
> +				cdma_netreg_manager_signals,
> +				NULL, cdma_netreg, NULL)) {
> +		ofono_error("Could not create %s interface",
> +				OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE);
> +		return;
> +	}
> +
> +	ofono_modem_add_interface(modem,
> +				OFONO_CDMA_NETWORK_REGISTRATION_INTERFACE);
> +
> +	__ofono_atom_register(cdma_netreg->atom, cdma_netreg_unregister);
> +}
> +
> +void ofono_cdma_netreg_remove(struct ofono_cdma_netreg *cdma_netreg)
> +{
> +	__ofono_atom_free(cdma_netreg->atom);
> +}
> +
> +void ofono_cdma_netreg_set_data(struct ofono_cdma_netreg *cdma_netreg, void *data)
> +{
> +	cdma_netreg->driver_data = data;
> +}
> +
> +void *ofono_cdma_netreg_get_data(struct ofono_cdma_netreg *cdma_netreg)
> +{
> +	return cdma_netreg->driver_data;
> +}

The rest is a 1:1 copy of the GSM netreg atom anyway.

> diff --git a/src/ofono.h b/src/ofono.h
> index 6524806..3de3052 100644
> --- a/src/ofono.h
> +++ b/src/ofono.h
> @@ -135,6 +135,7 @@ enum ofono_atom_type {
>  	OFONO_ATOM_TYPE_LOCATION_REPORTING,
>  	OFONO_ATOM_TYPE_GNSS,
>  	OFONO_ATOM_TYPE_CDMA_SMS,
> +	OFONO_ATOM_TYPE_CDMA_NETREG
>  };

Please do this in a separate patch. Same as with the D-Bus stuff.
>  
>  enum ofono_atom_watch_condition {
> @@ -474,6 +475,7 @@ void __ofono_gprs_provision_free_settings(
>  #include <ofono/emulator.h>
>  #include <ofono/gnss.h>
>  #include <ofono/cdma-sms.h>
> +#include <ofono/cdma-netreg.h>
>  #include <ofono/private-network.h>
>  
>  void __ofono_private_network_release(int id);

Regards

Marcel



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH_v2 2/3] cdmamodem: Create network registration atom.
  2011-07-23  4:09 ` [PATCH_v2 2/3] cdmamodem: Create network registration atom Bertrand Aygon
@ 2011-07-23 14:59   ` Marcel Holtmann
  2011-07-23 15:11     ` Denis Kenzior
  0 siblings, 1 reply; 10+ messages in thread
From: Marcel Holtmann @ 2011-07-23 14:59 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 6530 bytes --]

Hi Bertrand,

>  Makefile.am                              |    3 +-
>  drivers/cdmamodem/cdmamodem.c            |    2 +
>  drivers/cdmamodem/cdmamodem.h            |    5 +
>  drivers/cdmamodem/network-registration.c |  139 ++++++++++++++++++++++++++++++
>  4 files changed, 148 insertions(+), 1 deletions(-)
>  create mode 100644 drivers/cdmamodem/network-registration.c
> 
> diff --git a/Makefile.am b/Makefile.am
> index 837d374..e00f73b 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -269,7 +269,8 @@ builtin_sources += drivers/cdmamodem/cdmamodem.h \
>  			drivers/cdmamodem/cdmamodem.c \
>  			drivers/cdmamodem/voicecall.c \
>  			drivers/cdmamodem/devinfo.c \
> -			drivers/cdmamodem/connman.c
> +			drivers/cdmamodem/connman.c \
> +			drivers/cdmamodem/network-registration.c
>  endif
>  
>  builtin_modules += g1
> diff --git a/drivers/cdmamodem/cdmamodem.c b/drivers/cdmamodem/cdmamodem.c
> index 1b19a4a..60d5315 100644
> --- a/drivers/cdmamodem/cdmamodem.c
> +++ b/drivers/cdmamodem/cdmamodem.c
> @@ -37,6 +37,7 @@ static int cdmamodem_init(void)
>  	cdma_voicecall_init();
>  	cdma_devinfo_init();
>  	cdma_connman_init();
> +	cdma_netreg_init();
>  
>  	return 0;
>  }
> @@ -46,6 +47,7 @@ static void cdmamodem_exit(void)
>  	cdma_voicecall_exit();
>  	cdma_devinfo_exit();
>  	cdma_connman_exit();
> +	cdma_netreg_exit();
>  }
>  
>  OFONO_PLUGIN_DEFINE(cdmamodem, "CDMA AT modem driver", VERSION,
> diff --git a/drivers/cdmamodem/cdmamodem.h b/drivers/cdmamodem/cdmamodem.h
> index 90e2848..df69082 100644
> --- a/drivers/cdmamodem/cdmamodem.h
> +++ b/drivers/cdmamodem/cdmamodem.h
> @@ -23,7 +23,12 @@
>  
>  extern void cdma_voicecall_init(void);
>  extern void cdma_voicecall_exit(void);
> +
>  extern void cdma_devinfo_init(void);
>  extern void cdma_devinfo_exit(void);
> +
>  extern void cdma_connman_init(void);
>  extern void cdma_connman_exit(void);
> +

Please separate cleanup fixes out into separate patch.

> +extern void cdma_netreg_init(void);
> +extern void cdma_netreg_exit(void);
> diff --git a/drivers/cdmamodem/network-registration.c b/drivers/cdmamodem/network-registration.c
> new file mode 100644
> index 0000000..7641e66
> --- /dev/null
> +++ b/drivers/cdmamodem/network-registration.c
> @@ -0,0 +1,139 @@
> +/*
> + *
> + *  oFono - Open Source Telephony
> + *
> + *  Copyright (C) 2008-2011  Intel Corporation. All rights reserved.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#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/cdma-netreg.h>
> +
> +#include "gatchat.h"
> +#include "gatresult.h"
> +
> +#include "common.h"

I know that you are re-using the GSM network registration constants here
and thus need common.h.

Denis, is this acceptable for you?

> +#include "cdmamodem.h"
> +
> +static const char *sysinfo_prefix[] = { "^SYSINFO:", NULL };
> +
> +struct netreg_data {
> +	GAtChat *chat;
> +	unsigned int vendor;

You have no vendor quirks. So leave this out.

> +};
> +
> +static void sysinfo_cb(gboolean ok, GAtResult *result, gpointer user_data)
> +{
> +	struct ofono_cdma_netreg *netreg = user_data;
> +	gint service_state;
> +	gint service_domain;
> +	gint roaming_state;
> +	int status;
> +	GAtResultIter iter;
> +
> +	if (!ok)
> +		return;
> +
> +	g_at_result_iter_init(&iter, result);
> +
> +	if (!g_at_result_iter_next(&iter, "^SYSINFO:"))
> +		return;
> +
> +	if (!g_at_result_iter_next_number(&iter, &service_state))
> +		return;
> +
> +	if (!g_at_result_iter_next_number(&iter, &service_domain))
> +		return;
> +
> +	if (!g_at_result_iter_next_number(&iter, &roaming_state))
> +		return;
> +
> +	switch (service_state) {
> +	case 0:
> +	case 4:

Please put at least comments behind every case label so that we know
what it does mean.

> +		status = NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;
> +		break;
> +	case 2:
> +		if (roaming_state)
> +			status = NETWORK_REGISTRATION_STATUS_ROAMING;
> +		else
> +			status = NETWORK_REGISTRATION_STATUS_REGISTERED;
> +		break;
> +	default:
> +		status = NETWORK_REGISTRATION_STATUS_UNKNOWN;
> +		break;
> +	}
> +
> +	ofono_cdma_netreg_register(netreg);
> +
> +	ofono_cdma_netreg_status_notify(netreg, status);
> +}

How do we get unsolicited notification when the status changes?

> +
> +static int cdma_netreg_probe(struct ofono_cdma_netreg *netreg,
> +				unsigned int vendor, void *data)
> +{
> +	GAtChat *chat = data;
> +	struct netreg_data *nd;
> +
> +	nd = g_new0(struct netreg_data, 1);

Use g_try_new0 here. Even while old code is not doing this, we try to
update it eventually.

> +
> +	nd->chat = g_at_chat_clone(chat);
> +	nd->vendor = vendor;
> +	ofono_cdma_netreg_set_data(netreg, nd);
> +
> +	g_at_chat_send(nd->chat, "AT^SYSINFO", sysinfo_prefix,
> +			sysinfo_cb, netreg, NULL);
> +
> +	return 0;
> +}
> +
> +static void cdma_netreg_remove(struct ofono_cdma_netreg *netreg)
> +{
> +	struct netreg_data *nd = ofono_cdma_netreg_get_data(netreg);
> +
> +	ofono_cdma_netreg_set_data(netreg, NULL);
> +
> +	g_at_chat_unref(nd->chat);
> +	g_free(nd);
> +}
> +
> +static struct ofono_cdma_netreg_driver driver = {
> +	.name				= "cdmamodem",
> +	.probe				= cdma_netreg_probe,
> +	.remove				= cdma_netreg_remove,
> +};

No need for super extra tabs in between. Cut them to a minimum.

> +
> +void cdma_netreg_init(void)
> +{
> +	ofono_cdma_netreg_driver_register(&driver);
> +}
> +
> +void cdma_netreg_exit(void)
> +{
> +	ofono_cdma_netreg_driver_unregister(&driver);
> +}

Regards

Marcel



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH_v2 3/3] speedupcdma: register to cdma network registration atom.
  2011-07-23  4:09 ` [PATCH_v2 3/3] speedupcdma: register to cdma " Bertrand Aygon
@ 2011-07-23 15:00   ` Marcel Holtmann
  0 siblings, 0 replies; 10+ messages in thread
From: Marcel Holtmann @ 2011-07-23 15:00 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 979 bytes --]

Hi Bertrand,

>  plugins/speedupcdma.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/plugins/speedupcdma.c b/plugins/speedupcdma.c
> index b7e54a1..14189aa 100644
> --- a/plugins/speedupcdma.c
> +++ b/plugins/speedupcdma.c
> @@ -36,6 +36,7 @@
>  #include <ofono/modem.h>
>  #include <ofono/devinfo.h>
>  #include <ofono/cdma-connman.h>
> +#include <ofono/cdma-netreg.h>
>  #include <ofono/log.h>
>  
>  #include <drivers/atmodem/atutil.h>
> @@ -181,6 +182,8 @@ static void speedupcdma_post_online(struct ofono_modem *modem)
>  
>  	DBG("%p", modem);
>  
> +	ofono_cdma_netreg_create(modem, 0, "cdmamodem", data->chat);
> +
>  	ofono_cdma_connman_create(modem, 0, "cdmamodem", data->chat);

earlier you mentioned that the Speed Up modems have more than one TTY. I
don't think it is a good idea to run the netreg handling on the same TTY
where we will eventually establish the PPP connection.

Regards

Marcel



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH_v2 2/3] cdmamodem: Create network registration atom.
  2011-07-23 14:59   ` Marcel Holtmann
@ 2011-07-23 15:11     ` Denis Kenzior
  2011-07-23 15:15       ` Aygon, Bertrand
  0 siblings, 1 reply; 10+ messages in thread
From: Denis Kenzior @ 2011-07-23 15:11 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 1920 bytes --]

Hi Marcel,

<snip>

>> +#include "gatchat.h"
>> +#include "gatresult.h"
>> +
>> +#include "common.h"
> 
> I know that you are re-using the GSM network registration constants here
> and thus need common.h.
> 
> Denis, is this acceptable for you?
> 

No, not really.  Since CDMA has no defined standard API these should
probably be an enum inside cdma-netreg.h

<snip>

>> +static void sysinfo_cb(gboolean ok, GAtResult *result, gpointer user_data)
>> +{
>> +	struct ofono_cdma_netreg *netreg = user_data;
>> +	gint service_state;
>> +	gint service_domain;
>> +	gint roaming_state;
>> +	int status;
>> +	GAtResultIter iter;
>> +
>> +	if (!ok)
>> +		return;
>> +
>> +	g_at_result_iter_init(&iter, result);
>> +
>> +	if (!g_at_result_iter_next(&iter, "^SYSINFO:"))
>> +		return;
>> +
>> +	if (!g_at_result_iter_next_number(&iter, &service_state))
>> +		return;
>> +
>> +	if (!g_at_result_iter_next_number(&iter, &service_domain))
>> +		return;
>> +
>> +	if (!g_at_result_iter_next_number(&iter, &roaming_state))
>> +		return;
>> +
>> +	switch (service_state) {
>> +	case 0:
>> +	case 4:
> 
> Please put at least comments behind every case label so that we know
> what it does mean.
> 
>> +		status = NETWORK_REGISTRATION_STATUS_NOT_REGISTERED;
>> +		break;
>> +	case 2:
>> +		if (roaming_state)
>> +			status = NETWORK_REGISTRATION_STATUS_ROAMING;
>> +		else
>> +			status = NETWORK_REGISTRATION_STATUS_REGISTERED;
>> +		break;
>> +	default:
>> +		status = NETWORK_REGISTRATION_STATUS_UNKNOWN;
>> +		break;
>> +	}
>> +
>> +	ofono_cdma_netreg_register(netreg);
>> +
>> +	ofono_cdma_netreg_status_notify(netreg, status);
>> +}
> 
> How do we get unsolicited notification when the status changes?
> 

And why are we using a Huawei specific command inside the generic
cdmamodem driver without at least a vendor if/switch somewhere?

Regards,
-Denis

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH_v2 2/3] cdmamodem: Create network registration atom.
  2011-07-23 15:11     ` Denis Kenzior
@ 2011-07-23 15:15       ` Aygon, Bertrand
  2011-07-23 15:23         ` Denis Kenzior
  0 siblings, 1 reply; 10+ messages in thread
From: Aygon, Bertrand @ 2011-07-23 15:15 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

Hi Denis,


<<snip>>
 
> And why are we using a Huawei specific command inside the generic
> cdmamodem driver without at least a vendor if/switch somewhere?

We have SpeedUp dongle that also support this command...

I know this is not the best answer, but there is no standard for CDMA, so many customer seems to se the same AT command.

Regards,

Bertrand
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris, 
92196 Meudon Cedex, France
Registration Number:  302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH_v2 2/3] cdmamodem: Create network registration atom.
  2011-07-23 15:15       ` Aygon, Bertrand
@ 2011-07-23 15:23         ` Denis Kenzior
  0 siblings, 0 replies; 10+ messages in thread
From: Denis Kenzior @ 2011-07-23 15:23 UTC (permalink / raw)
  To: ofono

[-- Attachment #1: Type: text/plain, Size: 674 bytes --]

Hi Bertrand,

On 07/23/2011 10:15 AM, Aygon, Bertrand wrote:
> Hi Denis,
> 
> 
> <<snip>>
>  
>> And why are we using a Huawei specific command inside the generic
>> cdmamodem driver without at least a vendor if/switch somewhere?
> 
> We have SpeedUp dongle that also support this command...
> 
> I know this is not the best answer, but there is no standard for CDMA, so many customer seems to se the same AT command.
> 

Just because you saw two manufacturers doing this, it does not mean that
every one of them will, especially since many use qcdm for this info.
This either belongs in huaweicdma directory or behind a vendor quirk.

Regards,
-Denis

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-07-23 15:23 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-23  4:09 [PATCH_v2 0/3] Create CDMA Network Registration atom Bertrand Aygon
2011-07-23  4:09 ` [PATCH_v2 1/3] cdmamodem: Create org.ofono.cdma.NetworkRegistration interface Bertrand Aygon
2011-07-23 14:52   ` Marcel Holtmann
2011-07-23  4:09 ` [PATCH_v2 2/3] cdmamodem: Create network registration atom Bertrand Aygon
2011-07-23 14:59   ` Marcel Holtmann
2011-07-23 15:11     ` Denis Kenzior
2011-07-23 15:15       ` Aygon, Bertrand
2011-07-23 15:23         ` Denis Kenzior
2011-07-23  4:09 ` [PATCH_v2 3/3] speedupcdma: register to cdma " Bertrand Aygon
2011-07-23 15:00   ` Marcel Holtmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.