diff mbox series

[1/3] station: add channel number to diagnostics message

Message ID 20240221214624.601337-1-ram.subramanian@getcruise.com (mailing list archive)
State New
Headers show
Series [1/3] station: add channel number to diagnostics message | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
prestwoj/iwd-alpine-ci-fetch success Fetch PR
prestwoj/iwd-ci-gitlint success GitLint
prestwoj/iwd-ci-fetch success Fetch PR
prestwoj/iwd-ci-makedistcheck success Make Distcheck
prestwoj/iwd-alpine-ci-makedistcheck success Make Distcheck
prestwoj/iwd-ci-build success Build - Configure
prestwoj/iwd-alpine-ci-build success Build - Configure
prestwoj/iwd-ci-makecheckvalgrind success Make Check w/Valgrind
prestwoj/iwd-ci-clang success clang PASS
prestwoj/iwd-ci-makecheck success Make Check
prestwoj/iwd-alpine-ci-makecheckvalgrind success Make Check w/Valgrind
prestwoj/iwd-alpine-ci-makecheck success Make Check
prestwoj/iwd-ci-incremental_build success Incremental Build with patches
prestwoj/iwd-alpine-ci-incremental_build success Incremental Build with patches
prestwoj/iwd-ci-testrunner success test-runner PASS

Commit Message

Ram Subramanian Feb. 21, 2024, 9:46 p.m. UTC
As a small convenience to the user.
---
 src/station.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

James Prestwood Feb. 22, 2024, 2:27 p.m. UTC | #1
Hi Ram,

On 2/21/24 1:46 PM, Ram Subramanian wrote:
> As a small convenience to the user.
> ---
>   src/station.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/src/station.c b/src/station.c
> index ea505ca2..39b98037 100644
> --- a/src/station.c
> +++ b/src/station.c
> @@ -4702,12 +4702,16 @@ static void station_get_diagnostic_cb(
>   	struct l_dbus_message *reply;
>   	struct l_dbus_message_builder *builder;
>   	struct handshake_state *hs = netdev_get_handshake(station->netdev);
> +	uint8_t channel_num;
>   
>   	if (!info) {
>   		reply = dbus_error_aborted(station->get_station_pending);
>   		goto done;
>   	}
>   
> +	channel_num = band_freq_to_channel(station->connected_bss->frequency,
> +						NULL);
> +
>   	reply = l_dbus_message_new_method_return(station->get_station_pending);
>   
>   	builder = l_dbus_message_builder_new(reply);
> @@ -4718,6 +4722,10 @@ static void station_get_diagnostic_cb(
>   					util_address_to_string(info->addr));
>   	dbus_append_dict_basic(builder, "Frequency", 'u',
>   				&station->connected_bss->frequency);
> +
> +	if (channel_num != 0)
> +		dbus_append_dict_basic(builder, "Channel", 'y', &channel_num);
> +
>   	dbus_append_dict_basic(builder, "Security", 's',
>   				diagnostic_akm_suite_to_security(hs->akm_suite,
>   								hs->wpa_ie));

These all LGTM

Thanks,

James
Marcel Holtmann Feb. 22, 2024, 2:37 p.m. UTC | #2
Hi Ram,

> As a small convenience to the user.
> ---
> src/station.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
> 
> diff --git a/src/station.c b/src/station.c
> index ea505ca2..39b98037 100644
> --- a/src/station.c
> +++ b/src/station.c
> @@ -4702,12 +4702,16 @@ static void station_get_diagnostic_cb(
> struct l_dbus_message *reply;
> struct l_dbus_message_builder *builder;
> struct handshake_state *hs = netdev_get_handshake(station->netdev);
> + uint8_t channel_num;
> 
> if (!info) {
> reply = dbus_error_aborted(station->get_station_pending);
> goto done;
> }
> 
> + channel_num = band_freq_to_channel(station->connected_bss->frequency,
> + NULL);
> +
> reply = l_dbus_message_new_method_return(station->get_station_pending);
> 
> builder = l_dbus_message_builder_new(reply);
> @@ -4718,6 +4722,10 @@ static void station_get_diagnostic_cb(
> util_address_to_string(info->addr));
> dbus_append_dict_basic(builder, "Frequency", 'u',
> &station->connected_bss->frequency);
> +
> + if (channel_num != 0)
> + dbus_append_dict_basic(builder, "Channel", 'y', &channel_num);
> +

we are already at channel number 233 right now. Since this is public API and not some internal variable that we can change, it might be better to just go to uint16 here to be future proof.

Regards

Marcel
Ram Subramanian Feb. 23, 2024, 8:11 p.m. UTC | #3
Good point, Marcel! I've submitted v2 with the uint16_t change.

And thank you for the review James!

Best,
Ram
diff mbox series

Patch

diff --git a/src/station.c b/src/station.c
index ea505ca2..39b98037 100644
--- a/src/station.c
+++ b/src/station.c
@@ -4702,12 +4702,16 @@  static void station_get_diagnostic_cb(
 	struct l_dbus_message *reply;
 	struct l_dbus_message_builder *builder;
 	struct handshake_state *hs = netdev_get_handshake(station->netdev);
+	uint8_t channel_num;
 
 	if (!info) {
 		reply = dbus_error_aborted(station->get_station_pending);
 		goto done;
 	}
 
+	channel_num = band_freq_to_channel(station->connected_bss->frequency,
+						NULL);
+
 	reply = l_dbus_message_new_method_return(station->get_station_pending);
 
 	builder = l_dbus_message_builder_new(reply);
@@ -4718,6 +4722,10 @@  static void station_get_diagnostic_cb(
 					util_address_to_string(info->addr));
 	dbus_append_dict_basic(builder, "Frequency", 'u',
 				&station->connected_bss->frequency);
+
+	if (channel_num != 0)
+		dbus_append_dict_basic(builder, "Channel", 'y', &channel_num);
+
 	dbus_append_dict_basic(builder, "Security", 's',
 				diagnostic_akm_suite_to_security(hs->akm_suite,
 								hs->wpa_ie));