[PATCH v3 1/8] dbus: add helper for appending a dictionary

[PATCH v3 1/8] dbus: add helper for appending a dictionary

[James Prestwood]

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

Arrays of dictionaries are quite common, and for basic
types this API makes things much more convenient by
putting all the enter/append/leave calls in one place.
---
 src/dbus.c | 26 ++++++++++++++++++++++++++
 src/dbus.h |  3 +++
 2 files changed, 29 insertions(+)

diff --git a/src/dbus.c b/src/dbus.c
index ceede5c8..c2f1e1ec 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -202,6 +202,32 @@ void dbus_pending_reply(struct l_dbus_message **msg,
 	*msg = NULL;
 }
 
+/*
+ * Convenience helper for appending a dictionary "{sv}". This only works when
+ * the variant is a basic type.
+ */
+bool dbus_append_dict_basic(struct l_dbus_message_builder *builder,
+				const char *name, char type,
+				const void *data)
+{
+	char strtype[] = { type, '\0' };
+
+	if (!l_dbus_message_builder_enter_dict(builder, "sv"))
+		return false;
+	if (!l_dbus_message_builder_append_basic(builder, 's', name))
+		return false;
+	if (!l_dbus_message_builder_enter_variant(builder, strtype))
+		return false;
+	if (!l_dbus_message_builder_append_basic(builder, type, data))
+		return false;
+	if (!l_dbus_message_builder_leave_variant(builder))
+		return false;
+	if (!l_dbus_message_builder_leave_dict(builder))
+		return false;
+
+	return true;
+}
+
 struct l_dbus *dbus_get_bus(void)
 {
 	return g_dbus;
diff --git a/src/dbus.h b/src/dbus.h
index 3e2018d8..93055bf6 100644
--- a/src/dbus.h
+++ b/src/dbus.h
@@ -51,6 +51,9 @@ struct l_dbus *dbus_get_bus(void);
 
 void dbus_pending_reply(struct l_dbus_message **msg,
 				struct l_dbus_message *reply);
+bool dbus_append_dict_basic(struct l_dbus_message_builder *builder,
+				const char *name, char type,
+				const void *data);
 
 const char *dbus_iftype_to_string(unsigned int iftype);
 
-- 
2.26.2

[PATCH v3 2/8] netdev: add netdev_get_station/current_station

[James Prestwood]

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

This adds a generalized API for GET_STATION. This API handles
calling and parsing the results into a new structure,
netdev_station_info. This results structure will hold any
data needed by consumers of netdev_get_station. A helper API
(netdev_get_current_station) was added as a convenience which
automatically passes handshake->aa as the MAC.

For now only the RSSI is parsed as this is already being
done for RSSI polling/events. Looking further more info will
be added such as rx/tx rates and estimated throughput.
---
 src/netdev.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++
 src/netdev.h |  17 +++++++
 2 files changed, 139 insertions(+)

diff --git a/src/netdev.c b/src/netdev.c
index 3f78afbf..879bd275 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -132,6 +132,11 @@ struct netdev {
 	void *set_powered_user_data;
 	netdev_destroy_func_t set_powered_destroy;
 
+	uint32_t get_station_cmd_id;
+	netdev_get_station_cb_t get_station_cb;
+	void *get_station_data;
+	netdev_destroy_func_t get_station_destroy;
+
 	struct watchlist station_watches;
 
 	struct l_io *pae_io;  /* for drivers without EAPoL over NL80211 */
@@ -363,6 +368,28 @@ int netdev_set_powered(struct netdev *netdev, bool powered,
 	return 0;
 }
 
+static bool netdev_parse_sta_info(struct l_genl_attr *attr,
+					struct netdev_station_info *info)
+{
+	uint16_t type, len;
+	const void *data;
+
+	while (l_genl_attr_next(attr, &type, &len, &data)) {
+		switch (type) {
+		case NL80211_STA_INFO_SIGNAL_AVG:
+			if (len != 1)
+				return false;
+
+			info->cur_rssi = *(const int8_t *) data;
+			info->have_cur_rssi = true;
+
+			break;
+		}
+	}
+
+	return true;
+}
+
 static void netdev_set_rssi_level_idx(struct netdev *netdev)
 {
 	uint8_t new_level;
@@ -636,6 +663,11 @@ static void netdev_free(void *data)
 		netdev->mac_change_cmd_id = 0;
 	}
 
+	if (netdev->get_station_cmd_id) {
+		l_genl_family_cancel(nl80211, netdev->get_station_cmd_id);
+		netdev->get_station_cmd_id = 0;
+	}
+
 	if (netdev->events_ready)
 		WATCHLIST_NOTIFY(&netdev_watches, netdev_watch_func_t,
 					netdev, NETDEV_WATCH_EVENT_DEL);
@@ -4022,6 +4054,96 @@ done:
 	return 0;
 }
 
+static void netdev_get_station_cb(struct l_genl_msg *msg, void *user_data)
+{
+	struct netdev *netdev = user_data;
+	struct l_genl_attr attr, nested;
+	uint16_t type, len;
+	const void *data;
+	struct netdev_station_info info;
+
+	netdev->get_station_cmd_id = 0;
+
+	if (!l_genl_attr_init(&attr, msg))
+		goto parse_error;
+
+	while (l_genl_attr_next(&attr, &type, &len, &data)) {
+		switch (type) {
+		case NL80211_ATTR_STA_INFO:
+			if (!l_genl_attr_recurse(&attr, &nested))
+				goto parse_error;
+
+			if (!netdev_parse_sta_info(&nested, &info))
+				goto parse_error;
+
+			break;
+
+		case NL80211_ATTR_MAC:
+			if (len != 6)
+				goto parse_error;
+
+			memcpy(info.addr, data, 6);
+
+			break;
+		}
+	}
+
+	if (netdev->get_station_cb)
+		netdev->get_station_cb(&info, netdev->get_station_data);
+
+	return;
+
+parse_error:
+	if (netdev->get_station_cb)
+		netdev->get_station_cb(NULL, netdev->get_station_data);
+}
+
+static void netdev_get_station_destroy(void *user_data)
+{
+	struct netdev *netdev = user_data;
+
+	netdev->get_station_cmd_id = 0;
+
+	if (netdev->get_station_destroy)
+		netdev->get_station_destroy(netdev->get_station_data);
+}
+
+int netdev_get_station(struct netdev *netdev, const uint8_t *mac,
+			netdev_get_station_cb_t cb, void *user_data,
+			netdev_destroy_func_t destroy)
+{
+	struct l_genl_msg *msg;
+
+	if (netdev->get_station_cmd_id)
+		return -EBUSY;
+
+	msg = l_genl_msg_new_sized(NL80211_CMD_GET_STATION, 64);
+	l_genl_msg_append_attr(msg, NL80211_ATTR_IFINDEX, 4, &netdev->index);
+	l_genl_msg_append_attr(msg, NL80211_ATTR_MAC, ETH_ALEN, mac);
+
+	netdev->get_station_cmd_id = l_genl_family_send(nl80211, msg,
+						netdev_get_station_cb, netdev,
+						netdev_get_station_destroy);
+	if (!netdev->get_station_cmd_id) {
+		l_genl_msg_unref(msg);
+		return -EIO;
+	}
+
+	netdev->get_station_cb = cb;
+	netdev->get_station_data = user_data;
+	netdev->get_station_destroy = destroy;
+
+	return 0;
+}
+
+int netdev_get_current_station(struct netdev *netdev,
+			netdev_get_station_cb_t cb, void *user_data,
+			netdev_destroy_func_t destroy)
+{
+	return netdev_get_station(netdev, netdev->handshake->aa, cb,
+					user_data, destroy);
+}
+
 static int netdev_cqm_rssi_update(struct netdev *netdev)
 {
 	struct l_genl_msg *msg;
diff --git a/src/netdev.h b/src/netdev.h
index 65fdbaaf..e4a6eee3 100644
--- a/src/netdev.h
+++ b/src/netdev.h
@@ -114,6 +114,16 @@ typedef void (*netdev_station_watch_func_t)(struct netdev *netdev,
 					const uint8_t *mac, bool added,
 					void *user_data);
 
+struct netdev_station_info {
+	uint8_t addr[6];
+	int8_t cur_rssi;
+
+	bool have_cur_rssi : 1;
+};
+
+typedef void (*netdev_get_station_cb_t)(struct netdev_station_info *info,
+					void *user_data);
+
 struct wiphy *netdev_get_wiphy(struct netdev *netdev);
 const uint8_t *netdev_get_address(struct netdev *netdev);
 uint32_t netdev_get_ifindex(struct netdev *netdev);
@@ -174,6 +184,13 @@ int netdev_neighbor_report_req(struct netdev *netdev,
 int netdev_set_rssi_report_levels(struct netdev *netdev, const int8_t *levels,
 					size_t levels_num);
 
+int netdev_get_station(struct netdev *netdev, const uint8_t *mac,
+			netdev_get_station_cb_t cb, void *user_data,
+			netdev_destroy_func_t destroy);
+int netdev_get_current_station(struct netdev *netdev,
+			netdev_get_station_cb_t cb, void *user_data,
+			netdev_destroy_func_t destroy);
+
 void netdev_handshake_failed(struct handshake_state *hs, uint16_t reason_code);
 
 struct netdev *netdev_find(int ifindex);
-- 
2.26.2

[PATCH v3 3/8] netdev: parse rates in netdev_get_station

[James Prestwood]

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

---
 src/netdev.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/netdev.h |  9 ++++++++
 2 files changed, 72 insertions(+)

diff --git a/src/netdev.c b/src/netdev.c
index 879bd275..be24c999 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -368,11 +368,49 @@ int netdev_set_powered(struct netdev *netdev, bool powered,
 	return 0;
 }
 
+static bool netdev_parse_bitrate(struct l_genl_attr *attr, uint32_t *rate_out,
+					uint8_t *mcs_out)
+{
+	uint16_t type, len;
+	const void *data;
+	uint32_t rate = 0;
+	uint8_t mcs = 0;
+
+	while (l_genl_attr_next(attr, &type, &len, &data)) {
+		switch (type) {
+		case NL80211_RATE_INFO_BITRATE32:
+			if (len != 4)
+				return false;
+
+			rate = *(const uint32_t *) data;
+
+			break;
+
+		case NL80211_RATE_INFO_MCS:
+			if (len != 1)
+				return false;
+
+			mcs = *(const uint8_t *) data;
+
+			break;
+		}
+	}
+
+	if (!rate || !mcs)
+		return false;
+
+	*rate_out = rate;
+	*mcs_out = mcs;
+
+	return true;
+}
+
 static bool netdev_parse_sta_info(struct l_genl_attr *attr,
 					struct netdev_station_info *info)
 {
 	uint16_t type, len;
 	const void *data;
+	struct l_genl_attr nested;
 
 	while (l_genl_attr_next(attr, &type, &len, &data)) {
 		switch (type) {
@@ -383,6 +421,31 @@ static bool netdev_parse_sta_info(struct l_genl_attr *attr,
 			info->cur_rssi = *(const int8_t *) data;
 			info->have_cur_rssi = true;
 
+			break;
+		case NL80211_STA_INFO_RX_BITRATE:
+			if (!l_genl_attr_recurse(attr, &nested))
+				return false;
+
+			if (!netdev_parse_bitrate(&nested, &info->rx_bitrate,
+							&info->rx_mcs))
+				return false;
+
+			info->have_rx_bitrate = true;
+			info->have_rx_mcs = true;
+
+			break;
+
+		case NL80211_STA_INFO_TX_BITRATE:
+			if (!l_genl_attr_recurse(attr, &nested))
+				return false;
+
+			if (!netdev_parse_bitrate(&nested, &info->tx_bitrate,
+							&info->tx_mcs))
+				return false;
+
+			info->have_tx_bitrate = true;
+			info->have_tx_mcs = true;
+
 			break;
 		}
 	}
diff --git a/src/netdev.h b/src/netdev.h
index e4a6eee3..a69cd8ff 100644
--- a/src/netdev.h
+++ b/src/netdev.h
@@ -118,7 +118,16 @@ struct netdev_station_info {
 	uint8_t addr[6];
 	int8_t cur_rssi;
 
+	uint32_t rx_bitrate;
+	uint8_t rx_mcs;
+	uint32_t tx_bitrate;
+	uint8_t tx_mcs;
+
 	bool have_cur_rssi : 1;
+	bool have_rx_bitrate : 1;
+	bool have_rx_mcs : 1;
+	bool have_tx_bitrate : 1;
+	bool have_tx_mcs : 1;
 };
 
 typedef void (*netdev_get_station_cb_t)(struct netdev_station_info *info,
-- 
2.26.2

[PATCH v3 4/8] netdev: parse expected throughput in netdev_get_station

[James Prestwood]

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

---
 src/netdev.c | 9 +++++++++
 src/netdev.h | 3 +++
 2 files changed, 12 insertions(+)

diff --git a/src/netdev.c b/src/netdev.c
index be24c999..0618c579 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -446,6 +446,15 @@ static bool netdev_parse_sta_info(struct l_genl_attr *attr,
 			info->have_tx_bitrate = true;
 			info->have_tx_mcs = true;
 
+			break;
+
+		case NL80211_STA_INFO_EXPECTED_THROUGHPUT:
+			if (len != 4)
+				return false;
+
+			info->expected_throughput = *(const uint32_t *)data;
+			info->have_expected_throughput = true;
+
 			break;
 		}
 	}
diff --git a/src/netdev.h b/src/netdev.h
index a69cd8ff..b4835f92 100644
--- a/src/netdev.h
+++ b/src/netdev.h
@@ -123,11 +123,14 @@ struct netdev_station_info {
 	uint32_t tx_bitrate;
 	uint8_t tx_mcs;
 
+	uint32_t expected_throughput;
+
 	bool have_cur_rssi : 1;
 	bool have_rx_bitrate : 1;
 	bool have_rx_mcs : 1;
 	bool have_tx_bitrate : 1;
 	bool have_tx_mcs : 1;
+	bool have_expected_throughput : 1;
 };
 
 typedef void (*netdev_get_station_cb_t)(struct netdev_station_info *info,
-- 
2.26.2

[PATCH v3 5/8] station: create StationDiagnostic interface

[James Prestwood]

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

This interface sits aside the regular station interface but
provides low level connection details for diagnostic and
testing purposes.
---
 src/station.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 112 insertions(+)

diff --git a/src/station.c b/src/station.c
index 1e3706af..a8c9d982 100644
--- a/src/station.c
+++ b/src/station.c
@@ -77,6 +77,7 @@ struct station {
 	struct l_dbus_message *hidden_pending;
 	struct l_dbus_message *disconnect_pending;
 	struct l_dbus_message *scan_pending;
+	struct l_dbus_message *get_station_pending;
 	struct signal_agent *signal_agent;
 	uint32_t dbus_scan_id;
 	uint32_t quick_scan_id;
@@ -3333,6 +3334,9 @@ static struct station *station_create(struct netdev *netdev)
 
 	l_dbus_object_add_interface(dbus, netdev_get_path(netdev),
 					IWD_STATION_INTERFACE, station);
+	l_dbus_object_add_interface(dbus, netdev_get_path(netdev),
+					IWD_STATION_DIAGNOSTIC_INTERFACE,
+					station);
 
 	if (netconfig_enabled)
 		station->netconfig = netconfig_new(netdev_get_ifindex(netdev));
@@ -3455,6 +3459,107 @@ static void station_destroy_interface(void *user_data)
 	station_free(station);
 }
 
+static void station_get_diagnostic_cb(struct netdev_station_info *info,
+					void *user_data)
+{
+	struct station *station = user_data;
+	struct l_dbus_message *reply;
+	struct l_dbus_message_builder *builder;
+	int16_t rssi;
+
+
+	if (!info) {
+		reply = dbus_error_aborted(station->get_station_pending);
+		goto done;
+	}
+
+	reply = l_dbus_message_new_method_return(station->get_station_pending);
+
+	rssi = (int16_t)info->cur_rssi;
+
+	builder = l_dbus_message_builder_new(reply);
+
+	l_dbus_message_builder_enter_array(builder, "{sv}");
+
+	dbus_append_dict_basic(builder, "ConnectedBss", 's',
+					util_address_to_string(info->addr));
+
+	if (info->have_cur_rssi)
+		dbus_append_dict_basic(builder, "RSSI", 'n', &rssi);
+
+	if (info->have_tx_bitrate)
+		dbus_append_dict_basic(builder, "TxBitrate", 'u',
+					&info->tx_bitrate);
+
+	if (info->have_tx_mcs)
+		dbus_append_dict_basic(builder, "TxMCS", 'y', &info->tx_mcs);
+
+	if (info->have_rx_bitrate)
+		dbus_append_dict_basic(builder, "RxBitrate", 'u',
+					&info->rx_bitrate);
+
+	if (info->have_rx_mcs)
+		dbus_append_dict_basic(builder, "RxMCS", 'y', &info->rx_mcs);
+
+	if (info->have_expected_throughput)
+		dbus_append_dict_basic(builder, "ExpectedThroughput", 'u',
+					&info->expected_throughput);
+
+	l_dbus_message_builder_leave_array(builder);
+	l_dbus_message_builder_finalize(builder);
+	l_dbus_message_builder_destroy(builder);
+
+done:
+	dbus_pending_reply(&station->get_station_pending, reply);
+}
+
+static void station_get_diagnostic_destroy(void *user_data)
+{
+	struct station *station = user_data;
+	struct l_dbus_message *reply;
+
+	if (station->get_station_pending) {
+		reply = dbus_error_aborted(station->get_station_pending);
+		dbus_pending_reply(&station->get_station_pending, reply);
+	}
+}
+
+static struct l_dbus_message *station_get_diagnostics(struct l_dbus *dbus,
+						struct l_dbus_message *message,
+						void *user_data)
+{
+	struct station *station = user_data;
+	int ret;
+
+	/*
+	 * At this time all values depend on a connected state.
+	 */
+	if (station->state != STATION_STATE_CONNECTED)
+		return dbus_error_not_connected(message);
+
+	ret = netdev_get_current_station(station->netdev,
+				station_get_diagnostic_cb, station,
+				station_get_diagnostic_destroy);
+	if (ret < 0)
+		return dbus_error_from_errno(ret, message);
+
+	station->get_station_pending = l_dbus_message_ref(message);
+
+	return NULL;
+}
+
+static void station_setup_diagnostic_interface(
+					struct l_dbus_interface *interface)
+{
+	l_dbus_interface_method(interface, "GetDiagnostics", 0,
+				station_get_diagnostics, "a{sv}", "",
+				"diagnostics");
+}
+
+static void station_destroy_diagnostic_interface(void *user_data)
+{
+}
+
 static void station_netdev_watch(struct netdev *netdev,
 				enum netdev_watch_event event, void *userdata)
 {
@@ -3483,6 +3588,11 @@ static int station_init(void)
 	l_dbus_register_interface(dbus_get_bus(), IWD_STATION_INTERFACE,
 					station_setup_interface,
 					station_destroy_interface, false);
+	l_dbus_register_interface(dbus_get_bus(),
+					IWD_STATION_DIAGNOSTIC_INTERFACE,
+					station_setup_diagnostic_interface,
+					station_destroy_diagnostic_interface,
+					false);
 
 	if (!l_settings_get_uint(iwd_get_config(), "General",
 					"ManagementFrameProtection",
@@ -3521,6 +3631,8 @@ static int station_init(void)
 
 static void station_exit(void)
 {
+	l_dbus_unregister_interface(dbus_get_bus(),
+					IWD_STATION_DIAGNOSTIC_INTERFACE);
 	l_dbus_unregister_interface(dbus_get_bus(), IWD_STATION_INTERFACE);
 	netdev_watch_remove(netdev_watch);
 	l_queue_destroy(station_list, NULL);
-- 
2.26.2

[PATCH v3 6/8] test: add a script for GetDiagnostics

[James Prestwood]

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

---
 test/get-diagnostics | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
 create mode 100755 test/get-diagnostics

diff --git a/test/get-diagnostics b/test/get-diagnostics
new file mode 100755
index 00000000..c508c58c
--- /dev/null
+++ b/test/get-diagnostics
@@ -0,0 +1,37 @@
+#!/usr/bin/python3
+
+import sys
+import dbus
+
+# Map dict keys to units. Units can be a function/lambda which should return the
+# entire value with units as a string.
+unit_map = {
+    "ConnectedBss" : None,
+    "RSSI" : "dBm",
+    "RxBitrate" : lambda k : str(100 * int(k)) + ' Kbps',
+    "RxMCS" : lambda i : str(int(i)),
+    "TxBitrate" : lambda k : str(100 * int(k)) + ' Kbps',
+    "TxMCS" : lambda i : str(int(i)),
+    "ExpectedThroughput" : "Kbps"
+}
+
+if (len(sys.argv) != 2):
+    print("Usage: %s <device>" % (sys.argv[0]))
+    sys.exit(1)
+
+bus = dbus.SystemBus()
+device = dbus.Interface(bus.get_object("net.connman.iwd", sys.argv[1]),
+                                    "net.connman.iwd.StationDiagnostic")
+diagnostics = device.GetDiagnostics()
+
+for key, value in diagnostics.items():
+    if key in unit_map:
+        units = unit_map[key]
+        if units is None:
+            units = ''
+        elif callable(units):
+            value = units(value)
+            units = ''
+
+        print('%s : %s %s' % (key, value, units))
+
-- 
2.26.2

[PATCH v3 7/8] netdev: update RSSI polling to use station info parser

[James Prestwood]

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

---
 src/netdev.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/src/netdev.c b/src/netdev.c
index 0618c579..a5e21cb0 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -480,6 +480,7 @@ static void netdev_rssi_poll_cb(struct l_genl_msg *msg, void *user_data)
 	uint16_t type, len;
 	const void *data;
 	bool found;
+	struct netdev_station_info info;
 	uint8_t prev_rssi_level_idx = netdev->cur_rssi_level_idx;
 
 	netdev->rssi_poll_cmd_id = 0;
@@ -492,29 +493,21 @@ static void netdev_rssi_poll_cb(struct l_genl_msg *msg, void *user_data)
 		if (type != NL80211_ATTR_STA_INFO)
 			continue;
 
-		found = true;
-		break;
-	}
-
-	if (!found || !l_genl_attr_recurse(&attr, &nested))
-		goto done;
-
-	found = false;
-	while (l_genl_attr_next(&nested, &type, &len, &data)) {
-		if (type != NL80211_STA_INFO_SIGNAL_AVG)
-			continue;
+		if (!l_genl_attr_recurse(&attr, &nested))
+			goto done;
 
-		if (len != 1)
-			continue;
+		if (!netdev_parse_sta_info(&nested, &info))
+			goto done;
 
 		found = true;
-		netdev->cur_rssi = *(const int8_t *) data;
 		break;
 	}
 
-	if (!found)
+	if (!found || !info.have_cur_rssi)
 		goto done;
 
+	netdev->cur_rssi = info.cur_rssi;
+
 	/*
 	 * Note we don't have to handle LOW_SIGNAL_THRESHOLD here.  The
 	 * CQM single threshold RSSI monitoring should work even if the
-- 
2.26.2

[PATCH v3 8/8] doc: update diagnostics with [optional] tags

[James Prestwood]

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

Some elements, though unlikely, are not required to be included
with the GET_STATION call that GetDiagnostics relies on. mac80211
based drivers include most of these, but other drivers may not.
To be on the safe side all properties except ConnectedBss are now
optional and may not be included.
---
 doc/diagnostics.txt | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/doc/diagnostics.txt b/doc/diagnostics.txt
index c6d2aef0..f94ef6f8 100644
--- a/doc/diagnostics.txt
+++ b/doc/diagnostics.txt
@@ -21,15 +21,15 @@ Methods		dict GetDiagnostics()
 
 			ConnectedBss - MAC address of currently connected BSS.
 
-			RSSI -	The RSSI of the currently connected BSS.
+			RSSI [optional] - The RSSI of the currently connected BSS.
 
-			RxRate - Receive rate in 100kbit/s
+			RxRate [optional] - Receive rate in 100kbit/s
 
-			RxMCS - Receiving MCS index
+			RxMCS [optional] - Receiving MCS index
 
-			TxRate - Transmission rate in 100kbit/s
+			TxRate [optional] - Transmission rate in 100kbit/s
 
-			TxMCS - Transmitting MCS index
+			TxMCS [optional] - Transmitting MCS index
 
 			Possible errors: net.connman.iwd.Busy
 					 net.connman.iwd.Failed
-- 
2.26.2

Re: [PATCH v3 1/8] dbus: add helper for appending a dictionary

[Denis Kenzior]

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

Hi James,

On 1/12/21 11:17 AM, James Prestwood wrote:
> Arrays of dictionaries are quite common, and for basic
> types this API makes things much more convenient by
> putting all the enter/append/leave calls in one place.
> ---
>   src/dbus.c | 26 ++++++++++++++++++++++++++
>   src/dbus.h |  3 +++
>   2 files changed, 29 insertions(+)
> 

Applied, thanks.

Regards,
-Denis

Re: [PATCH v3 2/8] netdev: add netdev_get_station/current_station

[Denis Kenzior]

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

Hi James,

On 1/12/21 11:17 AM, James Prestwood wrote:
> This adds a generalized API for GET_STATION. This API handles
> calling and parsing the results into a new structure,
> netdev_station_info. This results structure will hold any
> data needed by consumers of netdev_get_station. A helper API
> (netdev_get_current_station) was added as a convenience which
> automatically passes handshake->aa as the MAC.
> 
> For now only the RSSI is parsed as this is already being
> done for RSSI polling/events. Looking further more info will
> be added such as rx/tx rates and estimated throughput.
> ---
>   src/netdev.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/netdev.h |  17 +++++++
>   2 files changed, 139 insertions(+)
> 

<snip>

> @@ -114,6 +114,16 @@ typedef void (*netdev_station_watch_func_t)(struct netdev *netdev,
>   					const uint8_t *mac, bool added,
>   					void *user_data);
>   
> +struct netdev_station_info {
> +	uint8_t addr[6];
> +	int8_t cur_rssi;
> +
> +	bool have_cur_rssi : 1;
> +};
> +
> +typedef void (*netdev_get_station_cb_t)(struct netdev_station_info *info,

I made this const struct netdev_station_info *

> +					void *user_data);
> +
>   struct wiphy *netdev_get_wiphy(struct netdev *netdev);
>   const uint8_t *netdev_get_address(struct netdev *netdev);
>   uint32_t netdev_get_ifindex(struct netdev *netdev);

Applied, thanks.

Regards,
-Denis

Re: [PATCH v3 6/8] test: add a script for GetDiagnostics

[Denis Kenzior]

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

Hi James,

On 1/12/21 11:17 AM, James Prestwood wrote:
> ---
>   test/get-diagnostics | 37 +++++++++++++++++++++++++++++++++++++
>   1 file changed, 37 insertions(+)
>   create mode 100755 test/get-diagnostics
> 

Patches 6-8 applied, thanks.

Regards,
-Denis

Re: [PATCH v3 3/8] netdev: parse rates in netdev_get_station

[Denis Kenzior]

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

Hi James,

On 1/12/21 11:17 AM, James Prestwood wrote:
> ---
>   src/netdev.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   src/netdev.h |  9 ++++++++
>   2 files changed, 72 insertions(+)
> 
> diff --git a/src/netdev.c b/src/netdev.c
> index 879bd275..be24c999 100644
> --- a/src/netdev.c
> +++ b/src/netdev.c
> @@ -368,11 +368,49 @@ int netdev_set_powered(struct netdev *netdev, bool powered,
>   	return 0;
>   }
>   
> +static bool netdev_parse_bitrate(struct l_genl_attr *attr, uint32_t *rate_out,
> +					uint8_t *mcs_out)
> +{
> +	uint16_t type, len;
> +	const void *data;
> +	uint32_t rate = 0;
> +	uint8_t mcs = 0;
> +
> +	while (l_genl_attr_next(attr, &type, &len, &data)) {
> +		switch (type) {
> +		case NL80211_RATE_INFO_BITRATE32:
> +			if (len != 4)
> +				return false;
> +
> +			rate = *(const uint32_t *) data;
> +
> +			break;
> +
> +		case NL80211_RATE_INFO_MCS:
> +			if (len != 1)
> +				return false;
> +
> +			mcs = *(const uint8_t *) data;
> +
> +			break;
> +		}
> +	}
> +
> +	if (!rate || !mcs)

So looking at how the kernel reports MCS info, I'm not sure this is entirely 
right.  It seems the kernel can report the VHT MCS index instead of the regular 
one (or HE one).  So the question is, do we want to report the VHT/non-VHT MCS 
indexes separately or report either of them in the mcs index variable.  If the 
latter, then we'd need to add some sort of 'tech attribute' to distinguish what 
MCS is being meant.  i.e. is it a VHT (802.11ac) MCS index or not.

We should also probably report the bandwidth (5/10/20/40/80/160, etc)

> +		return false;
> +
> +	*rate_out = rate;
> +	*mcs_out = mcs;
> +
> +	return true;
> +}
> +

Regards,
-Denis