Re: [PATCH 3/5] Add dun_enable() function
Hi Gustavo,
+static void dun_connect_reply(DBusPendingCall *call, gpointer
user_data)
+{
+ struct ofono_modem *modem = user_data;
+ struct dun_data *data = ofono_modem_get_data(modem);
+ const char *dev;
+ DBusError derr;
+ DBusMessage *reply, *msg;
+
+ reply = dbus_pending_call_steal_reply(call);
+
+ if (ofono_modem_get_powered(modem))
+ goto done;
I prefer you don't do this check above. If the core isn't doing
something right I want things to crash right away.
+
+ if (!dbus_message_get_args(reply, NULL, DBUS_TYPE_STRING, &dev,
+ DBUS_TYPE_INVALID))
+ goto done;
That seems wrong, you need to tell the core powering up failed. This
should be treated as an error.
+
+ data->rfcomm = g_strdup(dev);
+
+ dbus_error_init(&derr);
+ if (!dbus_set_error_from_message(&derr, reply)) {
+ ofono_modem_set_powered(modem, TRUE);
+ goto done;
+ }
Checking for D-Bus Error should be done before grabbing the reply
arguments, especially since you never free rfcomm above.
+
+ DBG("Connect reply: %s", derr.message);
+
+ if (dbus_error_has_name(&derr, DBUS_ERROR_NO_REPLY)) {
+ msg = dbus_message_new_method_call(BLUEZ_SERVICE,
+ data->dun_path,
+ BLUEZ_SERIAL_INTERFACE, "Disconnect");
+ if (!msg)
+ ofono_error("Disconnect failed");
+ else
+ g_dbus_send_message(connection, msg);
+ }
I'm not sure this part is really necessary. How can we trigger a
condition where this function fails but the tty device is created?
+
+ ofono_modem_set_powered(modem, FALSE);
+
+ dbus_error_free(&derr);
Might want to localize dbus_error_free() to the same area where it is
being used. Makes the code easier to read.
+
+done:
+ dbus_message_unref(reply);
+}
+
static int dun_enable(struct ofono_modem *modem)
{
- DBG("%p", modem);
Might want to leave the DBG in there.
- return 0;
+ struct dun_data *data = ofono_modem_get_data(modem);
+ int status;
+ const char *uuid = DUN_GW_UUID;
+
+ status = bluetooth_send_with_reply(data->dun_path,
+ BLUEZ_SERIAL_INTERFACE, "Connect",
+ dun_connect_reply, modem, NULL,
+ DBUS_TIMEOUT, DBUS_TYPE_STRING, &uuid,
+ DBUS_TYPE_INVALID);
+
+ if (status < 0)
+ return -EINVAL;
+
+ return -EINPROGRESS;
}
static int dun_disable(struct ofono_modem *modem)
Regards,
-Denis