Hi Guillaume,
Still some more comments:
On Mon, Aug 01, 2011 at 03:36:55PM +0200, Guillaume Zajac wrote:
> struct modem_data {
> char *path;
> struct connman_device *device;
> gboolean has_sim;
> gboolean has_reg;
> gboolean has_gprs;
> + gboolean is_cdma;
For consistency sake, we should call this one has_cdma.
> @@ -584,6 +627,9 @@ static int network_connect(struct connman_network
> *network)
> if (modem == NULL)
> return -ENODEV;
>
> + if (modem->is_cdma == TRUE)
> + return set_network_active(network);
> +
> if (modem->registered == FALSE)
> return -ENOLINK;
With a CDMA netreg implementation, you could just use the current
network_connect()...
> 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;
This chunk is not needed anymore.
> @@ -887,16 +944,31 @@ static int add_network(struct connman_device *device,
> connman_network_set_available(network, TRUE);
> connman_network_set_index(network, -1);
>
> - if (modem->operator)
> - connman_network_set_name(network, modem->operator);
> - else
> - connman_network_set_name(network, "");
> + if (modem->is_cdma == TRUE) {
> + char *name = g_strdup_printf("CDMA network %d", cdma_count++);
I'm not sure I like this one. Using the modem serial number or some actual
network identifier would be a lot better.
> + if (dict == NULL)
> + goto done;
> +
Too bad dbus_message_iter_get_arg_type() doesn't handle NULL args properly...
> @@ -1658,10 +1740,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 is_cdma = modem_is_cdma(&value);
> + gboolean was_cdma = modem->is_cdma;
s/was_cdma/had_cdma/
> @@ -1679,6 +1778,21 @@ 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);
> +
> + if (g_str_has_prefix(hw_serial, "+GSN:") == TRUE)
> + hw_serial = hw_serial + 5;
> +
> + if (g_str_has_prefix(hw_serial, "+CGSN:") == TRUE)
> + hw_serial = hw_serial + 6;
> +
> + add_device(modem->path, hw_serial);
The missing netreg implementation makes that part a bit hackish. Having the
IMSI would be a lot more consistent.
> + if (modem->is_cdma == TRUE)
> + add_network(modem->device, "/context1", NULL);
I'm a bit confused here: When do you expect to actually add the network ? When
getting the HW serial number, or when getting the CDMA interface ?
Cheers,
Samuel.
--
Intel Open Source Technology Centre
http://oss.intel.com/
_______________________________________________
connman mailing list
[email protected]
http://lists.connman.net/listinfo/connman
