Hi Marcel,
On 04/08/2011 18:46, Marcel Holtmann wrote:
Hi Guillaume,
Change log from v4:
- Check if modem name is sent by oFono
- CDMA modem name is used as network name
- Change modem_data property from is_cdma into has_cdma
plugins/ofono.c | 156 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 151 insertions(+), 5 deletions(-)
diff --git a/plugins/ofono.c b/plugins/ofono.c
index 8650bfc..88a1781 100644
--- a/plugins/ofono.c
+++ b/plugins/ofono.c
@@ -50,6 +50,7 @@
#define OFONO_CONTEXT_INTERFACE OFONO_SERVICE
".ConnectionContext"
#define OFONO_SIM_INTERFACE OFONO_SERVICE ".SimManager"
#define OFONO_REGISTRATION_INTERFACE OFONO_SERVICE ".NetworkRegistration"
+#define OFONO_CDMA_INTERFACE OFONO_SERVICE ".cdma.ConnectionManager"
#define PROPERTY_CHANGED "PropertyChanged"
#define GET_PROPERTIES "GetProperties"
@@ -76,6 +77,7 @@ struct modem_data {
gboolean has_sim;
gboolean has_reg;
gboolean has_gprs;
+ gboolean has_cdma;
gboolean available;
gboolean pending_online;
dbus_bool_t requested_online;
@@ -337,6 +339,7 @@ static struct connman_device_driver modem_driver = {
static void remove_device_networks(struct connman_device *device)
{
+ struct modem_data *modem = connman_device_get_data(device);
GHashTableIter iter;
gpointer key, value;
GSList *info_list = NULL;
@@ -362,6 +365,11 @@ static void remove_device_networks(struct connman_device
*device)
connman_device_remove_network(device, info->network);
}
+ if (modem->has_cdma == TRUE) {
+ g_hash_table_remove(network_hash, "/context1");
+ modem->has_cdma = FALSE;
+ }
+
g_slist_free(info_list);
}
I do not like this "/context1" stuff at all. What is wrong with just
using the modem object path that you get from oFono here. I do not
remember that devices and networks have to have distinct object paths. I
think that limitation is gone since a long time.
The drawback of this solution is:
When you need to save APN Username and Password that you entered, if you
use the
"/default/profile/cellular_IMEI_modem_path" path to save them for cdma,
when you will
plug/unplug the modem multiple times, your modem name will increase from
cdma0 to
cdma5...
Then you will create multiple references to the same configuration but
you won't be able
to find again the APN Username and Password.
What would you suggest to be used as key to save APN Username Password then?
As the 2 following comments depend of this, I will wait for your answer
:) to apply or
not the needed changes
@@ -541,25 +549,57 @@ done:
static int set_network_active(struct connman_network *network)
{
+ struct connman_device *device;
+ struct modem_data *modem;
dbus_bool_t value = TRUE;
const char *path = connman_network_get_string(network, "Path");
DBG("network %p, path %s", network, path);
- return set_property(path, OFONO_CONTEXT_INTERFACE,
- "Active", DBUS_TYPE_BOOLEAN,&value,
- set_active_reply, g_strdup(path), g_free);
+ device = connman_network_get_device(network);
+ if (device == NULL)
+ return -ENODEV;
+
+ modem = connman_device_get_data(device);
+ if (modem == NULL)
+ return -ENODEV;
+
+ if (modem->has_cdma == TRUE)
+ return set_property(modem->path, OFONO_CDMA_INTERFACE,
+ "Powered", DBUS_TYPE_BOOLEAN,&value,
+ set_active_reply, g_strdup(modem->path),
+ g_free);
+ else
+ return set_property(path, OFONO_CONTEXT_INTERFACE,
+ "Active", DBUS_TYPE_BOOLEAN,&value,
+ set_active_reply,
+ g_strdup(path), g_free);
}
the Path should be set properly here. So I prefer that a CDMA network
object looks similar to a GSM network object.
Dragging the device object into this here seems wrong.
static int set_network_inactive(struct connman_network *network)
{
+ struct connman_device *device;
+ struct modem_data *modem;
int err;
dbus_bool_t value = FALSE;
const char *path = connman_network_get_string(network, "Path");
DBG("network %p, path %s", network, path);
- err = set_property(path, OFONO_CONTEXT_INTERFACE,
+ device = connman_network_get_device(network);
+ if (device == NULL)
+ return -ENODEV;
+
+ modem = connman_device_get_data(device);
+ if (modem == NULL)
+ return -ENODEV;
+
+ if (modem->has_cdma == TRUE)
+ err = set_property(modem->path, OFONO_CDMA_INTERFACE,
+ "Powered", DBUS_TYPE_BOOLEAN,&value,
+ NULL, NULL, NULL);
+ else
+ err = set_property(path, OFONO_CONTEXT_INTERFACE,
"Active", DBUS_TYPE_BOOLEAN,&value,
NULL, NULL, NULL);
Same comments as above.
@@ -584,6 +624,9 @@ static int network_connect(struct connman_network *network)
if (modem == NULL)
return -ENODEV;
+ if (modem->has_cdma == TRUE)
+ return set_network_active(network);
+
if (modem->registered == FALSE)
return -ENOLINK;
@@ -1464,6 +1507,12 @@ static gboolean modem_has_gprs(DBusMessageIter *array)
return modem_has_interface(array, OFONO_GPRS_INTERFACE);
}
+static gboolean modem_has_cdma(DBusMessageIter *array)
+{
+ return modem_has_interface(array,
+ OFONO_CDMA_INTERFACE);
+}
+
static void add_modem(const char *path, DBusMessageIter *prop)
{
struct modem_data *modem;
@@ -1473,6 +1522,7 @@ static void add_modem(const char *path, DBusMessageIter
*prop)
gboolean has_sim = FALSE;
gboolean has_reg = FALSE;
gboolean has_gprs = FALSE;
+ gboolean has_cdma = FALSE;
modem = g_hash_table_lookup(modem_hash, path);
@@ -1486,6 +1536,7 @@ static void add_modem(const char *path, DBusMessageIter
*prop)
modem->path = g_strdup(path);
modem->device = NULL;
modem->available = TRUE;
+ modem->operator = NULL;
g_hash_table_insert(modem_hash, g_strdup(path), modem);
@@ -1507,6 +1558,7 @@ static void add_modem(const char *path, DBusMessageIter
*prop)
has_sim = modem_has_sim(&value);
has_reg = modem_has_reg(&value);
has_gprs = modem_has_gprs(&value);
+ has_cdma = modem_has_cdma(&value);
}
dbus_message_iter_next(prop);
@@ -1521,6 +1573,7 @@ static void add_modem(const char *path, DBusMessageIter
*prop)
modem->has_sim = has_sim;
modem->has_reg = has_reg;
modem->has_gprs = has_gprs;
+ modem->has_cdma = has_cdma;
update_modem_online(modem, online);
@@ -1658,10 +1711,27 @@ static gboolean modem_changed(DBusConnection
*connection, DBusMessage *message,
gboolean had_reg = modem->has_reg;
gboolean has_gprs = modem_has_gprs(&value);
gboolean had_gprs = modem->has_gprs;
+ gboolean has_cdma = modem_has_cdma(&value);
+ gboolean had_cdma = modem->has_cdma;
modem->has_sim = has_sim;
modem->has_reg = has_reg;
modem->has_gprs = has_gprs;
+ modem->has_cdma = has_cdma;
+
+ if (has_cdma) {
+ if (modem->device == NULL)
+ return TRUE;
+
+ add_network(modem->device, "/context1", NULL);
+
+ return TRUE;
+ } else {
+ if (has_cdma == FALSE&& had_cdma == TRUE) {
+ modem->has_cdma = had_cdma;
+ modem_remove_device(modem);
+ }
+ }
This is horrible :(
First of all, the second has_cdma check is implied anyway. And second,
why so complicated. This can be done surely a lot simpler.
I will rework this.
if (modem->device == NULL) {
if (has_sim)
@@ -1679,6 +1749,25 @@ static gboolean modem_changed(DBusConnection
*connection, DBusMessage *message,
check_gprs(modem);
}
}
+ } else if (g_str_equal(key, "Serial") == TRUE) {
+ const char *hw_serial;
+
+ dbus_message_iter_get_basic(&value,&hw_serial);
+
+ add_device(modem->path, hw_serial);
This breaks GSM. Have you actually tested that this does not break
anything. Now in the GSM case, you just add device with the IMEI here.
Yes, I forgot this, I will check if modem has_cdma to fix it.
+
+ if (modem->has_cdma == TRUE)
+ add_network(modem->device, "/context1", NULL);
+ } else if (g_str_equal(key, "Model") == TRUE) {
+ const char *model;
+
+ dbus_message_iter_get_basic(&value,&model);
+
+ if (modem->has_cdma == TRUE)
+ modem->operator = g_strdup(model);
+
+ if (modem->device != NULL&& modem->has_cdma == TRUE)
+ add_network(modem->device, "/context1", NULL);
}
So what happens if Model comes before Serial property?
If Model comes before Serial, then add_device() won't have been called,
so we won't add_network() because device is NULL.
return TRUE;
@@ -1888,6 +1977,55 @@ static gboolean context_changed(DBusConnection
*connection,
return TRUE;
}
+static gboolean cdma_changed(DBusConnection *connection,
+ DBusMessage *message, void *user_data)
+{
+ struct network_info *info;
+ DBusMessageIter iter, value;
+ const char *key;
+
+ DBG("");
+
+ info = g_hash_table_lookup(network_hash, "/context1");
+ if (info == NULL)
+ return TRUE;
+
+ if (!pending_network_is_available(info->network)) {
+ g_hash_table_remove(network_hash, "/context1");
+ return TRUE;
+ }
+
+ connman_network_set_associating(info->network, FALSE);
+
+ if (dbus_message_iter_init(message,&iter) == FALSE)
+ return TRUE;
+
+ dbus_message_iter_get_basic(&iter,&key);
+
+ dbus_message_iter_next(&iter);
+ dbus_message_iter_recurse(&iter,&value);
+
+ DBG("key %s", key);
+
+ if (g_str_equal(key, "Settings") == TRUE)
+ update_ipv4_settings(&value, info);
+ else if (g_str_equal(key, "Powered") == TRUE) {
+ dbus_bool_t active;
+
+ dbus_message_iter_get_basic(&value,&active);
+
+ if (active == FALSE)
+ set_connected(info, active);
+
+ /* Connect only if requested to do so */
+ if (active == TRUE&&
+ connman_network_get_connecting(info->network) == TRUE)
+ set_connected(info, active);
Actually "... else if ..." statement exists for a reason ;)
Right, I copied/pasted it from context_changed() function so I will fix
also this one :)
+ }
+
+ return TRUE;
+}
+
static guint watch;
static guint reg_watch;
static guint sim_watch;
@@ -1898,6 +2036,7 @@ static guint modem_watch;
static guint modem_added_watch;
static guint modem_removed_watch;
static guint context_watch;
+static guint cdma_watch;
static int ofono_init(void)
{
@@ -1964,11 +2103,17 @@ static int ofono_init(void)
context_changed,
NULL, NULL);
+ cdma_watch = g_dbus_add_signal_watch(connection, NULL, NULL,
+ OFONO_CDMA_INTERFACE,
+ PROPERTY_CHANGED,
+ cdma_changed,
+ NULL, NULL);
+
if (watch == 0 || gprs_watch == 0 || context_added_watch == 0 ||
context_removed_watch == 0 || modem_watch == 0 ||
reg_watch == 0 || sim_watch == 0 ||
modem_added_watch == 0 || modem_removed_watch == 0 ||
- context_watch == 0) {
+ context_watch == 0 || cdma_watch == 0) {
err = -EIO;
goto remove;
}
@@ -1996,6 +2141,7 @@ remove:
g_dbus_remove_watch(connection, modem_added_watch);
g_dbus_remove_watch(connection, modem_removed_watch);
g_dbus_remove_watch(connection, context_watch);
+ g_dbus_remove_watch(connection, cdma_watch);
dbus_connection_unref(connection);
Regards
Marcel
Kind regards,
Guillaume
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
