Re: [PATCH v2 4/5] client: implement "ap <wlan> show"

[Denis Kenzior <denkenz@gmail.com>]

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

Hi James,

On 1/21/21 12:11 PM, James Prestwood wrote:
> This command uses GetDiagnostics to show a list of connected
> clients and some information about them. The information
> contained for each connected station nearly maps 1:1 with the
> station diagnostics information shown in "station <wlan> show"
> apart from "ConnectedBss" which is now "Address".
> ---
>   client/ap.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 73 insertions(+)
> 
> diff --git a/client/ap.c b/client/ap.c
> index a6a2681b..6e2e96c4 100644
> --- a/client/ap.c
> +++ b/client/ap.c
> @@ -189,6 +189,76 @@ static enum cmd_status cmd_stop(const char *device_name, char **argv, int argc)
>   	return CMD_STATUS_TRIGGERED;
>   }
>   
> +static struct proxy_interface_type ap_diagnostic_interface_type = {
> +	.interface = IWD_AP_DIAGNOSTIC_INTERFACE,
> +};
> +
> +static void display_bitrate_100kbps(struct l_dbus_message_iter *variant,
> +				const char *key, const char *margin,
> +				int name_column_width, int value_column_width)
> +{
> +	uint32_t rate;
> +
> +	l_dbus_message_iter_get_variant(variant, "u", &rate);
> +
> +	display("%s%-*s%-*u Kbit/s\n", margin, name_column_width, key,
> +			value_column_width, rate * 100);
> +}

You don't want this in display.[ch] somewhere?

> +
> +static const struct display_dict_mapping diagnostic_mapping[] = {
> +	{ "Address", 's', NULL },
> +	{ "RxMode", 's', NULL },
> +	{ "TxMode", 's', NULL },
> +	{ "RxBitrate", 0, display_bitrate_100kbps },
> +	{ "TxBitrate", 0, display_bitrate_100kbps },
> +	{ "ExpectedThroughput", 'u', "Kbit/s" },
> +	{ "RSSI", 'n', "dBm" },
> +	{ "RxMCS", 'y', NULL },
> +	{ "TxMCS", 'y', NULL},
> +	{ NULL }
> +};
> +
> +static void ap_get_diagnostics_callback(struct l_dbus_message *message,
> +					void *user_data)
> +{
> +	struct l_dbus_message_iter array;
> +	struct l_dbus_message_iter iter;
> +	uint16_t idx = 0;
> +	char client_num[15];
> +
> +	if (dbus_message_has_error(message))
> +		return;

Does this need to print something as well?

> +
> +	if (!l_dbus_message_get_arguments(message, "aa{sv}", &array)) {
> +		display("Failed to parse GetDiagnostics message");
> +		return;
> +	}
> +
> +	while (l_dbus_message_iter_next_entry(&array, &iter)) {
> +		sprintf(client_num, "Client %u", idx++);
> +		display_table_header(client_num, "%-*s%-*s",
> +					20, "Property", 20, "Value");
> +		display_dictionary(&iter, diagnostic_mapping, "", 20, 20);
> +		display_table_footer();
> +	}
> +}
> +
> +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);
> +
> +	if (!ap_diagnostic) {
> +		display("No ap on device: '%s'\n", device_name);
> +		return CMD_STATUS_INVALID_VALUE;
> +	}
> +
> +	proxy_interface_method_call(ap_diagnostic, "GetDiagnostics", "",
> +					ap_get_diagnostics_callback);
> +
> +	return CMD_STATUS_TRIGGERED;
> +}
> +
>   static const struct command ap_commands[] = {
>   	{ NULL, "list", NULL, cmd_list, "List devices in AP mode", true },
>   	{ "<wlan>", "start", "<\"network name\"> <passphrase>", cmd_start,

Regards,
-Denis