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