Discussion:
[PATCH] connmanctl: Implement clock API with new clock commands
(too old to reply)
Craig McQueen
2015-10-15 04:07:29 UTC
Permalink
---

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.

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) {
+ res = -EINVAL;
+ }
+ else {
+ 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 {
+ 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, }
+};
+
+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);
+ break;
+
case DBUS_TYPE_DOUBLE:
dbus_message_iter_get_basic(iter, &d);
fprintf(stdout, "%f", d);
--
2.1.4
Patrik Flykt
2015-10-29 08:05:51 UTC
Permalink
Hi,
Post by Craig McQueen
---
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.
Post by Craig McQueen
My earlier e-mail--
"connmanctl quitting before multiple return functions called" --
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)) {
+ dbus_message_iter_recurse(&entry, &iter2);
+ __connmanctl_dbus_print(&iter2, "", "=", " ");
+ break;
+ __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) {
+ res = __connmanctl_dbus_method_call(connection, CONNMAN_SERVICE, "/",
+ "net.connman.Clock", "GetProperties",
+ one_object_one_property, "Time", NULL, NULL);
+ break;
+
+ res = __connmanctl_dbus_method_call(connection, CONNMAN_SERVICE, "/",
+ "net.connman.Clock", "GetProperties",
+ one_object_one_property, "TimeUpdates", NULL, NULL);
+ break;
+
+ res = __connmanctl_dbus_method_call(connection, CONNMAN_SERVICE, "/",
+ "net.connman.Clock", "GetProperties",
+ one_object_one_property, "TimezoneUpdates", NULL, NULL);
+ break;
+
+ res = __connmanctl_dbus_method_call(connection, CONNMAN_SERVICE, "/",
+ "net.connman.Clock", "GetProperties",
+ one_object_one_property, "Timezone", NULL, NULL);
+ break;
+
+ res = __connmanctl_dbus_method_call(connection, CONNMAN_SERVICE, "/",
+ "net.connman.Clock", "GetProperties",
+ one_object_one_property, "Timeservers", NULL, NULL);
+ break;
+
+ 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) {
+ 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.
Post by Craig McQueen
+ res = -EINVAL;
+ }
+ else {
else on the same line as }
Post by Craig McQueen
+ res = __connmanctl_dbus_set_property(connection,
+ path, "net.connman.Clock",
+ configclock_return,
+ g_strdup("TimeUpdates"),
+ "TimeUpdates",
+ DBUS_TYPE_STRING, opt_start);
+ }
+ index++;
+ break;
+
+ if (strcasecmp(*opt_start, "auto") != 0 && strcasecmp(*opt_start, "manual") != 0) {
+ res = -EINVAL;
+ }
+ else {
As above.
Post by Craig McQueen
+ res = __connmanctl_dbus_set_property(connection,
+ path, "net.connman.Clock",
+ configclock_return,
+ g_strdup("TimezoneUpdates"),
+ "TimezoneUpdates",
+ DBUS_TYPE_STRING, opt_start);
+ }
+ index++;
+ break;
+
+ res = __connmanctl_dbus_set_property(connection,
+ path, "net.connman.Clock",
+ configclock_return,
+ g_strdup("Timezone"),
+ "Timezone",
+ DBUS_TYPE_STRING, opt_start);
+ index++;
+ break;
+
+ 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;
+
+ 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()'.
Post by Craig McQueen
+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;
+ 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.
Post by Craig McQueen
+ break;
+
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

Loading...