Hi, On Thu, 2015-10-15 at 15:07 +1100, Craig McQueen wrote: > --- > > I'm not sure if you would like this use of commands. I'm interested not > only in _setting_ clock settings, but also in reading them conveniently. > The precedent in connmanctl is that there is one command to configure a > service ('config'), and a separate command to see its properties > ('services'). So for net.connman.Clock, the analagous commands are > 'configclock' to configure, and 'clock' to read. But 'clock' also takes > parameters which allow individual properties to be read.
Here I'd go for the simpler option of just having a 'clock' command that shows all Clock properties. This way it is similar to 'technologies' and 'state' commands. > My earlier e-mail-- > "connmanctl quitting before multiple return functions called" -- > can be seen with the 'clock' command, reading multiple items. E.g.: > > connmanctl clock time timeupdates timezone > > which on my ARM embedded Linux based system, sometimes gets all three > responses, but often only two. > > client/commands.c | 273 > ++++++++++++++++++++++++++++++++++++++++++++++++++ > client/dbus_helpers.c | 6 ++ > 2 files changed, 279 insertions(+) > > diff --git a/client/commands.c b/client/commands.c > index 3bfdfd7..8b56965 100644 > --- a/client/commands.c > +++ b/client/commands.c > @@ -291,6 +291,75 @@ static int peers_list(DBusMessageIter *iter, > return 0; > } > > +static int one_object_properties(DBusMessageIter *iter, > + const char *error, void *user_data) > +{ > + DBusMessageIter dict; > + > + if (!error) { > + dbus_message_iter_recurse(iter, &dict); > + __connmanctl_dbus_print(&dict, "", " = ", "\n"); > + > + fprintf(stdout, "\n"); > + > + } else { > + fprintf(stderr, "Error: %s\n", error); > + } > + > + return 0; > +} > + > +static int one_object_one_property(DBusMessageIter *iter, > + const char *error, void *user_data) > +{ > + char *property = user_data; > + int arg_type; > + DBusMessageIter dict; > + DBusMessageIter entry; > + DBusMessageIter iter2; > + char *str; > + > + if (error) { > + fprintf(stderr, "Error: %s\n", error); > + return 0; > + } > + > + dbus_message_iter_recurse(iter, &dict); > + while (dbus_message_iter_get_arg_type(&dict) == DBUS_TYPE_DICT_ENTRY) { > + dbus_message_iter_recurse(&dict, &entry); > + arg_type = dbus_message_iter_get_arg_type(&entry); > + if (arg_type == DBUS_TYPE_STRING) { > + dbus_message_iter_get_basic(&entry, &str); > + } else { > + str = ""; > + } > + if (g_strcmp0(str, property) == 0) { > + dbus_message_iter_next(&entry); > + while (dbus_message_iter_get_arg_type(&entry) == > DBUS_TYPE_VARIANT) { > + dbus_message_iter_recurse(&entry, &iter2); > + entry = iter2; > + } > + switch (dbus_message_iter_get_arg_type(&entry)) { > + case DBUS_TYPE_STRUCT: > + case DBUS_TYPE_ARRAY: > + case DBUS_TYPE_DICT_ENTRY: > + dbus_message_iter_recurse(&entry, &iter2); > + __connmanctl_dbus_print(&iter2, "", "=", " "); > + break; > + default: > + __connmanctl_dbus_print(&entry, "", " = ", " = > "); > + break; > + } > + break; > + } > + dbus_message_iter_next(&dict); > + } > + > + fprintf(stdout, "\n"); > + > + return 0; > +} > + > static int object_properties(DBusMessageIter *iter, > const char *error, void *user_data) > { > @@ -1188,6 +1257,189 @@ static int cmd_config(char *args[], int num, struct > connman_option *options) > return result; > } > > +static int configclock_return(DBusMessageIter *iter, const char *error, > + void *user_data) > +{ > + char *property = user_data; > + > + if (error) > + fprintf(stderr, "Error %s: %s\n", property, error); > + > + g_free(user_data); > + > + return 0; > +} > + > +static int cmd_clock(char *args[], int num, struct connman_option *options) > +{ > + int result = 0, res = 0, index = 1, oldindex = 0; > + int c; > + char *path; > + char **opt_start; > + struct config_append append; > + > + if (num == 1) { > + return __connmanctl_dbus_method_call(connection, > CONNMAN_SERVICE, "/", > + "net.connman.Clock", "GetProperties", > + one_object_properties, NULL, NULL, NULL); > + } > + > + while (index < num && args[index]) { > + c = parse_args(args[index], options); > + opt_start = &args[index + 1]; > + append.opts = opt_start; > + append.values = 0; > + > + res = 0; > + > + oldindex = index; > + path = g_strdup("/"); > + > + switch (c) { > + case 'T': > + res = __connmanctl_dbus_method_call(connection, > CONNMAN_SERVICE, "/", > + "net.connman.Clock", "GetProperties", > + one_object_one_property, "Time", NULL, > NULL); > + break; > + > + case 'u': > + res = __connmanctl_dbus_method_call(connection, > CONNMAN_SERVICE, "/", > + "net.connman.Clock", "GetProperties", > + one_object_one_property, "TimeUpdates", > NULL, NULL); > + break; > + > + case 'Z': > + res = __connmanctl_dbus_method_call(connection, > CONNMAN_SERVICE, "/", > + "net.connman.Clock", "GetProperties", > + one_object_one_property, > "TimezoneUpdates", NULL, NULL); > + break; > + > + case 'z': > + res = __connmanctl_dbus_method_call(connection, > CONNMAN_SERVICE, "/", > + "net.connman.Clock", "GetProperties", > + one_object_one_property, "Timezone", > NULL, NULL); > + break; > + > + case 't': > + res = __connmanctl_dbus_method_call(connection, > CONNMAN_SERVICE, "/", > + "net.connman.Clock", "GetProperties", > + one_object_one_property, "Timeservers", > NULL, NULL); > + break; > + > + default: > + res = -EINVAL; > + break; > + } > + > + g_free(path); > + > + if (res < 0) { > + if (res == -EINPROGRESS) > + result = -EINPROGRESS; > + else > + printf("Error '%s': %s\n", args[oldindex], > + strerror(-res)); > + } else > + index += res; > + > + index++; > + } > + > + return result; > +} > + > +static int cmd_configclock(char *args[], int num, struct connman_option > *options) > +{ > + int result = 0, res = 0, index = 1, oldindex = 0; > + int c; > + char *path; > + char **opt_start; > + struct config_append append; > + > + while (index < num && args[index]) { > + c = parse_args(args[index], options); > + opt_start = &args[index + 1]; > + append.opts = opt_start; > + append.values = 0; > + > + res = 0; > + > + oldindex = index; > + path = g_strdup("/"); > + > + switch (c) { > + case 'u': > + if (strcasecmp(*opt_start, "auto") != 0 && > strcasecmp(*opt_start, "manual") != 0) { Try to keep lines < 80 chars in lenght. The convetion has been to just use '(condition)' instead of comparing not equal to zero. > + res = -EINVAL; > + } > + else { else on the same line as } > + res = __connmanctl_dbus_set_property(connection, > + path, "net.connman.Clock", > + configclock_return, > + g_strdup("TimeUpdates"), > + "TimeUpdates", > + DBUS_TYPE_STRING, opt_start); > + } > + index++; > + break; > + > + case 'Z': > + if (strcasecmp(*opt_start, "auto") != 0 && > strcasecmp(*opt_start, "manual") != 0) { > + res = -EINVAL; > + } > + else { As above. > + res = __connmanctl_dbus_set_property(connection, > + path, "net.connman.Clock", > + configclock_return, > + g_strdup("TimezoneUpdates"), > + "TimezoneUpdates", > + DBUS_TYPE_STRING, opt_start); > + } > + index++; > + break; > + > + case 'z': > + res = __connmanctl_dbus_set_property(connection, > + path, "net.connman.Clock", > + configclock_return, > + g_strdup("Timezone"), > + "Timezone", > + DBUS_TYPE_STRING, opt_start); > + index++; > + break; > + > + case 't': > + res = __connmanctl_dbus_set_property_array(connection, > + path, "net.connman.Clock", > + configclock_return, > g_strdup("Timeservers"), > + "Timeservers", > + DBUS_TYPE_STRING, config_append_str, > + &append); > + index += append.values; > + break; > + > + default: > + res = -EINVAL; > + break; > + } > + > + g_free(path); > + > + if (res < 0) { > + if (res == -EINPROGRESS) > + result = -EINPROGRESS; > + else > + printf("Error '%s': %s\n", args[oldindex], > + strerror(-res)); > + } else > + index += res; > + > + index++; > + } > + > + return result; > +} > + > static DBusHandlerResult monitor_changed(DBusConnection *connection, > DBusMessage *message, void *user_data) > { > @@ -2145,6 +2397,23 @@ static struct connman_option service_options[] = { > { NULL, } > }; > > +static struct connman_option clock_options[] = { > + {"time", 'T', ""}, > + {"timeservers", 't', ""}, > + {"timeupdates", 'u', ""}, > + {"timezoneupdates", 'Z', ""}, > + {"timezone", 'z', ""}, > + { NULL, } > +}; > + Here I'd take the simpler option and have the 'clock' command print out all properties without taking any arguments. Then the return function looks identical(?) to 'state_print()'. > +static struct connman_option configclock_options[] = { > + {"timeservers", 't', "<ntp1> [<ntp2>] [...]"}, > + {"timeupdates", 'u', "auto|manual"}, > + {"timezoneupdates", 'Z', "auto|manual"}, > + {"timezone", 'z', "<timezone>"}, > + { NULL, } > +}; > + > static struct connman_option config_options[] = { > {"nameservers", 'n', "<dns1> [<dns2>] [<dns3>]"}, > {"timeservers", 't', "<ntp1> [<ntp2>] [...]"}, > @@ -2535,6 +2804,10 @@ static const struct { > { "move-after", "<service> <target service> ", NULL, > cmd_service_move_after, "Move <service> after <target service>", > lookup_service_arg }, > + { "clock", "[optons]", clock_options, cmd_clock, > + "Show clock properties", NULL }, > + { "configclock", "[optons]", configclock_options, > cmd_configclock, > + "Set clock properties", NULL }, > { "config", "<service>", config_options, cmd_config, > "Set service configuration options", lookup_config }, > { "monitor", "[off]", monitor_options, cmd_monitor, > diff --git a/client/dbus_helpers.c b/client/dbus_helpers.c > index 9c4cfee..f7ad7c2 100644 > --- a/client/dbus_helpers.c > +++ b/client/dbus_helpers.c > @@ -37,6 +37,7 @@ void __connmanctl_dbus_print(DBusMessageIter *iter, const > char *pre, > unsigned char c; > dbus_uint16_t u16; > dbus_uint32_t u; > + dbus_uint64_t u64; > dbus_int32_t i; > double d; > > @@ -113,6 +114,11 @@ void __connmanctl_dbus_print(DBusMessageIter *iter, > const char *pre, > fprintf(stdout, "%d", i); > break; > > + case DBUS_TYPE_UINT64: > + dbus_message_iter_get_basic(iter, &u64); > + fprintf(stdout, "%lu", u64); To get this part to work correctly in all cases, please use "%"PRIu64 instead of "%lu" here. > + break; > + > case DBUS_TYPE_DOUBLE: > dbus_message_iter_get_basic(iter, &d); > fprintf(stdout, "%f", d); Sorry the review took this many calendar days to achieve. Thanks for your efforts! Patrik _______________________________________________ connman mailing list connman@connman.net https://lists.connman.net/mailman/listinfo/connman