[PATCH 1/5] ap: return error from GetDiagnostics if not started

[PATCH 1/5] ap: return error from GetDiagnostics if not started

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 477 bytes --]

---
 src/ap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/ap.c b/src/ap.c
index 37dd05b9..fddf0255 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -3200,6 +3200,9 @@ static struct l_dbus_message *ap_dbus_get_diagnostics(struct l_dbus *dbus,
 	struct diagnostic_data *data;
 	int ret;
 
+	if (!ap_if->ap)
+		return dbus_error_not_found(message);
+
 	data = l_new(struct diagnostic_data, 1);
 	data->pending = l_dbus_message_ref(message);
 
-- 
2.26.2

[PATCH 2/5] client: ap: handle NotFound on diagnostic interface

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 994 bytes --]

This can happen when 'ap <wlan> show' is used on an AP interface
which has not been started. This isn't an error per-se so it
should be handled and print nothing rather than print an error.
---
 client/ap.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/client/ap.c b/client/ap.c
index 7130bb9a..9a7f8642 100644
--- a/client/ap.c
+++ b/client/ap.c
@@ -201,9 +201,18 @@ static void ap_get_diagnostics_callback(struct l_dbus_message *message,
 	struct l_dbus_message_iter iter;
 	uint16_t idx = 0;
 	char client_num[15];
+	const char *name;
+	const char *text;
 
-	if (dbus_message_has_error(message))
+	if (l_dbus_message_get_error(message, &name, &text)) {
+		if (!strcmp(name, "net.connman.iwd.NotFound")) {
+			display_table_footer();
+			return;
+		}
+
+		display_error(text);
 		return;
+	}
 
 	if (!l_dbus_message_get_arguments(message, "aa{sv}", &array)) {
 		display("Failed to parse GetDiagnostics message");
-- 
2.26.2

[PATCH 3/5] ap: add SSID property

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 2093 bytes --]

Its useful being able to refer to the SSID once an AP is
started. For example opening an iwctl session with an
already started AP provides no way of obtaining the SSID.
---
 src/ap.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/src/ap.c b/src/ap.c
index fddf0255..d58932dd 100644
--- a/src/ap.c
+++ b/src/ap.c
@@ -2971,12 +2971,18 @@ static void ap_if_event_func(enum ap_event_type type, const void *event_data,
 		l_dbus_property_changed(dbus_get_bus(),
 					netdev_get_path(ap_if->netdev),
 					IWD_AP_INTERFACE, "Started");
+		l_dbus_property_changed(dbus_get_bus(),
+					netdev_get_path(ap_if->netdev),
+					IWD_AP_INTERFACE, "SSID");
 		break;
 
 	case AP_EVENT_STOPPING:
 		l_dbus_property_changed(dbus_get_bus(),
 					netdev_get_path(ap_if->netdev),
 					IWD_AP_INTERFACE, "Started");
+		l_dbus_property_changed(dbus_get_bus(),
+					netdev_get_path(ap_if->netdev),
+					IWD_AP_INTERFACE, "SSID");
 
 		if (!ap_if->pending)
 			ap_if->ap = NULL;
@@ -3112,6 +3118,24 @@ static bool ap_dbus_property_get_started(struct l_dbus *dbus,
 	return true;
 }
 
+static bool ap_dbus_property_get_ssid(struct l_dbus *dbus,
+					struct l_dbus_message *message,
+					struct l_dbus_message_builder *builder,
+					void *user_data)
+{
+	struct ap_if_data *ap_if = user_data;
+	const char *ssid;
+
+	if (!ap_if->ap || !ap_if->ap->config || !ap_if->ap->started)
+		ssid = "";
+	else
+		ssid = ap_if->ap->config->ssid;
+
+	l_dbus_message_builder_append_basic(builder, 's', ssid);
+
+	return true;
+}
+
 static void ap_setup_interface(struct l_dbus_interface *interface)
 {
 	l_dbus_interface_method(interface, "Start", 0, ap_dbus_start, "",
@@ -3123,6 +3147,8 @@ static void ap_setup_interface(struct l_dbus_interface *interface)
 
 	l_dbus_interface_property(interface, "Started", 0, "b",
 					ap_dbus_property_get_started, NULL);
+	l_dbus_interface_property(interface, "SSID", 0, "s",
+					ap_dbus_property_get_ssid, NULL);
 }
 
 static void ap_destroy_interface(void *user_data)
-- 
2.26.2

[PATCH 4/5] doc: document SSID property on AP interface

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 563 bytes --]

---
 doc/access-point-api.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/access-point-api.txt b/doc/access-point-api.txt
index 9682b3c7..27f4e244 100644
--- a/doc/access-point-api.txt
+++ b/doc/access-point-api.txt
@@ -40,3 +40,8 @@ Methods		void Start(string ssid, string psk)
 Properties	boolean Started [readonly]
 
 			Reflects whether an access point has been started.
+
+		string SSID [readonly]
+
+			The SSID being broadcast for a started AP. This will be
+			an empty string prior to the AP being started.
-- 
2.26.2

[PATCH 5/5] client: ap: show SSID in 'ap <wlan> show'

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 2467 bytes --]

This will show some basic AP information like Started and
SSID. Some cleanup was done to make the AP interface and
client table columns line up.
---
 client/ap.c | 39 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/client/ap.c b/client/ap.c
index 9a7f8642..783f1e19 100644
--- a/client/ap.c
+++ b/client/ap.c
@@ -34,6 +34,7 @@
 
 struct ap {
 	bool started;
+	char *ssid;
 };
 
 static void *ap_create(void)
@@ -74,8 +75,35 @@ static void update_started(void *data, struct l_dbus_message_iter *variant)
 	ap->started = value;
 }
 
