* Re: [PATCH V2] Add GetKnownServices api to connaman
2021-10-27 14:01 [PATCH V2] Add GetKnownServices api to connaman Michael Trimarchi
@ 2021-10-27 17:25 ` Michael Nazzareno Trimarchi
2021-10-28 6:31 ` Daniel Wagner
2021-10-28 6:16 ` Daniel Wagner
1 sibling, 1 reply; 5+ messages in thread
From: Michael Nazzareno Trimarchi @ 2021-10-27 17:25 UTC (permalink / raw)
To: connman, Daniel Wagner; +Cc: Jan.Ryll, Simon.Holesch
Hi Daniel
I even have the patch to remove known-networks. I have few question:
- what should be the behavior for an ethernet on?
- for wifi, it's removed and if you are connected it's disconnected is
that correct?
Apart of that:
I'm working on a patch that allow to store network configuration using
the config storage
but I have few questions:
- I have added autoconnect as a configuration option. I would like to
store networks but not have them
in auto connect mode. This should allow me to connect when they are
visible. (ethernet??)
- I have add an option to store if I want it immutable or not. The
stored configuration should be applied
if it's valid but I would like to connect using a different way from
connect call
--- a/src/config.c
+++ b/src/config.c
@@ -61,6 +61,8 @@ struct connman_config_service {
GSList *service_identifiers;
char *config_ident; /* file prefix */
char *config_entry; /* entry name */
+ bool autoconnect;
+ bool immutable;
bool hidden;
bool virtual;
char *virtual_file;
@@ -115,6 +117,8 @@ static bool cleanup = false;
#define SERVICE_KEY_SECURITY "Security"
#define SERVICE_KEY_HIDDEN "Hidden"
#define SERVICE_KEY_MDNS "mDNS"
+#define SERVICE_KEY_AUTOCONNECT "Autoconnect"
+#define SERVICE_KEY_IMMUTABLE "Immutable"
#define SERVICE_KEY_IPv4 "IPv4"
#define SERVICE_KEY_IPv6 "IPv6"
@@ -162,6 +166,8 @@ static const char *service_possible_keys[] = {
SERVICE_KEY_SEARCH_DOMAINS,
SERVICE_KEY_TIMESERVERS,
SERVICE_KEY_DOMAIN,
+ SERVICE_KEY_AUTOCONNECT,
+ SERVICE_KEY_IMMUTABLE,
NULL,
};
@@ -229,7 +235,7 @@ static void unregister_service(gpointer data)
}
}
- if (!__connman_storage_remove_service(service_id))
+ if (config_service->autoconnect &&
!__connman_storage_remove_service(service_id))
DBG("Could not remove all files for service %s",
service_id);
}
@@ -599,6 +605,12 @@ static bool load_service(GKeyFile *keyfile, const
char *group,
return true;
}
+ service->autoconnect = __connman_config_get_bool(keyfile, group,
+ SERVICE_KEY_AUTOCONNECT, NULL);
+
+ service->immutable = __connman_config_get_bool(keyfile, group,
+ SERVICE_KEY_IMMUTABLE, NULL);
+
str = __connman_config_get_string(keyfile, group,
SERVICE_KEY_NAME, NULL);
if (str) {
g_free(service->name);
@@ -1180,6 +1192,8 @@ static void provision_service_wifi(struct
connman_config_service *config,
if (config->hidden)
__connman_service_set_hidden(service);
+
+ __connman_service_set_auto_connect(service, config->autoconnect);
Michael
On Wed, Oct 27, 2021 at 2:01 PM Michael Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Add a way to retrieve all the networks that were configured
> using connman and not all the ones visible
>
> ---
> Changes V1->V2:
> - remove SoB
> - Add documentation
> - remove LOG print
> - Avoid check null on functions that assert
> - adjust the function the look for service
> - clean up a bit variable name
> - remove initialization of variables that are not needed
> ---
> client/commands.c | 14 +++++++++++
> doc/manager-api.txt | 12 ++++++++++
> src/connman.h | 1 +
> src/manager.c | 23 ++++++++++++++++++
> src/service.c | 55 ++++++++++++++++++++++++++++++++++++++++++++
> tools/manager-api.c | 30 ++++++++++++++++++++++++
> tools/session-test.h | 1 +
> 7 files changed, 136 insertions(+)
>
> diff --git a/client/commands.c b/client/commands.c
> index 53cc14c8..f4366e8c 100644
> --- a/client/commands.c
> +++ b/client/commands.c
> @@ -420,6 +420,18 @@ static int cmd_services(char *args[], int num, struct connman_option *options)
> object_properties, path, NULL, NULL);
> }
>
> +static int cmd_known_services(char *args[], int num, struct connman_option *options)
> +{
> + if (num > 1)
> + return -E2BIG;
> +
> + return __connmanctl_dbus_method_call(connection,
> + CONNMAN_SERVICE, CONNMAN_PATH,
> + "net.connman.Manager",
> + "GetKnownServices",
> + services_list, NULL, NULL, NULL);
> +}
> +
> static int cmd_peers(char *args[], int num, struct connman_option *options)
> {
> char *peer_name = NULL;
> @@ -2808,6 +2820,8 @@ static const struct {
> "Display tethering clients", NULL },
> { "services", "[<service>]", service_options, cmd_services,
> "Display services", lookup_service_arg },
> + { "known-services", NULL, NULL, cmd_known_services,
> + "Display known services", NULL },
> { "peers", "[peer]", NULL, cmd_peers,
> "Display peers", lookup_peer_arg },
> { "scan", "<technology>", NULL, cmd_scan,
> diff --git a/doc/manager-api.txt b/doc/manager-api.txt
> index 6eaa0a38..093b25c1 100644
> --- a/doc/manager-api.txt
> +++ b/doc/manager-api.txt
> @@ -39,6 +39,18 @@ Methods dict GetProperties()
>
> Possible Errors: [service].Error.InvalidArguments
>
> + array{object,dict} GetKnownServices()
> +
> + Returns a sorted list of tuples with service
> + object path and dictionary of service propertiesa
> + Those are all the services including the one that
> + are registered but not visible or not in use.
> +
> + This list will not contain sensitive information
> + like passphrases etc.
> +
> + Possible Errors: [service].Error.InvalidArguments
> +
> array{object,dict} GetPeers() [experimental]
>
> Returns a sorted list of tuples with peer object path
> diff --git a/src/connman.h b/src/connman.h
> index 68176086..cdc88532 100644
> --- a/src/connman.h
> +++ b/src/connman.h
> @@ -682,6 +682,7 @@ int __connman_service_move(struct connman_service *service,
> struct connman_service *target, bool before);
> int __connman_service_load_modifiable(struct connman_service *service);
>
> +void __connman_known_service_list_struct(DBusMessageIter *iter);
> void __connman_service_list_struct(DBusMessageIter *iter);
>
> int __connman_service_compare(const struct connman_service *a,
> diff --git a/src/manager.c b/src/manager.c
> index 892d3a42..b196109a 100644
> --- a/src/manager.c
> +++ b/src/manager.c
> @@ -175,6 +175,11 @@ static const struct connman_notifier technology_notifier = {
> .idle_state = idle_state,
> };
>
> +static void append_known_service_structs(DBusMessageIter *iter, void *user_data)
> +{
> + __connman_known_service_list_struct(iter);
> +}
> +
> static void append_service_structs(DBusMessageIter *iter, void *user_data)
> {
> __connman_service_list_struct(iter);
> @@ -195,6 +200,21 @@ static DBusMessage *get_services(DBusConnection *conn,
> return reply;
> }
>
> +static DBusMessage *get_known_services(DBusConnection *conn,
> + DBusMessage *msg, void *data)
> +{
> + DBusMessage *reply;
> +
> + reply = dbus_message_new_method_return(msg);
> + if (!reply)
> + return NULL;
> +
> + __connman_dbus_append_objpath_dict_array(reply,
> + append_known_service_structs, NULL);
> +
> + return reply;
> +}
> +
> static void append_peer_structs(DBusMessageIter *iter, void *user_data)
> {
> __connman_peer_list_struct(iter);
> @@ -534,6 +554,9 @@ static const GDBusMethodTable manager_methods[] = {
> { GDBUS_DEPRECATED_METHOD("RemoveProvider",
> GDBUS_ARGS({ "provider", "o" }), NULL,
> remove_provider) },
> + { GDBUS_METHOD("GetKnownServices",
> + NULL, GDBUS_ARGS({"knownServices", "a(oa{sv})" }),
> + get_known_services) },
> { GDBUS_METHOD("GetServices",
> NULL, GDBUS_ARGS({ "services", "a(oa{sv})" }),
> get_services) },
> diff --git a/src/service.c b/src/service.c
> index 8d3c75a5..da8304cc 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -2705,6 +2705,61 @@ static void append_struct(gpointer value, gpointer user_data)
> append_struct_service(iter, append_dict_properties, service);
> }
>
> +void __connman_known_service_list_struct(DBusMessageIter *iter)
> +{
> + gchar **services;
> + gchar **srv_id_parts;
> + guint i;
> + struct connman_service *service;
> +
> + services = connman_storage_get_services();
> + if (!services)
> + return;
> +
> + for (i = 0; i < g_strv_length(services); i++) {
> + struct connman_service *srv;
> +
> + service = connman_service_create();
> + if (!service) {
> + connman_error("connman_service_create() allocation failed");
> + return;
> + }
> +
> + service->identifier = g_strdup(services[i]);
> + srv_id_parts = g_strsplit(services[i], "_", -1);
> + if (srv_id_parts == NULL) {
> + g_free(service);
> + continue;
> + }
> +
> + service->type = __connman_service_string2type(srv_id_parts[0]);
> + service->path = g_strdup_printf("%s/%s", STORAGEDIR, service->identifier);
> + if (service->type == CONNMAN_SERVICE_TYPE_WIFI)
> + service->security = __connman_service_string2security(srv_id_parts[g_strv_length(srv_id_parts) - 1]);
> +
> + srv = find_service(service->path);
> + if (srv)
> + service->state = srv->state;
> +
> + if (service->type == CONNMAN_SERVICE_TYPE_ETHERNET && srv)
> + service->name = srv->name;
> +
> + if (service_load(service)) {
> + connman_error("service_load() returned error");
> + g_free(service);
> + g_strfreev(srv_id_parts);
> + g_strfreev(services);
> + return;
> + }
> +
> + append_struct_service(iter, append_dict_properties, service);
> + g_strfreev(srv_id_parts);
> + g_free(service);
> + }
> +
> + g_strfreev(services);
> +}
> +
> void __connman_service_list_struct(DBusMessageIter *iter)
> {
> g_list_foreach(service_list, append_struct, iter);
> diff --git a/tools/manager-api.c b/tools/manager-api.c
> index e082962d..8668ca9e 100644
> --- a/tools/manager-api.c
> +++ b/tools/manager-api.c
> @@ -64,6 +64,36 @@ static DBusMessage *set_property(DBusConnection *connection,
> return reply;
> }
>
> +DBusMessage *manager_get_known_services(DBusConnection *connection)
> +{
> + DBusMessage *message, *reply;
> + DBusError error;
> +
> + message = dbus_message_new_method_call(CONNMAN_SERVICE,
> + CONNMAN_MANAGER_PATH,
> + CONNMAN_MANAGER_INTERFACE,
> + "GetKnownServices");
> +
> + if (!message)
> + return NULL;
> +
> + dbus_error_init(&error);
> +
> + reply = dbus_connection_send_with_reply_and_block(connection,
> + message, -1, &error);
> + if (!reply) {
> + if (dbus_error_is_set(&error))
> + dbus_error_free(&error);
> +
> + dbus_message_unref(message);
> + return NULL;
> + }
> +
> + dbus_message_unref(message);
> +
> + return reply;
> +}
> +
> DBusMessage *manager_get_services(DBusConnection *connection)
> {
> DBusMessage *message, *reply;
> diff --git a/tools/session-test.h b/tools/session-test.h
> index 2c068bd7..ab89f8c5 100644
> --- a/tools/session-test.h
> +++ b/tools/session-test.h
> @@ -114,6 +114,7 @@ DBusMessage *session_disconnect(DBusConnection *connection,
> struct test_session *session);
>
> /* manager-api.c */
> +DBusMessage *manager_get_known_services(DBusConnection *connection);
> DBusMessage *manager_get_services(DBusConnection *connection);
> DBusMessage *manager_get_properties(DBusConnection *connection);
> DBusMessage *manager_create_session(DBusConnection *connection,
> --
> 2.25.1
>
--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________
Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH V2] Add GetKnownServices api to connaman
2021-10-27 14:01 [PATCH V2] Add GetKnownServices api to connaman Michael Trimarchi
2021-10-27 17:25 ` Michael Nazzareno Trimarchi
@ 2021-10-28 6:16 ` Daniel Wagner
1 sibling, 0 replies; 5+ messages in thread
From: Daniel Wagner @ 2021-10-28 6:16 UTC (permalink / raw)
To: Michael Trimarchi; +Cc: connman, Jan.Ryll, Simon.Holesch
Hi Michael,
On Wed, Oct 27, 2021 at 02:01:41PM +0000, Michael Trimarchi wrote:
> Add a way to retrieve all the networks that were configured
> using connman and not all the ones visible
>
> ---
> Changes V1->V2:
> - remove SoB
> - Add documentation
> - remove LOG print
> - Avoid check null on functions that assert
> - adjust the function the look for service
> - clean up a bit variable name
> - remove initialization of variables that are not needed
> ---
> client/commands.c | 14 +++++++++++
> doc/manager-api.txt | 12 ++++++++++
> src/connman.h | 1 +
> src/manager.c | 23 ++++++++++++++++++
> src/service.c | 55 ++++++++++++++++++++++++++++++++++++++++++++
> tools/manager-api.c | 30 ++++++++++++++++++++++++
> tools/session-test.h | 1 +
> 7 files changed, 136 insertions(+)
>
> diff --git a/client/commands.c b/client/commands.c
> index 53cc14c8..f4366e8c 100644
> --- a/client/commands.c
> +++ b/client/commands.c
> @@ -420,6 +420,18 @@ static int cmd_services(char *args[], int num, struct connman_option *options)
> object_properties, path, NULL, NULL);
> }
>
> +static int cmd_known_services(char *args[], int num, struct connman_option *options)
> +{
> + if (num > 1)
> + return -E2BIG;
> +
> + return __connmanctl_dbus_method_call(connection,
> + CONNMAN_SERVICE, CONNMAN_PATH,
> + "net.connman.Manager",
> + "GetKnownServices",
> + services_list, NULL, NULL, NULL);
> +}
> +
> static int cmd_peers(char *args[], int num, struct connman_option *options)
> {
> char *peer_name = NULL;
> @@ -2808,6 +2820,8 @@ static const struct {
> "Display tethering clients", NULL },
> { "services", "[<service>]", service_options, cmd_services,
> "Display services", lookup_service_arg },
> + { "known-services", NULL, NULL, cmd_known_services,
> + "Display known services", NULL },
> { "peers", "[peer]", NULL, cmd_peers,
> "Display peers", lookup_peer_arg },
> { "scan", "<technology>", NULL, cmd_scan,
> diff --git a/doc/manager-api.txt b/doc/manager-api.txt
> index 6eaa0a38..093b25c1 100644
> --- a/doc/manager-api.txt
> +++ b/doc/manager-api.txt
> @@ -39,6 +39,18 @@ Methods dict GetProperties()
>
> Possible Errors: [service].Error.InvalidArguments
>
> + array{object,dict} GetKnownServices()
> +
> + Returns a sorted list of tuples with service
> + object path and dictionary of service propertiesa
... properties. But the text is wrong anyway. 'object path' is not
correct, it is the storage path of the dictionary containing the
associated resources with this known service.
About those properties, e.g.
[('/var/lib/connman/ethernet_2cf05d0a93f0_cable',
{'AutoConnect': False,
'Domains': [],
'Domains.Configuration': [],
'Ethernet': {},
'Favorite': False,
'IPv4': {},
'IPv4.Configuration': {},
'IPv6': {},
'IPv6.Configuration': {},
'Immutable': False,
'Nameservers': [],
'Nameservers.Configuration': [],
'Provider': {},
'Proxy': {},
'Proxy.Configuration': {},
'Security': [],
'Timeservers': [],
'Timeservers.Configuration': [],
'Type': 'ethernet',
'mDNS': False,
'mDNS.Configuration': False}),
I don't think this makes sense, especially that all services return
empty properties. GetKnownServices() should just return the object path
and then the dictionary containing the storage path.
> +void __connman_known_service_list_struct(DBusMessageIter *iter)
> +{
> + gchar **services;
> + gchar **srv_id_parts;
> + guint i;
> + struct connman_service *service;
Reverse Christmas tree, no glib types when not needed,
struct connman_service *service;
char **srv_id_parts;
char **services;
unsigned int i;
> +
> + services = connman_storage_get_services();
> + if (!services)
> + return;
> +
> + for (i = 0; i < g_strv_length(services); i++) {
> + struct connman_service *srv;
> +
> + service = connman_service_create();
> + if (!service) {
> + connman_error("connman_service_create() allocation failed");
> + return;
> + }
> +
> + service->identifier = g_strdup(services[i]);
> + srv_id_parts = g_strsplit(services[i], "_", -1);
> + if (srv_id_parts == NULL) {
Does not make sense srv_id_parts is never NULL. See glib documentation.
> + g_free(service);
> + continue;
> + }
> +
> + service->type = __connman_service_string2type(srv_id_parts[0]);
> + service->path = g_strdup_printf("%s/%s", STORAGEDIR, service->identifier);
> + if (service->type == CONNMAN_SERVICE_TYPE_WIFI)
> + service->security = __connman_service_string2security(srv_id_parts[g_strv_length(srv_id_parts) - 1]);
> +
The line is way too long. Do something like
if (service->type == CONNMAN_SERVICE_TYPE_WIFI) {
char *id = srv_id_parts[g_strv_length(srv_id_parts) - 1];
service->security = __connman_service_string2security(id);
}
> + srv = find_service(service->path);
> + if (srv)
> + service->state = srv->state;
> +
> + if (service->type == CONNMAN_SERVICE_TYPE_ETHERNET && srv)
> + service->name = srv->name;
> +
> + if (service_load(service)) {
> + connman_error("service_load() returned error");
> + g_free(service);
> + g_strfreev(srv_id_parts);
> + g_strfreev(services);
> + return;
> + }
> +
> + append_struct_service(iter, append_dict_properties, service);
> + g_strfreev(srv_id_parts);
> + g_free(service);
> + }
> +
> + g_strfreev(services);
> +}
> +
Thanks,
Daniel
^ permalink raw reply [flat|nested] 5+ messages in thread