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

[Denis Kenzior <denkenz@gmail.com>]

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

Hi James,

On 1/21/21 2:05 PM, James Prestwood wrote:
> Hi,
> 
>>> +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?
> 
> I could do that. Its the same for station/ap. The reason I put it here
> is because its not really a generalized display API since its so unique
> to this specific value. But I'm fine either way, I'll put it in
> display.c/h so we don't have it duplicated.

Could also put this into diagnostic.[ch] or something as well.  In theory even 
the dict_mapping parts can be shared since they're mostly the same, and 
Address/CurrentBss won't occur in the same message.

> 
>>
>>> +
>>> +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?
> 
> That function does more than just check (though you wouldn't guess it
> from the name). It ends up converting the error into a string and
> printing it.

Ok, cool.

Regards,
-Denis