+static const char *get_ssid_tostr(const void *data)
+{
+	const struct ap *ap = data;
+
+	if (!ap->ssid)
+		return "";
+
+	return ap->ssid;
+}
+
+static void update_ssid(void *data, struct l_dbus_message_iter *variant)
+{
+	struct ap *ap = data;
+	const char *ssid;
+
+	if (ap->ssid)
+		l_free(ap->ssid);
+
+	if (!l_dbus_message_iter_get_variant(variant, "s", &ssid)) {
+		ap->ssid = NULL;
+		return;
+	}
+
+	ap->ssid = l_strdup(ssid);
+}
+
 static const struct proxy_interface_property ap_properties[] = {
 	{ "Started",  "b", update_started,  get_started_tostr },
+	{ "SSID",     "s", update_ssid, get_ssid_tostr },
 	{ }
 };
 
@@ -221,9 +249,9 @@ static void ap_get_diagnostics_callback(struct l_dbus_message *message,
 
 	while (l_dbus_message_iter_next_entry(&array, &iter)) {
 		sprintf(client_num, "Client %u", idx++);
-		display_table_header(client_num, "%-*s%-*s",
+		display_table_header(client_num, "            %-*s%-*s",
 					20, "Property", 20, "Value");
-		diagnostic_display(&iter, "", 20, 20);
+		diagnostic_display(&iter, "            ", 20, 20);
 		display_table_footer();
 	}
 }
@@ -232,12 +260,17 @@ static enum cmd_status cmd_show(const char *device_name, char **argv, int argc)
 {
 	const struct proxy_interface *ap_diagnostic =
 		device_proxy_find(device_name, IWD_AP_DIAGNOSTIC_INTERFACE);
+	const struct proxy_interface *ap_i =
+		device_proxy_find(device_name, IWD_ACCESS_POINT_INTERFACE);
 
-	if (!ap_diagnostic) {
+	if (!ap_diagnostic || !ap_i) {
 		display("No ap on device: '%s'\n", device_name);
 		return CMD_STATUS_INVALID_VALUE;
 	}
 
+	proxy_properties_display(ap_i, "Access Point Interface", MARGIN, 20, 20);
+	display_table_footer();
+
 	proxy_interface_method_call(ap_diagnostic, "GetDiagnostics", "",
 					ap_get_diagnostics_callback);
 
-- 
2.26.2

Re: [PATCH 5/5] client: ap: show SSID in 'ap <wlan> show'

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

On Mon, 2021-02-01 at 14:05 -0800, James Prestwood wrote:
> This will show some basic AP information like Started and
> SSID. Some cleanup was done to make the AP interface and
> client table columns line up.
> ---
>  client/ap.c | 39 ++++++++++++++++++++++++++++++++++++---
>  1 file changed, 36 insertions(+), 3 deletions(-)
> 
> diff --git a/client/ap.c b/client/ap.c
> index 9a7f8642..783f1e19 100644
> --- a/client/ap.c
> +++ b/client/ap.c
> @@ -34,6 +34,7 @@
>  
>  struct ap {
>  	bool started;
> +	char *ssid;

Looks like I forgot to free this in destroy. Will send v2 after any
comments.

Thanks,
James

Re: [PATCH 4/5] doc: document SSID property on AP interface

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 824 bytes --]

Hi James,

On 2/1/21 4:05 PM, James Prestwood wrote:
> ---
>   doc/access-point-api.txt | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/doc/access-point-api.txt b/doc/access-point-api.txt
> index 9682b3c7..27f4e244 100644
> --- a/doc/access-point-api.txt
> +++ b/doc/access-point-api.txt
> @@ -40,3 +40,8 @@ Methods		void Start(string ssid, string psk)
>   Properties	boolean Started [readonly]
>   
>   			Reflects whether an access point has been started.
> +
> +		string SSID [readonly]

Do we want to call it 'Name' to be symmetrical with Network?
also, [readonly, optional]

> +
> +			The SSID being broadcast for a started AP. This will be
> +			an empty string prior to the AP being started.

Ah I think we should rather omit it if the AP isn't started
> 

Regards,
-Denis