[PATCH 1/4] Add: Online property to modem

[PATCH 1/4] Add: Online property to modem

[Pekka Pessi]

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

The online/offline state is changed with the new set_online() modem driver
method.

In order to track atoms, there are modem states as follows:
- OFONO_MODEM_STATE_POWER_OFF
- OFONO_MODEM_STATE_PRE_SIM
- OFONO_MODEM_STATE_OFFLINE
- OFONO_MODEM_STATE_ONLINE

Atoms are removed by modem core according to the state. Atoms are added with
driver methods pre_sim(), post_sim() and post_online().
---
 include/modem.h |   12 ++++
 src/modem.c     |  194 +++++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 164 insertions(+), 42 deletions(-)

diff --git a/include/modem.h b/include/modem.h
index d502640..69c7ad2 100644
--- a/include/modem.h
+++ b/include/modem.h
@@ -50,6 +50,8 @@ void ofono_modem_remove(struct ofono_modem *modem);
 void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered);
 ofono_bool_t ofono_modem_get_powered(struct ofono_modem *modem);
 
+ofono_bool_t ofono_modem_get_online(struct ofono_modem *modem);
+
 void ofono_modem_set_name(struct ofono_modem *modem, const char *name);
 
 int ofono_modem_set_string(struct ofono_modem *modem,
@@ -64,6 +66,9 @@ int ofono_modem_set_boolean(struct ofono_modem *modem,
 				const char *key, bool value);
 bool ofono_modem_get_boolean(struct ofono_modem *modem, const char *key);
 
+typedef void (*ofono_modem_online_cb)(const struct ofono_error *error,
+				void *data);
+
 struct ofono_modem_driver {
 	const char *name;
 
@@ -80,11 +85,18 @@ struct ofono_modem_driver {
 	/* Power down device */
 	int (*disable)(struct ofono_modem *modem);
 
+	/* Enable or disable cellular radio */
+	void (*set_online)(struct ofono_modem *modem, ofono_bool_t online,
+		ofono_modem_online_cb callback, void *data);
+
 	/* Populate the atoms available without SIM / Locked SIM */
 	void (*pre_sim)(struct ofono_modem *modem);
 
 	/* Populate the atoms that are available with SIM / Unlocked SIM*/
 	void (*post_sim)(struct ofono_modem *modem);
+
+	/* Populate the atoms available online */
+	void (*post_online)(struct ofono_modem *modem);
 };
 
 int ofono_modem_driver_register(const struct ofono_modem_driver *);
diff --git a/src/modem.c b/src/modem.c
index 04fba15..b52bcb0 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -49,10 +49,17 @@ enum ofono_property_type {
 	OFONO_PROPERTY_TYPE_BOOLEAN,
 };
 
+enum ofono_modem_state {
+	OFONO_MODEM_STATE_POWER_OFF,
+	OFONO_MODEM_STATE_PRE_SIM,
+	OFONO_MODEM_STATE_OFFLINE,
+	OFONO_MODEM_STATE_ONLINE,
+};
+
 struct ofono_modem {
 	char			*path;
+	enum ofono_modem_state   modem_state;
 	GSList			*atoms;
-	GSList			*pre_sim_atoms;
 	struct ofono_watchlist	*atom_watches;
 	GSList			*interface_list;
 	unsigned int		call_ids;
@@ -61,6 +68,8 @@ struct ofono_modem {
 	ofono_bool_t		powered;
 	ofono_bool_t		powered_pending;
 	guint			timeout;
+	ofono_bool_t		online;
+	ofono_bool_t            online_pending;
 	GHashTable		*properties;
 	struct ofono_sim	*sim;
 	unsigned int		sim_watch;
@@ -83,6 +92,7 @@ struct ofono_devinfo {
 
 struct ofono_atom {
 	enum ofono_atom_type type;
+	enum ofono_modem_state modem_state;
 	void (*destruct)(struct ofono_atom *atom);
 	void (*unregister)(struct ofono_atom *atom);
 	void *data;
@@ -160,6 +170,7 @@ struct ofono_atom *__ofono_modem_add_atom(struct ofono_modem *modem,
 	atom = g_new0(struct ofono_atom, 1);
 
 	atom->type = type;
+	atom->modem_state = modem->modem_state;
 	atom->destruct = destruct;
 	atom->data = data;
 	atom->modem = modem;
@@ -275,7 +286,6 @@ struct ofono_atom *__ofono_modem_find_atom(struct ofono_modem *modem,
 		return NULL;
 
 	FIND_ATOM_IN_LIST(modem->atoms)
-	FIND_ATOM_IN_LIST(modem->pre_sim_atoms);
 
 	return NULL;
 }
@@ -301,7 +311,6 @@ void __ofono_modem_foreach_atom(struct ofono_modem *modem,
 		return;
 
 	FOREACH_ATOM_IN_LIST(modem->atoms)
-	FOREACH_ATOM_IN_LIST(modem->pre_sim_atoms)
 }
 
 void __ofono_atom_free(struct ofono_atom *atom)
@@ -309,7 +318,6 @@ void __ofono_atom_free(struct ofono_atom *atom)
 	struct ofono_modem *modem = atom->modem;
 
 	modem->atoms = g_slist_remove(modem->atoms, atom);
-	modem->pre_sim_atoms = g_slist_remove(modem->pre_sim_atoms, atom);
 
 	__ofono_atom_unregister(atom);
 
@@ -319,24 +327,139 @@ void __ofono_atom_free(struct ofono_atom *atom)
 	g_free(atom);
 }
 
-static void remove_all_atoms(GSList **atoms)
+static void flush_atoms(struct ofono_modem *modem,
+			enum ofono_modem_state new_state)
 {
-	GSList *l;
+	GSList *l, *next;
 	struct ofono_atom *atom;
 
-	for (l = *atoms; l; l = l->next) {
+	for (l = modem->atoms; l; l = next) {
 		atom = l->data;
+		next = l->next;
 
-		__ofono_atom_unregister(atom);
+		if (atom->modem_state > new_state)
+			__ofono_atom_free(atom);
+	}
+}
 
-		if (atom->destruct)
-			atom->destruct(atom);
+static void modem_change_state(struct ofono_modem *modem,
+				enum ofono_modem_state new_state)
+{
+	struct ofono_modem_driver const *driver = modem->driver;
+	enum ofono_modem_state old_state = modem->modem_state;
+	ofono_bool_t new_online = new_state == OFONO_MODEM_STATE_ONLINE;
+
+	if (old_state == new_state)
+		return;
 
-		g_free(atom);
+	if (new_online != modem->online) {
+		DBusConnection *conn = ofono_dbus_get_connection();
+		modem->online = new_online;
+		ofono_dbus_signal_property_changed(conn, modem->path,
+					OFONO_MODEM_INTERFACE, "Online",
+					DBUS_TYPE_BOOLEAN, &modem->online);
 	}
 
-	g_slist_free(*atoms);
-	*atoms = NULL;
+	modem->modem_state = new_state;
+
+	if (old_state > new_state)
+		flush_atoms(modem, new_state);
+
+	switch (new_state) {
+	case OFONO_MODEM_STATE_POWER_OFF:
+		modem->call_ids = 0;
+		break;
+
+	case OFONO_MODEM_STATE_PRE_SIM:
+		if (old_state < OFONO_MODEM_STATE_PRE_SIM) {
+			if (driver->pre_sim)
+				driver->pre_sim(modem);
+		} else if (old_state == OFONO_MODEM_STATE_ONLINE) {
+			if (driver->set_online)
+				driver->set_online(modem, 0, NULL, NULL);
+		}
+		break;
+
+	case OFONO_MODEM_STATE_OFFLINE:
+		if (old_state < OFONO_MODEM_STATE_OFFLINE) {
+			if (driver->post_sim)
+				driver->post_sim(modem);
+			__ofono_history_probe_drivers(modem);
+			__ofono_nettime_probe_drivers(modem);
+		}
+		break;
+
+	case OFONO_MODEM_STATE_ONLINE:
+		if (driver->post_online)
+			driver->post_online(modem);
+		break;
+	}
+}
+
+static void set_online_callback(const struct ofono_error *error,
+				void *data)
+{
+	struct ofono_modem *modem = data;
+	DBusMessage *reply = NULL;
+	ofono_bool_t online = modem->online_pending;
+
+	if (error && error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		reply = __ofono_error_failed(modem->pending);
+		online = modem->online;
+	} else if (online && modem->modem_state < OFONO_MODEM_STATE_OFFLINE) {
+		reply = __ofono_error_failed(modem->pending);
+		online = FALSE;
+	} else
+		reply = dbus_message_new_method_return(modem->pending);
+
+	__ofono_dbus_pending_reply(&modem->pending, reply);
+
+	modem->online_pending = online;
+
+	if (modem->online == online)
+		return;
+
+	if (online)
+		modem_change_state(modem, OFONO_MODEM_STATE_ONLINE);
+	else
+		modem_change_state(modem, OFONO_MODEM_STATE_OFFLINE);
+}
+
+static DBusMessage *set_property_online(struct ofono_modem *modem,
+					DBusMessage *msg,
+					DBusMessageIter *var)
+{
+	ofono_bool_t online;
+	const struct ofono_modem_driver *driver = modem->driver;
+
+	if (dbus_message_iter_get_arg_type(var) != DBUS_TYPE_BOOLEAN)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(var, &online);
+
+	if (modem->online == online)
+		return dbus_message_new_method_return(msg);
+
+	if (!driver || !driver->set_online)
+		return __ofono_error_failed(msg);
+
+	if (modem->pending != NULL)
+		return __ofono_error_busy(msg);
+
+	modem->pending = dbus_message_ref(msg);
+	modem->online_pending = online;
+
+	driver->set_online(modem, online, set_online_callback, modem);
+
+	return NULL;
+}
+
+ofono_bool_t ofono_modem_get_online(struct ofono_modem *modem)
+{
+	if (modem == NULL)
+		return FALSE;
+
+	return modem->online;
 }
 
 static DBusMessage *modem_get_properties(DBusConnection *conn,
@@ -361,6 +484,9 @@ static DBusMessage *modem_get_properties(DBusConnection *conn,
 					OFONO_PROPERTIES_ARRAY_SIGNATURE,
 					&dict);
 
+	ofono_dbus_dict_append(&dict, "Online", DBUS_TYPE_BOOLEAN,
+				&modem->online);
+
 	ofono_dbus_dict_append(&dict, "Powered", DBUS_TYPE_BOOLEAN,
 				&modem->powered);
 
@@ -420,11 +546,8 @@ static int set_powered(struct ofono_modem *modem, ofono_bool_t powered)
 		return -EALREADY;
 
 	/* Remove the atoms even if the driver is no longer available */
-	if (powered == FALSE) {
-		remove_all_atoms(&modem->atoms);
-		remove_all_atoms(&modem->pre_sim_atoms);
-		modem->call_ids = 0;
-	}
+	if (powered == FALSE)
+		modem_change_state(modem, OFONO_MODEM_STATE_POWER_OFF);
 
 	modem->powered_pending = powered;
 
@@ -502,6 +625,9 @@ static DBusMessage *modem_set_property(DBusConnection *conn,
 
 	dbus_message_iter_recurse(&iter, &var);
 
+	if (g_str_equal(name, "Online"))
+		return set_property_online(modem, msg, &var);
+
 	if (g_str_equal(name, "Powered") == TRUE) {
 		ofono_bool_t powered;
 		int err;
@@ -535,10 +661,8 @@ static DBusMessage *modem_set_property(DBusConnection *conn,
 						"Powered", DBUS_TYPE_BOOLEAN,
 						&powered);
 
-		if (powered) {
-			if (modem->driver->pre_sim)
-				modem->driver->pre_sim(modem);
-		}
+		if (powered)
+			modem_change_state(modem, OFONO_MODEM_STATE_PRE_SIM);
 
 		return NULL;
 	}
@@ -596,14 +720,10 @@ void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered)
 						"Powered", DBUS_TYPE_BOOLEAN,
 						&dbus_powered);
 
-		if (powered) {
-			if (modem->driver->pre_sim)
-				modem->driver->pre_sim(modem);
-		} else {
-			remove_all_atoms(&modem->atoms);
-			remove_all_atoms(&modem->pre_sim_atoms);
-			modem->call_ids = 0;
-		}
+		if (powered)
+			modem_change_state(modem, OFONO_MODEM_STATE_PRE_SIM);
+		else
+			modem_change_state(modem, OFONO_MODEM_STATE_POWER_OFF);
 	}
 
 	if (powering_down && powered == FALSE) {
@@ -1132,22 +1252,12 @@ static void modem_sim_ready(void *user, enum ofono_sim_state new_state)
 
 	switch (new_state) {
 	case OFONO_SIM_STATE_NOT_PRESENT:
-		if (modem->pre_sim_atoms != NULL)
-			remove_all_atoms(&modem->atoms);
+		modem_change_state(modem, OFONO_MODEM_STATE_PRE_SIM);
 		break;
 	case OFONO_SIM_STATE_INSERTED:
 		break;
 	case OFONO_SIM_STATE_READY:
-		if (modem->pre_sim_atoms == NULL) {
-			modem->pre_sim_atoms = modem->atoms;
-			modem->atoms = NULL;
-		}
-
-		if (modem->driver->post_sim)
-			modem->driver->post_sim(modem);
-
-		__ofono_history_probe_drivers(modem);
-		__ofono_nettime_probe_drivers(modem);
+		modem_change_state(modem, OFONO_MODEM_STATE_OFFLINE);
 	}
 }
 
-- 
1.6.3.3

[PATCH 2/4] Add: test scripts online-modem and offline-modem

[Pekka Pessi]

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

---
 test/offline-modem |   17 +++++++++++++++++
 test/online-modem  |   17 +++++++++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)
 create mode 100755 test/offline-modem
 create mode 100755 test/online-modem

diff --git a/test/offline-modem b/test/offline-modem
new file mode 100755
index 0000000..16325ad
--- /dev/null
+++ b/test/offline-modem
@@ -0,0 +1,17 @@
+#!/usr/bin/python
+
+import dbus, sys
+
+bus = dbus.SystemBus()
+
+if len(sys.argv) == 2:
+	path = sys.argv[1]
+else:
+	manager = dbus.Interface(bus.get_object('org.ofono', '/'),
+			'org.ofono.Manager')
+	properties = manager.GetProperties()
+	path = properties["Modems"][0]
+
+print "Setting modem %s offline..." % path
+modem = dbus.Interface(bus.get_object('org.ofono', path), 'org.ofono.Modem')
+modem.SetProperty("Online", dbus.Boolean(0))
diff --git a/test/online-modem b/test/online-modem
new file mode 100755
index 0000000..db0b9f5
--- /dev/null
+++ b/test/online-modem
@@ -0,0 +1,17 @@
+#!/usr/bin/python
+
+import dbus, sys
+
+bus = dbus.SystemBus()
+
+if len(sys.argv) == 2:
+	path = sys.argv[1]
+else:
+	manager = dbus.Interface(bus.get_object('org.ofono', '/'),
+			'org.ofono.Manager')
+	properties = manager.GetProperties()
+	path = properties["Modems"][0]
+
+print "Setting modem %s online..." % path
+modem = dbus.Interface(bus.get_object('org.ofono', path), 'org.ofono.Modem')
+modem.SetProperty("Online", dbus.Boolean(1))
-- 
1.6.3.3

[PATCH 3/4] Add: set_online and post_online methods to isimodem driver

[Pekka Pessi]

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

---
 drivers/isimodem/isimodem.c |  120 +++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 115 insertions(+), 5 deletions(-)

diff --git a/drivers/isimodem/isimodem.c b/drivers/isimodem/isimodem.c
index 4085a55..7986540 100644
--- a/drivers/isimodem/isimodem.c
+++ b/drivers/isimodem/isimodem.c
@@ -68,6 +68,11 @@ struct isi_data {
 	unsigned interval;
 	int reported;
 	int mtc_state;
+	int online;
+	struct {
+		ofono_modem_online_cb callback;
+		void *data;
+	} online_request;
 };
 
 static void report_powered(struct isi_data *isi, ofono_bool_t powered)
@@ -76,9 +81,33 @@ static void report_powered(struct isi_data *isi, ofono_bool_t powered)
 		ofono_modem_set_powered(isi->modem, isi->reported = powered);
 }
 
-static void set_power_by_mtc_state(struct isi_data *isi, int state)
+static void report_online(struct isi_data *isi, uint8_t mtc_state)
 {
-	switch (isi->mtc_state = state) {
+	ofono_modem_online_cb cb = isi->online_request.callback;
+	struct ofono_error error = {
+		(isi->online
+			? mtc_state == MTC_NORMAL
+			: mtc_state != MTC_NORMAL)
+		? OFONO_ERROR_TYPE_NO_ERROR
+		: OFONO_ERROR_TYPE_FAILURE,
+	};
+	void *data = isi->online_request.data;
+
+	isi->online_request.callback = 0;
+	isi->online_request.data = 0;
+
+	if (cb)
+		cb(&error, data);
+}
+
+
+static void set_power_by_mtc_state(struct isi_data *isi, int mtc_state)
+{
+	isi->mtc_state = mtc_state;
+
+	report_online(isi, mtc_state);
+
+	switch (mtc_state) {
 	case MTC_STATE_NONE:
 	case MTC_POWER_OFF:
 	case MTC_CHARGING:
@@ -87,6 +116,9 @@ static void set_power_by_mtc_state(struct isi_data *isi, int state)
 		break;
 
 	case MTC_RF_INACTIVE:
+		report_powered(isi, 1);
+		break;
+
 	case MTC_NORMAL:
 		report_powered(isi, 1);
 		break;
@@ -96,6 +128,7 @@ static void set_power_by_mtc_state(struct isi_data *isi, int state)
 	}
 }
 
+
 static void mtc_state_ind_cb(GIsiClient *client, const void *restrict data,
 				size_t len, uint16_t object, void *opaque)
 {
@@ -155,7 +188,8 @@ static bool mtc_poll_query_cb(GIsiClient *client, const void *restrict data,
 	DBG("target modem state: %s (0x%02X)",
 		mtc_modem_state_name(msg[2]), msg[2]);
 
-	set_power_by_mtc_state(isi, msg[1]);
+	if (msg[1] == msg[2])
+		set_power_by_mtc_state(isi, msg[1]);
 
 	return true;
 }
@@ -179,7 +213,8 @@ static bool mtc_query_cb(GIsiClient *client, const void *restrict data,
 	DBG("target modem state: %s (0x%02X)",
 		mtc_modem_state_name(msg[2]), msg[2]);
 
-	set_power_by_mtc_state(isi, msg[1]);
+	if (msg[1] == msg[2])
+		set_power_by_mtc_state(isi, msg[1]);
 
 	return true;
 }
@@ -276,6 +311,7 @@ static int isi_modem_probe(struct ofono_modem *modem)
 	isi->ifname = ifname;
 	isi->link = link;
 	isi->client = g_isi_client_create(isi->idx, PN_MTC);
+	isi->reported = -1;
 
 	return 0;
 }
@@ -292,6 +328,70 @@ static void isi_modem_remove(struct ofono_modem *modem)
 	g_free(isi);
 }
 
+static bool mtc_state_cb(GIsiClient *client, const void *restrict data,
+			size_t len, uint16_t object, void *opaque)
+{
+	struct isi_data *isi = opaque;
+	const unsigned char *msg = data;
+
+	if (!msg) {
+		DBG("ISI client error: %d", g_isi_client_error(client));
+		goto err;
+	}
+
+	if (len < 3 || msg[0] != MTC_STATE_RESP)
+		return false;
+	DBG("cause: %s (0x%02X)", mtc_isi_cause_name(msg[1]), msg[1]);
+	if (msg[1] == MTC_OK)
+		return true;
+
+err:
+	if (isi->online_request.callback) {
+		ofono_modem_online_cb cb = isi->online_request.callback;
+		struct ofono_error error = {
+			msg && msg[1] == MTC_ALREADY_ACTIVE
+			? OFONO_ERROR_TYPE_NO_ERROR
+			: OFONO_ERROR_TYPE_FAILURE,
+		};
+		void *cbdata = isi->online_request.data;
+		isi->online_request.callback = 0;
+		isi->online_request.data = 0;
+		cb(&error, cbdata);
+	}
+
+	return true;
+}
+
+static int mtc_state_req(struct isi_data *isi, uint8_t state)
+{
+	const unsigned char req[] = {
+		MTC_STATE_REQ, state, 0x00
+	};
+
+	if (g_isi_request_make(isi->client, req, sizeof(req), MTC_TIMEOUT,
+					mtc_state_cb, isi))
+		return 0;
+	else
+		return -errno;
+}
+
+static void isi_modem_online(struct ofono_modem *modem, ofono_bool_t online,
+			ofono_modem_online_cb callback, void *data)
+{
+	struct isi_data *isi = ofono_modem_get_data(modem);
+	int err;
+
+	DBG("(%p) with %s", modem, isi->ifname);
+	isi->online = online;
+	isi->online_request.callback = callback;
+	isi->online_request.data = data;
+
+	err = mtc_state_req(isi, online ? MTC_NORMAL : MTC_RF_INACTIVE);
+
+	if (err)
+		report_online(isi, MTC_STATE_NONE);
+}
+
 static void isi_modem_pre_sim(struct ofono_modem *modem)
 {
 	struct isi_data *isi = ofono_modem_get_data(modem);
@@ -306,12 +406,20 @@ static void isi_modem_pre_sim(struct ofono_modem *modem)
 static void isi_modem_post_sim(struct ofono_modem *modem)
 {
 	struct isi_data *isi = ofono_modem_get_data(modem);
+
+	DBG("(%p) with %s", modem, isi->ifname);
+
+	ofono_phonebook_create(isi->modem, 0, "isimodem", isi->idx);
+}
+
+static void isi_modem_post_online(struct ofono_modem *modem)
+{
+	struct isi_data *isi = ofono_modem_get_data(modem);
 	struct ofono_gprs *gprs;
 	struct ofono_gprs_context *gc;
 
 	DBG("(%p) with %s", modem, isi->ifname);
 
-	ofono_phonebook_create(isi->modem, 0, "isimodem", isi->idx);
 	ofono_netreg_create(isi->modem, 0, "isimodem", isi->idx);
 	ofono_sms_create(isi->modem, 0, "isimodem", isi->idx);
 	ofono_cbs_create(isi->modem, 0, "isimodem", isi->idx);
@@ -335,8 +443,10 @@ static struct ofono_modem_driver driver = {
 	.name = "isimodem",
 	.probe = isi_modem_probe,
 	.remove = isi_modem_remove,
+	.set_online = isi_modem_online,
 	.pre_sim = isi_modem_pre_sim,
 	.post_sim = isi_modem_post_sim,
+	.post_online = isi_modem_post_online,
 };
 
 static int isimodem_init(void)
-- 
1.6.3.3

[PATCH 4/4] Add description of Online property to modem-api.txt

[Pekka Pessi]

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

---
 doc/modem-api.txt |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/doc/modem-api.txt b/doc/modem-api.txt
index 7cccd57..4e0525a 100644
--- a/doc/modem-api.txt
+++ b/doc/modem-api.txt
@@ -32,6 +32,11 @@ Properties	boolean Powered [readwrite]
 			Boolean representing the power state of the modem
 			device.
 
+		boolean Online [readwrite]
+
+			Boolean representing the rf state of the modem.
+			Online is false in flight mode.
+
 		string Name [readonly, optional]
 
 			Friendly name of the modem device.
-- 
1.6.3.3

Re: [PATCH 4/4] Add description of Online property to modem-api.txt

[Denis Kenzior]

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

Hi Pekka,

> ---
>  doc/modem-api.txt |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)

Patch has been applied, thanks.

Regards,
-Denis

Re: [PATCH 2/4] Add: test scripts online-modem and offline-modem

[Denis Kenzior]

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

Hi Pekka,

> ---
>  test/offline-modem |   17 +++++++++++++++++
>  test/online-modem  |   17 +++++++++++++++++
>  2 files changed, 34 insertions(+), 0 deletions(-)
>  create mode 100755 test/offline-modem
>  create mode 100755 test/online-modem

Patch has been applied, thanks.

Regards,
-Denis

[PATCH 1/4] Add: Online property to modem

[Pekka Pessi]

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

The online/offline state is changed with the new set_online() modem driver
method.

In order to track atoms, there are modem states as follows:
- OFONO_MODEM_STATE_POWER_OFF
- OFONO_MODEM_STATE_PRE_SIM
- OFONO_MODEM_STATE_OFFLINE
- OFONO_MODEM_STATE_ONLINE

Atoms are added with driver methods pre_sim(), post_sim() and post_online().
Atoms are removed by modem core when the state changes.
---
 include/modem.h |   12 +++
 src/modem.c     |  200 +++++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 170 insertions(+), 42 deletions(-)

diff --git a/include/modem.h b/include/modem.h
index d502640..69c7ad2 100644
--- a/include/modem.h
+++ b/include/modem.h
@@ -50,6 +50,8 @@ void ofono_modem_remove(struct ofono_modem *modem);
 void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered);
 ofono_bool_t ofono_modem_get_powered(struct ofono_modem *modem);
 
+ofono_bool_t ofono_modem_get_online(struct ofono_modem *modem);
+
 void ofono_modem_set_name(struct ofono_modem *modem, const char *name);
 
 int ofono_modem_set_string(struct ofono_modem *modem,
@@ -64,6 +66,9 @@ int ofono_modem_set_boolean(struct ofono_modem *modem,
 				const char *key, bool value);
 bool ofono_modem_get_boolean(struct ofono_modem *modem, const char *key);
 
+typedef void (*ofono_modem_online_cb)(const struct ofono_error *error,
+				void *data);
+
 struct ofono_modem_driver {
 	const char *name;
 
@@ -80,11 +85,18 @@ struct ofono_modem_driver {
 	/* Power down device */
 	int (*disable)(struct ofono_modem *modem);
 
+	/* Enable or disable cellular radio */
+	void (*set_online)(struct ofono_modem *modem, ofono_bool_t online,
+		ofono_modem_online_cb callback, void *data);
+
 	/* Populate the atoms available without SIM / Locked SIM */
 	void (*pre_sim)(struct ofono_modem *modem);
 
 	/* Populate the atoms that are available with SIM / Unlocked SIM*/
 	void (*post_sim)(struct ofono_modem *modem);
+
+	/* Populate the atoms available online */
+	void (*post_online)(struct ofono_modem *modem);
 };
 
 int ofono_modem_driver_register(const struct ofono_modem_driver *);
diff --git a/src/modem.c b/src/modem.c
index 04fba15..403ce5d 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -49,10 +49,17 @@ enum ofono_property_type {
 	OFONO_PROPERTY_TYPE_BOOLEAN,
 };
 
+enum ofono_modem_state {
+	OFONO_MODEM_STATE_POWER_OFF,
+	OFONO_MODEM_STATE_PRE_SIM,
+	OFONO_MODEM_STATE_OFFLINE,
+	OFONO_MODEM_STATE_ONLINE,
+};
+
 struct ofono_modem {
 	char			*path;
+	enum ofono_modem_state   modem_state;
 	GSList			*atoms;
-	GSList			*pre_sim_atoms;
 	struct ofono_watchlist	*atom_watches;
 	GSList			*interface_list;
 	unsigned int		call_ids;
@@ -61,6 +68,8 @@ struct ofono_modem {
 	ofono_bool_t		powered;
 	ofono_bool_t		powered_pending;
 	guint			timeout;
+	ofono_bool_t		online;
+	ofono_bool_t            online_pending;
 	GHashTable		*properties;
 	struct ofono_sim	*sim;
 	unsigned int		sim_watch;
@@ -83,6 +92,7 @@ struct ofono_devinfo {
 
 struct ofono_atom {
 	enum ofono_atom_type type;
+	enum ofono_modem_state modem_state;
 	void (*destruct)(struct ofono_atom *atom);
 	void (*unregister)(struct ofono_atom *atom);
 	void *data;
@@ -160,6 +170,7 @@ struct ofono_atom *__ofono_modem_add_atom(struct ofono_modem *modem,
 	atom = g_new0(struct ofono_atom, 1);
 
 	atom->type = type;
+	atom->modem_state = modem->modem_state;
 	atom->destruct = destruct;
 	atom->data = data;
 	atom->modem = modem;
@@ -275,7 +286,6 @@ struct ofono_atom *__ofono_modem_find_atom(struct ofono_modem *modem,
 		return NULL;
 
 	FIND_ATOM_IN_LIST(modem->atoms)
-	FIND_ATOM_IN_LIST(modem->pre_sim_atoms);
 
 	return NULL;
 }
@@ -301,7 +311,6 @@ void __ofono_modem_foreach_atom(struct ofono_modem *modem,
 		return;
 
 	FOREACH_ATOM_IN_LIST(modem->atoms)
-	FOREACH_ATOM_IN_LIST(modem->pre_sim_atoms)
 }
 
 void __ofono_atom_free(struct ofono_atom *atom)
@@ -309,7 +318,6 @@ void __ofono_atom_free(struct ofono_atom *atom)
 	struct ofono_modem *modem = atom->modem;
 
 	modem->atoms = g_slist_remove(modem->atoms, atom);
-	modem->pre_sim_atoms = g_slist_remove(modem->pre_sim_atoms, atom);
 
 	__ofono_atom_unregister(atom);
 
@@ -319,24 +327,145 @@ void __ofono_atom_free(struct ofono_atom *atom)
 	g_free(atom);
 }
 
-static void remove_all_atoms(GSList **atoms)
+static void flush_atoms(struct ofono_modem *modem,
+			enum ofono_modem_state new_state)
 {
-	GSList *l;
+	GSList *l, *next;
 	struct ofono_atom *atom;
 
-	for (l = *atoms; l; l = l->next) {
+	for (l = modem->atoms; l; l = next) {
 		atom = l->data;
+		next = l->next;
+
+		if (atom->modem_state > new_state)
+			__ofono_atom_free(atom);
+	}
+}
+
+static void dummy_online_callback(const struct ofono_error *error,
+				void *data)
+{
+}
 
-		__ofono_atom_unregister(atom);
+static void modem_change_state(struct ofono_modem *modem,
+				enum ofono_modem_state new_state)
+{
+	struct ofono_modem_driver const *driver = modem->driver;
+	enum ofono_modem_state old_state = modem->modem_state;
+	ofono_bool_t new_online = new_state == OFONO_MODEM_STATE_ONLINE;
 
-		if (atom->destruct)
-			atom->destruct(atom);
+	if (old_state == new_state)
+		return;
 
-		g_free(atom);
+	if (new_online != modem->online) {
+		DBusConnection *conn = ofono_dbus_get_connection();
+		modem->online = new_online;
+		ofono_dbus_signal_property_changed(conn, modem->path,
+					OFONO_MODEM_INTERFACE, "Online",
+					DBUS_TYPE_BOOLEAN, &modem->online);
 	}
 
-	g_slist_free(*atoms);
-	*atoms = NULL;
+	modem->modem_state = new_state;
+
+	if (old_state > new_state)
+		flush_atoms(modem, new_state);
+
+	switch (new_state) {
+	case OFONO_MODEM_STATE_POWER_OFF:
+		modem->call_ids = 0;
+		break;
+
+	case OFONO_MODEM_STATE_PRE_SIM:
+		if (old_state < OFONO_MODEM_STATE_PRE_SIM) {
+			if (driver->pre_sim)
+				driver->pre_sim(modem);
+		} else if (old_state == OFONO_MODEM_STATE_ONLINE) {
+			if (driver->set_online)
+				driver->set_online(modem, 0,
+						dummy_online_callback, modem);
+		}
+		break;
+
+	case OFONO_MODEM_STATE_OFFLINE:
+		if (old_state < OFONO_MODEM_STATE_OFFLINE) {
+			if (driver->post_sim)
+				driver->post_sim(modem);
+			__ofono_history_probe_drivers(modem);
+			__ofono_nettime_probe_drivers(modem);
+		}
+		break;
+
+	case OFONO_MODEM_STATE_ONLINE:
+		if (driver->post_online)
+			driver->post_online(modem);
+		break;
+	}
+}
+
+static void set_online_callback(const struct ofono_error *error,
+				void *data)
+{
+	struct ofono_modem *modem = data;
+	DBusMessage *reply = NULL;
+	ofono_bool_t online = modem->online_pending;
+
+	if (error && error->type != OFONO_ERROR_TYPE_NO_ERROR) {
+		reply = __ofono_error_failed(modem->pending);
+		online = modem->online;
+	} else if (online && modem->modem_state < OFONO_MODEM_STATE_OFFLINE) {
+		reply = __ofono_error_failed(modem->pending);
+		online = FALSE;
+	} else
+		reply = dbus_message_new_method_return(modem->pending);
+
+	__ofono_dbus_pending_reply(&modem->pending, reply);
+
+	modem->online_pending = online;
+
+	if (modem->online == online)
+		return;
+
+	if (online)
+		modem_change_state(modem, OFONO_MODEM_STATE_ONLINE);
+	else
+		modem_change_state(modem, OFONO_MODEM_STATE_OFFLINE);
+}
+
+static DBusMessage *set_property_online(struct ofono_modem *modem,
+					DBusMessage *msg,
+					DBusMessageIter *var)
+{
+	ofono_bool_t online;
+	const struct ofono_modem_driver *driver = modem->driver;
+
+	if (dbus_message_iter_get_arg_type(var) != DBUS_TYPE_BOOLEAN)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(var, &online);
+
+	if (modem->online == online)
+		return dbus_message_new_method_return(msg);
+
+	if (!driver || !driver->set_online)
+		return __ofono_error_failed(msg);
+
+	if (modem->pending != NULL)
+		return __ofono_error_busy(msg);
+
+	modem->pending = dbus_message_ref(msg);
+	modem->online_pending = online;
+
+	driver->set_online(modem, online, set_online_callback, modem);
+
+	return NULL;
+}
+
+ofono_bool_t ofono_modem_get_online(struct ofono_modem *modem)
+{
+	if (modem == NULL)
+		return FALSE;
+
+	return modem->online;
 }
 
 static DBusMessage *modem_get_properties(DBusConnection *conn,
@@ -361,6 +490,9 @@ static DBusMessage *modem_get_properties(DBusConnection *conn,
 					OFONO_PROPERTIES_ARRAY_SIGNATURE,
 					&dict);
 
+	ofono_dbus_dict_append(&dict, "Online", DBUS_TYPE_BOOLEAN,
+				&modem->online);
+
 	ofono_dbus_dict_append(&dict, "Powered", DBUS_TYPE_BOOLEAN,
 				&modem->powered);
 
@@ -420,11 +552,8 @@ static int set_powered(struct ofono_modem *modem, ofono_bool_t powered)
 		return -EALREADY;
 
 	/* Remove the atoms even if the driver is no longer available */
-	if (powered == FALSE) {
-		remove_all_atoms(&modem->atoms);
-		remove_all_atoms(&modem->pre_sim_atoms);
-		modem->call_ids = 0;
-	}
+	if (powered == FALSE)
+		modem_change_state(modem, OFONO_MODEM_STATE_POWER_OFF);
 
 	modem->powered_pending = powered;
 
@@ -502,6 +631,9 @@ static DBusMessage *modem_set_property(DBusConnection *conn,
 
 	dbus_message_iter_recurse(&iter, &var);
 
+	if (g_str_equal(name, "Online"))
+		return set_property_online(modem, msg, &var);
+
 	if (g_str_equal(name, "Powered") == TRUE) {
 		ofono_bool_t powered;
 		int err;
@@ -535,10 +667,8 @@ static DBusMessage *modem_set_property(DBusConnection *conn,
 						"Powered", DBUS_TYPE_BOOLEAN,
 						&powered);
 
-		if (powered) {
-			if (modem->driver->pre_sim)
-				modem->driver->pre_sim(modem);
-		}
+		if (powered)
+			modem_change_state(modem, OFONO_MODEM_STATE_PRE_SIM);
 
 		return NULL;
 	}
@@ -596,14 +726,10 @@ void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered)
 						"Powered", DBUS_TYPE_BOOLEAN,
 						&dbus_powered);
 
-		if (powered) {
-			if (modem->driver->pre_sim)
-				modem->driver->pre_sim(modem);
-		} else {
-			remove_all_atoms(&modem->atoms);
-			remove_all_atoms(&modem->pre_sim_atoms);
-			modem->call_ids = 0;
-		}
+		if (powered)
+			modem_change_state(modem, OFONO_MODEM_STATE_PRE_SIM);
+		else
+			modem_change_state(modem, OFONO_MODEM_STATE_POWER_OFF);
 	}
 
 	if (powering_down && powered == FALSE) {
@@ -1132,22 +1258,12 @@ static void modem_sim_ready(void *user, enum ofono_sim_state new_state)
 
 	switch (new_state) {
 	case OFONO_SIM_STATE_NOT_PRESENT:
-		if (modem->pre_sim_atoms != NULL)
-			remove_all_atoms(&modem->atoms);
+		modem_change_state(modem, OFONO_MODEM_STATE_PRE_SIM);
 		break;
 	case OFONO_SIM_STATE_INSERTED:
 		break;
 	case OFONO_SIM_STATE_READY:
-		if (modem->pre_sim_atoms == NULL) {
-			modem->pre_sim_atoms = modem->atoms;
-			modem->atoms = NULL;
-		}
-
-		if (modem->driver->post_sim)
-			modem->driver->post_sim(modem);
-
-		__ofono_history_probe_drivers(modem);
-		__ofono_nettime_probe_drivers(modem);
+		modem_change_state(modem, OFONO_MODEM_STATE_OFFLINE);
 	}
 }
 
-- 
1.6.3.3

Re: [PATCH 1/4] Add: Online property to modem

[Denis Kenzior]

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

Hi Pekka,

> The online/offline state is changed with the new set_online() modem driver
> method.
> 
> In order to track atoms, there are modem states as follows:
> - OFONO_MODEM_STATE_POWER_OFF
> - OFONO_MODEM_STATE_PRE_SIM
> - OFONO_MODEM_STATE_OFFLINE
> - OFONO_MODEM_STATE_ONLINE
> 
> Atoms are added with driver methods pre_sim(), post_sim() and
>  post_online(). Atoms are removed by modem core when the state changes.

Patch has been applied, however I did some refactoring afterward.  Please test 
that I didn't break something.

Regards,
-Denis

Re: [PATCH 1/4] Add: Online property to modem

[Pekka Pessi]

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

2010/5/27 Denis Kenzior <denkenz@gmail.com>:
> Patch has been applied, however I did some refactoring afterward.  Please test
> that I didn't break something.

There is now a race condition in online/offline and sim hotswap. With
imperfect hw a missing sim gets detected only when modem attempts to
register to network and actually tries to read something from sim
card. I have no idea if the missing sim can be reported before +cops
command response is returned.

However, at least isimodem needs explicitly told to go offline when
sim gets removed.

-- 
Pekka.Pessi mail at nokia.com

Re: [PATCH 1/4] Add: Online property to modem

[Denis Kenzior]

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

Hi Pekka,

> 2010/5/27 Denis Kenzior <denkenz@gmail.com>:
> > Patch has been applied, however I did some refactoring afterward.  Please
> > test that I didn't break something.
> 
> There is now a race condition in online/offline and sim hotswap. With
> imperfect hw a missing sim gets detected only when modem attempts to
> register to network and actually tries to read something from sim
> card. I have no idea if the missing sim can be reported before +cops
> command response is returned.

Sorry, but how does such a modem even get to the sim ready state?  oFono reads 
the IMSI before it even tries to send post_sim, and you certainly can't read 
the IMSI without a sim card.

Or are you talking about an emergency call case here?

> 
> However, at least isimodem needs explicitly told to go offline when
> sim gets removed.
> 

My thinking is that the modem drivers can take care of this.  The core needs 
to be kept simple here and assume an 'ideal' implementation.

I'm still having trouble understanding how we'll support emergency calls with 
no SIM with this framework though.

Regards,
-Denis

Re: [PATCH 1/4] Add: Online property to modem

[Pekka Pessi]

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

2010/5/27 Denis Kenzior <denkenz@gmail.com>:
> Hi Pekka,
>
>> 2010/5/27 Denis Kenzior <denkenz@gmail.com>:
>> > Patch has been applied, however I did some refactoring afterward.  Please
>> > test that I didn't break something.
>>
>> There is now a race condition in online/offline and sim hotswap. With
>> imperfect hw a missing sim gets detected only when modem attempts to
>> register to network and actually tries to read something from sim
>> card. I have no idea if the missing sim can be reported before +cops
>> command response is returned.
>
> Sorry, but how does such a modem even get to the sim ready state?  oFono reads
> the IMSI before it even tries to send post_sim, and you certainly can't read
> the IMSI without a sim card.

If you have a SIM card and then remove it, modem should go to pre_sim
state (and the expectation is that it will go offline, too.)  However,
unless explicitly told otherwise, isimodem goes to limited service
mode.

>> However, at least isimodem needs explicitly told to go offline when
>> sim gets removed.
>>
>
> My thinking is that the modem drivers can take care of this.  The core needs
> to be kept simple here and assume an 'ideal' implementation.
>
> I'm still having trouble understanding how we'll support emergency calls with
> no SIM with this framework though.

The online/offline cuts in only if you have SIM card (and PIN is
entered/disabled).

-- 
Pekka.Pessi mail at nokia.com

Re: [PATCH 1/4] Add: Online property to modem

[Denis Kenzior]

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

Hi Pekka,

> 2010/5/27 Denis Kenzior <denkenz@gmail.com>:
> > Hi Pekka,
> >
> >> 2010/5/27 Denis Kenzior <denkenz@gmail.com>:
> >> > Patch has been applied, however I did some refactoring afterward.
> >> >  Please test that I didn't break something.
> >>
> >> There is now a race condition in online/offline and sim hotswap. With
> >> imperfect hw a missing sim gets detected only when modem attempts to
> >> register to network and actually tries to read something from sim
> >> card. I have no idea if the missing sim can be reported before +cops
> >> command response is returned.
> >
> > Sorry, but how does such a modem even get to the sim ready state?  oFono
> > reads the IMSI before it even tries to send post_sim, and you certainly
> > can't read the IMSI without a sim card.
> 
> If you have a SIM card and then remove it, modem should go to pre_sim
> state (and the expectation is that it will go offline, too.)  However,
> unless explicitly told otherwise, isimodem goes to limited service
> mode.

Still having trouble seeing your concerns.  oFono core transitions the modem 
to the pre_sim state.  If a modem requires special steps to be taken to be in 
sync with the core, then the driver has to take care of it.

> 
> >> However, at least isimodem needs explicitly told to go offline when
> >> sim gets removed.
> >
> > My thinking is that the modem drivers can take care of this.  The core
> > needs to be kept simple here and assume an 'ideal' implementation.
> >
> > I'm still having trouble understanding how we'll support emergency calls
> > with no SIM with this framework though.
> 
> The online/offline cuts in only if you have SIM card (and PIN is
> entered/disabled).
>

And we rely on the modem to activate rx/tx in the case of an emergency call? 
How does this work with Flight mode?

Regards,
-Denis