Hi Samuel,
On 29/07/2011 17:21, Samuel Ortiz wrote:
Hi Guillaume,
On Thu, Jul 28, 2011 at 02:07:20PM +0200, Guillaume Zajac wrote:
--- 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_CONNECTION_MANAGER_INTERFACE OFONO_SERVICE
".cdma.ConnectionManager"
Could we just call this one OFONO_CDMA_INTERFACE ?
Ok then we will get less than 80 characters.
+static int set_cdma_network_inactive(struct connman_network *network,
+ const char *path)
+{
+ int err;
+ dbus_bool_t value = FALSE;
+
+ DBG("network %p, path %s", network, path);
+
+ err = set_property(path, OFONO_CDMA_CONNECTION_MANAGER_INTERFACE,
+ "Powered", DBUS_TYPE_BOOLEAN,&value,
+ NULL, NULL, NULL);
+
+ if (err == -EINPROGRESS)
+ err = 0;
+
+ return err;
+}
So I'd rather use the set_network_active|inactive() routine and have them
support cdma. You would get the modem from network -> device -> modem.
Then you'd set the interface and the property paths depending on the modem
being a cdma one or not.
Fine for me.
@@ -584,6 +616,9 @@ static int network_connect(struct connman_network *network)
if (modem == NULL)
return -ENODEV;
+ if (modem->is_cdma)
+ return set_cdma_network_active(network, modem->path);
+
if (modem->registered == FALSE)
return -ENOLINK;
@@ -598,13 +633,27 @@ static int network_connect(struct connman_network
*network)
static int network_disconnect(struct connman_network *network)
{
+ struct connman_device *device;
+ struct modem_data *modem;
+
DBG("network %p", network);
+ device = connman_network_get_device(network);
+ if (device == NULL)
+ return -ENODEV;
+
+ modem = connman_device_get_data(device);
+ if (modem == NULL)
+ return -ENODEV;
+
if (connman_network_get_index(network)< 0)
return -ENOTCONN;
connman_network_set_associating(network, FALSE);
+ if (modem->is_cdma)
+ return set_cdma_network_inactive(network, modem->path);
+
return set_network_inactive(network);
}
With my above proposition, those 2 chunks are removed.
Yes
+static int add_cdma_network(struct connman_device *device, const char *path)
+{
+ struct modem_data *modem = connman_device_get_data(device);
+ struct connman_network *network;
+ struct network_info *info;
+ char *ident;
+ const char *hash_path;
+
+ DBG("modem %p device %p path %s", modem, device, path);
+
+ ident = get_ident(path);
+
+ network = connman_device_get_network(device, ident);
+ if (network != NULL)
+ return -EALREADY;
+
+ info = g_hash_table_lookup(network_hash, path);
+ if (info != NULL) {
+ DBG("path %p already exists with device %p", path,
+ connman_network_get_device(info->network));
+ if (connman_network_get_device(info->network))
+ return -EALREADY;
+ g_hash_table_remove(network_hash, path);
+ }
+
+ network = connman_network_create(ident, CONNMAN_NETWORK_TYPE_CELLULAR);
+ if (network == NULL)
+ return -ENOMEM;
+
+ info = g_try_new0(struct network_info, 1);
+ if (info == NULL) {
+ connman_network_unref(network);
+ return -ENOMEM;
+ }
+
+ connman_ipaddress_clear(&info->ipv4_address);
+ connman_ipaddress_clear(&info->ipv6_address);
+ info->network = network;
+
+ connman_network_set_string(network, "Path", path);
+ hash_path = connman_network_get_string(network, "Path");
+ if (hash_path == NULL) {
+ connman_network_unref(network);
+ g_free(info);
+ return -EIO;
+ }
+
+ create_service(network);
+
+ connman_network_ref(network);
+ g_hash_table_insert(network_hash, (char *) hash_path, info);
+
+ connman_network_set_available(network, TRUE);
+ connman_network_set_index(network, -1);
+
+ connman_network_set_name(network, "CDMA Network");
+
+ if (modem->has_strength)
+ connman_network_set_strength(network, modem->strength);
+
+ if (connman_device_add_network(device, network) != 0)
+ goto error;
+
+ return 0;
+
+error:
+ connman_network_unref(network);
+ g_hash_table_remove(network_hash, hash_path);
+ return -EIO;
+}
So my first question would be: Why do we need a specific network addition
routine for CDMA ? The only addition to the existing add_network() routine
would be for the network name setting since oFono doesn't have a CDMA netreg
support. By the way, setting all CDMA network names to "CDMA network" is not
good, you need to find a unique name.
I will keep one add_network() function. I will also associate a number
to "CDMA network %d" to keep it unique.
My second question is: Why do you need that ?:
+ connman_network_set_string(network, "Path", path);
+ hash_path = connman_network_get_string(network, "Path");
At the beginning I worked on 0.69 ConnMan and there was this part into
ofono plugin in add_network() function.
My add_cdma_network() function was based on it, that is why on HEAD
version there is this stupid thing :)
I will fix it.
@@ -1656,10 +1786,21 @@ 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 is_cdma = modem_is_cdma(&value);
modem->has_sim = has_sim;
modem->has_reg = has_reg;
modem->has_gprs = has_gprs;
+ modem->is_cdma = is_cdma;
+
+ if (is_cdma) {
+ if (modem->device == NULL)
+ return TRUE;
+
+ add_cdma_network(modem->device, "/context1");
With CDMA supportting only one context, I suppose the hardcoded path here is
ok.
Cheers,
Samuel.
Kind regards,
Guillaume
_______________________________________________
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
