Hi Samuel, 2010/8/18 Samuel Ortiz <[email protected]>: >> The Modem Online property is tied with the cellular device Powered property. > Here again, I would appreciate more detailed explanations as to why we want to > use the Online property instead of the current one. > > More comments below: > >> --- >> plugins/ofono.c | 82 >> ++++++++++++++++++++++++++++++++++++++++++++++--------- >> 1 files changed, 69 insertions(+), 13 deletions(-) >> >> diff --git a/plugins/ofono.c b/plugins/ofono.c >> index 1a4b8e6..43d2410 100644 >> --- a/plugins/ofono.c >> +++ b/plugins/ofono.c >> @@ -65,6 +65,8 @@ struct modem_data { >> char *path; >> struct connman_device *device; >> gboolean available; >> + dbus_bool_t online; >> + gboolean online_is_valid; >> }; >> >> static int modem_probe(struct connman_device *device) >> @@ -159,13 +161,61 @@ static int set_property(const char *path, >> return -EINPROGRESS; >> } >> >> -static int gprs_change_powered(const char *path, dbus_bool_t powered) >> +static void update_online(struct modem_data *modem, connman_bool_t online) >> { >> - DBG("path %s powered %d", path, powered); >> + DBG("modem %p, path %s, online %d", modem, modem->path, online); >> >> - return set_property(path, OFONO_GPRS_INTERFACE, "Powered", >> - DBUS_TYPE_BOOLEAN, &powered, >> - NULL, NULL, NULL); >> + modem->online = online; >> + modem->online_is_valid = TRUE; >> + >> + if (modem->device) >> + connman_device_set_powered(modem->device, online); >> +} >> + >> +static void set_online_reply(DBusPendingCall *call, void *user_data) >> +{ >> + DBusMessage *reply; >> + DBusError error; >> + struct modem_data *modem; >> + >> + DBG(""); >> + >> + dbus_error_init(&error); >> + >> + reply = dbus_pending_call_steal_reply(call); >> + >> + if (dbus_set_error_from_message(&error, reply)) { >> + connman_error("%s(\"%s\"): %s: %s", >> + "SetProperty", "Online", >> + error.name, error.message); > I'm picky again, but this error string is not consistent with the rest of the > ConnMan code. > > >> + dbus_error_free(&error); >> + modem = g_hash_table_lookup(modem_hash, user_data); >> + if (modem && modem->device) >> + connman_device_set_powered(modem->device, >> + modem->online); > So Marcel and Denis told me that currently none of the oFono drivers support > the Online setting currently.
The isimodem driver for N900 and compatible Nokia modems already has support online. >So I'd like to see us falling back on the GPRS > interface if here we get a NotImplemented answer from oFono. I feel that is cheating. The GPRS Powered controls GPRS attach/detach, it does not really rfkill the modem. >> + /* Online default is TRUE for backward compatibility reasons */ >> + dbus_bool_t online = TRUE; > Not sure we need this one as the oFono drivers not implementing the Online > property will set it to TRUE by default (and reply NotImplemented when trying > to set the property). This is for older oFono cores which does not support Online property at all. >> @@ -1162,6 +1210,8 @@ static void modem_properties_reply(DBusPendingCall >> *call, void *user_data) >> if (!powered) >> modem_change_powered(path, TRUE); >> >> + update_online(new_modem, online); >> + > Ok, so this one will be called only once when creating the modem, right ? Yes, it updates the online/online_is_valid fields in modem struct. -- Pekka.Pessi mail at nokia.com _______________________________________________ connman mailing list [email protected] http://lists.connman.net/listinfo/connman
