[PATCH wpan-tools 0/2] Update cca mode and options output

[PATCH wpan-tools 0/2] Update cca mode and options output

[Christoffer Holmstedt]

Earlier cca_modes and cca_opts were printed separately but these two patches
now prints them together so cca_opts are only printed when the specified mode
 is (3).

Output for "current" mode (only one line is printed for each wpan):
	cca_mode: 1, energy above threshold.
	cca_mode: 2, carrier sense only.
	cca_mode: 3, carrier sense with energy above threshold (logical operator is 'and').
	cca_mode: 3, carrier sense with energy above threshold (logical operator is 'or').
	cca_mode: 3, carrier sense with energy above threshold (logical operator is unknown).
	cca_mode: 4, ALOHA.
	cca_mode: 5, UWB preamble sense based on the SHR of a frame.
	cca_mode: 6, UWB preamble sense based on the packet with the multiplexed preamble.

capabilities:
	...
	cca_modes:
		(1) energy above threshold
		(2) carrier sense only
		(3, cca_opt: 0) carrier sense with energy above threshold (logical operator is 'and')
		(3, cca_opt: 1) carrier sense with energy above threshold (logical operator is 'or')


Christoffer Holmstedt (2):
  info: add cca mode descriptive text to output
  info: remove old cca options capabilities output

 src/info.c | 99 +++++++++++++++++++++++++++++++++++++-------------------------
 1 file changed, 60 insertions(+), 39 deletions(-)

-- 
1.9.1


-- 
Christoffer Holmstedt

Re: [PATCH wpan-tools 0/2] Update cca mode and options output

[Alexander Aring]

On Thu, Jun 04, 2015 at 11:20:06AM +0200, Christoffer Holmstedt wrote:
> Earlier cca_modes and cca_opts were printed separately but these two patches
> now prints them together so cca_opts are only printed when the specified mode
>  is (3).
> 
> Output for "current" mode (only one line is printed for each wpan):
> 	cca_mode: 1, energy above threshold.
> 	cca_mode: 2, carrier sense only.
> 	cca_mode: 3, carrier sense with energy above threshold (logical operator is 'and').
> 	cca_mode: 3, carrier sense with energy above threshold (logical operator is 'or').
> 	cca_mode: 3, carrier sense with energy above threshold (logical operator is unknown).
> 	cca_mode: 4, ALOHA.
> 	cca_mode: 5, UWB preamble sense based on the SHR of a frame.
> 	cca_mode: 6, UWB preamble sense based on the packet with the multiplexed preamble.
> 
> capabilities:
> 	...
> 	cca_modes:
> 		(1) energy above threshold
> 		(2) carrier sense only
> 		(3, cca_opt: 0) carrier sense with energy above threshold (logical operator is 'and')
> 		(3, cca_opt: 1) carrier sense with energy above threshold (logical operator is 'or')
> 
> 

Please check your mail settings, can you try to make some "In-reply-to"
behaviour when you send more than one patch?

- Alex

Re: [PATCH wpan-tools 0/2] Update cca mode and options output

[Christoffer Holmstedt]

On Thu, Jun 04, 2015 at 01:02:15PM +0200, Alexander Aring wrote:
> On Thu, Jun 04, 2015 at 11:20:06AM +0200, Christoffer Holmstedt wrote:
> > Earlier cca_modes and cca_opts were printed separately but these two patches
> > now prints them together so cca_opts are only printed when the specified mode
> >  is (3).
> > 
> > Output for "current" mode (only one line is printed for each wpan):
> > 	cca_mode: 1, energy above threshold.
> > 	cca_mode: 2, carrier sense only.
> > 	cca_mode: 3, carrier sense with energy above threshold (logical operator is 'and').
> > 	cca_mode: 3, carrier sense with energy above threshold (logical operator is 'or').
> > 	cca_mode: 3, carrier sense with energy above threshold (logical operator is unknown).
> > 	cca_mode: 4, ALOHA.
> > 	cca_mode: 5, UWB preamble sense based on the SHR of a frame.
> > 	cca_mode: 6, UWB preamble sense based on the packet with the multiplexed preamble.
> > 
> > capabilities:
> > 	...
> > 	cca_modes:
> > 		(1) energy above threshold
> > 		(2) carrier sense only
> > 		(3, cca_opt: 0) carrier sense with energy above threshold (logical operator is 'and')
> > 		(3, cca_opt: 1) carrier sense with energy above threshold (logical operator is 'or')
> > 
> > 
> 
> Please check your mail settings, can you try to make some "In-reply-to"
> behaviour when you send more than one patch?
> 
> - Alex

Sure, I will try to fix that. I just sent three different emails with "mutt -H"
now but will configure git and use git send-email instead in the future I think
that will fix the "In-reply-to" problem.

-- 
Christoffer Holmstedt

Re: [PATCH wpan-tools 0/2] Update cca mode and options output

[Varka Bhadram]

Hi Christoffer Holmstedt,

On 06/04/2015 02:50 PM, Christoffer Holmstedt wrote:

> Earlier cca_modes and cca_opts were printed separately but these two patches
> now prints them together so cca_opts are only printed when the specified mode
>   is (3).
>
> Output for "current" mode (only one line is printed for each wpan):
> 	cca_mode: 1, energy above threshold.
> 	cca_mode: 2, carrier sense only.
> 	cca_mode: 3, carrier sense with energy above threshold (logical operator is 'and').
> 	cca_mode: 3, carrier sense with energy above threshold (logical operator is 'or').
> 	cca_mode: 3, carrier sense with energy above threshold (logical operator is unknown).
> 	cca_mode: 4, ALOHA.
> 	cca_mode: 5, UWB preamble sense based on the SHR of a frame.
> 	cca_mode: 6, UWB preamble sense based on the packet with the multiplexed preamble.

Do you think that cca_mode number is required overhear..?

What i mean is that:
	The cca mode number 1 indicates that its *energy above threshold*,
So what i feel is that printing the number is required.

May be Alex has to tell its preferred or not. I think we can remove the number.

>
> capabilities:
> 	...
> 	cca_modes:
> 		(1) energy above threshold
> 		(2) carrier sense only
> 		(3, cca_opt: 0) carrier sense with energy above threshold (logical operator is 'and')
> 		(3, cca_opt: 1) carrier sense with energy above threshold (logical operator is 'or')
>
>
> Christoffer Holmstedt (2):
>    info: add cca mode descriptive text to output
>    info: remove old cca options capabilities output
>
>   src/info.c | 99 +++++++++++++++++++++++++++++++++++++-------------------------
>   1 file changed, 60 insertions(+), 39 deletions(-)
>

-- 
Varka Bhadram

Re: [PATCH wpan-tools 0/2] Update cca mode and options output

[Christoffer Holmstedt]

On Thu, Jun 04, 2015 at 05:21:25PM +0530, Varka Bhadram wrote:
> Hi Christoffer Holmstedt,
> 
> On 06/04/2015 02:50 PM, Christoffer Holmstedt wrote:
> 
> >Earlier cca_modes and cca_opts were printed separately but these two patches
> >now prints them together so cca_opts are only printed when the specified mode
> >  is (3).
> >
> >Output for "current" mode (only one line is printed for each wpan):
> >	cca_mode: 1, energy above threshold.
> >	cca_mode: 2, carrier sense only.
> >	cca_mode: 3, carrier sense with energy above threshold (logical operator is 'and').
> >	cca_mode: 3, carrier sense with energy above threshold (logical operator is 'or').
> >	cca_mode: 3, carrier sense with energy above threshold (logical operator is unknown).
> >	cca_mode: 4, ALOHA.
> >	cca_mode: 5, UWB preamble sense based on the SHR of a frame.
> >	cca_mode: 6, UWB preamble sense based on the packet with the multiplexed preamble.
> 
> Do you think that cca_mode number is required overhear..?
> 
> What i mean is that:
> 	The cca mode number 1 indicates that its *energy above threshold*,
> So what i feel is that printing the number is required.
> 
> May be Alex has to tell its preferred or not. I think we can remove the number.
> 
...cut...
> -- 
> Varka Bhadram
> 

I was convinced before that I needed it there, but then I tried to make my
case and I ended up with the conclusion that the user don't need it. Only
scenario it is needed is when the user wants to change it and it that case the
number is listed in the capabilities section.

I will remove the number if no one objects.

-- 
Christoffer Holmstedt

[PATCH v2 wpan-tools 0/2] Update CCA mode and CCA options output

[christoffer]

This patch series updates the CCA output text when running "iwpan list".
Instead of the single digits indicating mode the output is now strings
describing the mode and options.

Changes v1 -> v2:
* cca_mode number is removed from "current cca_mode", only text is printed as
an example:

	cca_mode: Energy above threshold
	was
	cca_mode: 1, Energy above threshold

* Changed "enum" to string mapping in print_cca_mode_handler. Instead of
printing out the strings in the function I return a tempbuf that is formatted
by the caller.

* Added default value to cca_opt when printing current cca_mode and cca_opt. It
is set to the "random" value of 99 as the enum only has 2 values, is this a
proper solution? (or should we use limits.h with MAX_INT perhaps?)

PS. Also configured git send-email so trying it out now to the wpan-list and
hope it works this time wiht proper In-Reply-To header.

[PATCH v2 wpan-tools 1/2] info: add cca mode descriptive text to output

[christoffer]

From: Christoffer Holmstedt <christoffer@christofferholmstedt.se>

Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
---
 src/info.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 63 insertions(+), 18 deletions(-)

diff --git a/src/info.c b/src/info.c
index 2434bef6a87f..364c02f718aa 100644
--- a/src/info.c
+++ b/src/info.c
@@ -146,6 +146,41 @@ static void print_freq_handler(int channel_page, int channel)
 	}
 }
 
+static char cca_mode_buf[100];
+
+const char *print_cca_mode_handler(enum nl802154_cca_modes cca_mode,
+				   enum nl802154_cca_opts cca_opt)
+{
+	switch (cca_mode) {
+	case NL802154_CCA_ENERGY:
+		return "Energy above threshold";
+	case NL802154_CCA_CARRIER:
+		return "Carrier sense only";
+	case NL802154_CCA_ENERGY_CARRIER:
+		switch (cca_opt) {
+		case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
+			return "Carrier sense with energy above threshold (logical operator is 'and')";
+		case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
+			return "Carrier sense with energy above threshold (logical operator is 'or')";
+		default:
+			sprintf(cca_mode_buf,
+				"Unknown CCA option (%d) for CCA mode (%d)",
+				cca_opt,
+				cca_mode);
+			return cca_mode_buf;
+		}
+	case NL802154_CCA_ALOHA:
+		return "ALOHA";
+	case NL802154_CCA_UWB_SHR:
+		return "UWB preamble sense based on the SHR of a frame";
+	case NL802154_CCA_UWB_MULTIPEXED:
+		return "UWB preamble sense based on the packet with the multiplexed preamble";
+	default:
+		sprintf(cca_mode_buf, "Unknown CCA mode (%d)", cca_mode);
+		return cca_mode_buf;
+	}
+}
+
 static const char *commands[NL802154_CMD_MAX + 1] = {
 	[NL802154_CMD_UNSPEC] = "unspec",
 	[NL802154_CMD_GET_WPAN_PHY] = "get_wpan_phy",
@@ -235,23 +270,13 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
 	}
 
 	if (tb_msg[NL802154_ATTR_CCA_MODE]) {
+		enum nl802154_cca_opts cca_opt = 99;
+		if (tb_msg[NL802154_ATTR_CCA_OPT])
+			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
+
 		cca_mode = nla_get_u32(tb_msg[NL802154_ATTR_CCA_MODE]);
-		printf("cca_mode: %d", cca_mode);
-		if (cca_mode == NL802154_CCA_ENERGY_CARRIER) {
-			enum nl802154_cca_opts cca_opt;
 
-			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
-			switch (cca_opt) {
-			case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
-				printf(" logical and ");
-				break;
-			case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
-				printf(" logical or ");
-				break;
-			default:
-				printf(" logical op mode unkown ");
-			}
-		}
+		printf("cca_mode: %s", print_cca_mode_handler(cca_mode, cca_opt));
 		printf("\n");
 	}
 
@@ -361,10 +386,30 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
 			printf("\tcca_modes: ");
 			nla_for_each_nested(nl_cca_modes,
 					    tb_caps[NL802154_CAP_ATTR_CCA_MODES],
-					    rem_cca_modes)
-				printf("%d,", nla_type(nl_cca_modes));
+					    rem_cca_modes) {
+				if (nla_type(nl_cca_modes) != NL802154_CCA_ENERGY_CARRIER) {
+					printf("\n\t\t(%d) %s",
+						nla_type(nl_cca_modes),
+						print_cca_mode_handler(nla_type(nl_cca_modes), 0));
+				} else {
+					if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
+						struct nlattr *nl_cca_opts;
+						int rem_cca_opts;
+
+						nla_for_each_nested(nl_cca_opts,
+								    tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
+								    rem_cca_opts) {
+
+							printf("\n\t\t(%d, cca_opt: %d) %s",
+								nla_type(nl_cca_modes),
+								nla_type(nl_cca_opts),
+								print_cca_mode_handler(nla_type(nl_cca_modes), nla_type(nl_cca_opts)));
+						}
+					}
+				}
+			}
 			/* TODO */
-			printf("\b \n");
+			printf("\n");
 		}
 
 		if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
-- 
2.1.4

[PATCH v2 wpan-tools 2/2] info: remove old cca options capabilities output

[christoffer]

From: Christoffer Holmstedt <christoffer@christofferholmstedt.se>

Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
---
 src/info.c | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/src/info.c b/src/info.c
index 364c02f718aa..e70f33da8997 100644
--- a/src/info.c
+++ b/src/info.c
@@ -412,29 +412,6 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
 			printf("\n");
 		}
 
-		if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
-			struct nlattr *nl_cca_opts;
-			int rem_cca_opts;
-
-			printf("\tcca_opts: ");
-			nla_for_each_nested(nl_cca_opts,
-					    tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
-					    rem_cca_opts) {
-				printf("%d", nla_type(nl_cca_opts));
-				switch (nla_type(nl_cca_opts)) {
-				case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
-				case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
-					printf("(cca_mode: 3),");
-					break;
-				default:
-					printf("unkown\n");
-					break;
-				}
-			}
-			/* TODO */
-			printf("\b \n");
-		}
-
 		if (tb_caps[NL802154_CAP_ATTR_MIN_MINBE] &&
 		    tb_caps[NL802154_CAP_ATTR_MAX_MINBE] &&
 		    tb_caps[NL802154_CAP_ATTR_MIN_MAXBE] &&
-- 
2.1.4

[PATCH v3 wpan-tools 0/3] Update CCA mode and CCA options output

[Christoffer Holmstedt]

This patch series updates the CCA output text when running "iwpan list".
Instead of the single digits indicating mode the output is now strings
describing the mode and options.

Changes v2 -> v3:
* Fixed misspelled enum in nl802154

Changes v1 -> v2:
* cca_mode number is removed from "current cca_mode", only text is printed as
an example:

        cca_mode: Energy above threshold
        was
        cca_mode: 1, Energy above threshold

* Changed "enum" to string mapping in print_cca_mode_handler. Instead of
printing out the strings in the function I return a tempbuf that is formatted
by the caller.

* Added default value to cca_opt when printing current cca_mode and cca_opt. It
is set to the "random" value of 99 as the enum only has 2 values, is this a
proper solution? (or should we use limits.h with MAX_INT perhaps?)

[PATCH v3 wpan-tools 1/3] info: add cca mode descriptive text to output

[Christoffer Holmstedt]

Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
---
 src/info.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 63 insertions(+), 18 deletions(-)

diff --git a/src/info.c b/src/info.c
index 2434bef6a87f..364c02f718aa 100644
--- a/src/info.c
+++ b/src/info.c
@@ -146,6 +146,41 @@ static void print_freq_handler(int channel_page, int channel)
 	}
 }
 
+static char cca_mode_buf[100];
+
+const char *print_cca_mode_handler(enum nl802154_cca_modes cca_mode,
+				   enum nl802154_cca_opts cca_opt)
+{
+	switch (cca_mode) {
+	case NL802154_CCA_ENERGY:
+		return "Energy above threshold";
+	case NL802154_CCA_CARRIER:
+		return "Carrier sense only";
+	case NL802154_CCA_ENERGY_CARRIER:
+		switch (cca_opt) {
+		case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
+			return "Carrier sense with energy above threshold (logical operator is 'and')";
+		case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
+			return "Carrier sense with energy above threshold (logical operator is 'or')";
+		default:
+			sprintf(cca_mode_buf,
+				"Unknown CCA option (%d) for CCA mode (%d)",
+				cca_opt,
+				cca_mode);
+			return cca_mode_buf;
+		}
+	case NL802154_CCA_ALOHA:
+		return "ALOHA";
+	case NL802154_CCA_UWB_SHR:
+		return "UWB preamble sense based on the SHR of a frame";
+	case NL802154_CCA_UWB_MULTIPEXED:
+		return "UWB preamble sense based on the packet with the multiplexed preamble";
+	default:
+		sprintf(cca_mode_buf, "Unknown CCA mode (%d)", cca_mode);
+		return cca_mode_buf;
+	}
+}
+
 static const char *commands[NL802154_CMD_MAX + 1] = {
 	[NL802154_CMD_UNSPEC] = "unspec",
 	[NL802154_CMD_GET_WPAN_PHY] = "get_wpan_phy",
@@ -235,23 +270,13 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
 	}
 
 	if (tb_msg[NL802154_ATTR_CCA_MODE]) {
+		enum nl802154_cca_opts cca_opt = 99;
+		if (tb_msg[NL802154_ATTR_CCA_OPT])
+			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
+
 		cca_mode = nla_get_u32(tb_msg[NL802154_ATTR_CCA_MODE]);
-		printf("cca_mode: %d", cca_mode);
-		if (cca_mode == NL802154_CCA_ENERGY_CARRIER) {
-			enum nl802154_cca_opts cca_opt;
 
-			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
-			switch (cca_opt) {
-			case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
-				printf(" logical and ");
-				break;
-			case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
-				printf(" logical or ");
-				break;
-			default:
-				printf(" logical op mode unkown ");
-			}
-		}
+		printf("cca_mode: %s", print_cca_mode_handler(cca_mode, cca_opt));
 		printf("\n");
 	}
 
@@ -361,10 +386,30 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
 			printf("\tcca_modes: ");
 			nla_for_each_nested(nl_cca_modes,
 					    tb_caps[NL802154_CAP_ATTR_CCA_MODES],
-					    rem_cca_modes)
-				printf("%d,", nla_type(nl_cca_modes));
+					    rem_cca_modes) {
+				if (nla_type(nl_cca_modes) != NL802154_CCA_ENERGY_CARRIER) {
+					printf("\n\t\t(%d) %s",
+						nla_type(nl_cca_modes),
+						print_cca_mode_handler(nla_type(nl_cca_modes), 0));
+				} else {
+					if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
+						struct nlattr *nl_cca_opts;
+						int rem_cca_opts;
+
+						nla_for_each_nested(nl_cca_opts,
+								    tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
+								    rem_cca_opts) {
+
+							printf("\n\t\t(%d, cca_opt: %d) %s",
+								nla_type(nl_cca_modes),
+								nla_type(nl_cca_opts),
+								print_cca_mode_handler(nla_type(nl_cca_modes), nla_type(nl_cca_opts)));
+						}
+					}
+				}
+			}
 			/* TODO */
-			printf("\b \n");
+			printf("\n");
 		}
 
 		if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
-- 
2.1.4

[PATCH v3 wpan-tools 2/3] info: remove old cca options capabilities output

[Christoffer Holmstedt]

Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
---
 src/info.c | 23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/src/info.c b/src/info.c
index 364c02f718aa..e70f33da8997 100644
--- a/src/info.c
+++ b/src/info.c
@@ -412,29 +412,6 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
 			printf("\n");
 		}
 
-		if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
-			struct nlattr *nl_cca_opts;
-			int rem_cca_opts;
-
-			printf("\tcca_opts: ");
-			nla_for_each_nested(nl_cca_opts,
-					    tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
-					    rem_cca_opts) {
-				printf("%d", nla_type(nl_cca_opts));
-				switch (nla_type(nl_cca_opts)) {
-				case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
-				case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
-					printf("(cca_mode: 3),");
-					break;
-				default:
-					printf("unkown\n");
-					break;
-				}
-			}
-			/* TODO */
-			printf("\b \n");
-		}
-
 		if (tb_caps[NL802154_CAP_ATTR_MIN_MINBE] &&
 		    tb_caps[NL802154_CAP_ATTR_MAX_MINBE] &&
 		    tb_caps[NL802154_CAP_ATTR_MIN_MAXBE] &&
-- 
2.1.4

[PATCH v3 wpan-tools 3/3] nl802154: fix misspelled enum

[Christoffer Holmstedt]

This patch updates nl802154 enum to match the one in bluetooth-next tree from
commit d446278c408b ("nl802154: fix misspelled enum").

Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
---
 src/info.c     | 2 +-
 src/nl802154.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/info.c b/src/info.c
index e70f33da8997..21b7e9e9f64c 100644
--- a/src/info.c
+++ b/src/info.c
@@ -173,7 +173,7 @@ const char *print_cca_mode_handler(enum nl802154_cca_modes cca_mode,
 		return "ALOHA";
 	case NL802154_CCA_UWB_SHR:
 		return "UWB preamble sense based on the SHR of a frame";
-	case NL802154_CCA_UWB_MULTIPEXED:
+	case NL802154_CCA_UWB_MULTIPLEXED:
 		return "UWB preamble sense based on the packet with the multiplexed preamble";
 	default:
 		sprintf(cca_mode_buf, "Unknown CCA mode (%d)", cca_mode);
diff --git a/src/nl802154.h b/src/nl802154.h
index 6fc231e60920..b0ab530d28cd 100644
--- a/src/nl802154.h
+++ b/src/nl802154.h
@@ -187,7 +187,7 @@ enum nl802154_wpan_phy_capability_attr {
  * @NL802154_CCA_ENERGY_CARRIER: Carrier sense with energy above threshold
  * @NL802154_CCA_ALOHA: CCA shall always report an idle medium
  * @NL802154_CCA_UWB_SHR: UWB preamble sense based on the SHR of a frame
- * @NL802154_CCA_UWB_MULTIPEXED: UWB preamble sense based on the packet with
+ * @NL802154_CCA_UWB_MULTIPLEXED: UWB preamble sense based on the packet with
  *	the multiplexed preamble
  * @__NL802154_CCA_ATTR_AFTER_LAST: Internal
  * @NL802154_CCA_ATTR_MAX: Maximum CCA attribute number
@@ -199,7 +199,7 @@ enum nl802154_cca_modes {
 	NL802154_CCA_ENERGY_CARRIER,
 	NL802154_CCA_ALOHA,
 	NL802154_CCA_UWB_SHR,
-	NL802154_CCA_UWB_MULTIPEXED,
+	NL802154_CCA_UWB_MULTIPLEXED,
 
 	/* keep last */
 	__NL802154_CCA_ATTR_AFTER_LAST,
-- 
2.1.4

Re: [PATCH v3 wpan-tools 1/3] info: add cca mode descriptive text to output

[Alexander Aring]

On Wed, Jun 10, 2015 at 01:18:07PM +0200, Christoffer Holmstedt wrote:
> Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
> ---
>  src/info.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 63 insertions(+), 18 deletions(-)
> 
> diff --git a/src/info.c b/src/info.c
> index 2434bef6a87f..364c02f718aa 100644
> --- a/src/info.c
> +++ b/src/info.c
> @@ -146,6 +146,41 @@ static void print_freq_handler(int channel_page, int channel)
>  	}
>  }
>  
> +static char cca_mode_buf[100];
> +
> +const char *print_cca_mode_handler(enum nl802154_cca_modes cca_mode,
> +				   enum nl802154_cca_opts cca_opt)
> +{
> +	switch (cca_mode) {
> +	case NL802154_CCA_ENERGY:
> +		return "Energy above threshold";
> +	case NL802154_CCA_CARRIER:
> +		return "Carrier sense only";
> +	case NL802154_CCA_ENERGY_CARRIER:
> +		switch (cca_opt) {
> +		case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
> +			return "Carrier sense with energy above threshold (logical operator is 'and')";
> +		case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
> +			return "Carrier sense with energy above threshold (logical operator is 'or')";
> +		default:
> +			sprintf(cca_mode_buf,
> +				"Unknown CCA option (%d) for CCA mode (%d)",
> +				cca_opt,
> +				cca_mode);
> +			return cca_mode_buf;
> +		}
> +	case NL802154_CCA_ALOHA:
> +		return "ALOHA";
> +	case NL802154_CCA_UWB_SHR:
> +		return "UWB preamble sense based on the SHR of a frame";
> +	case NL802154_CCA_UWB_MULTIPEXED:
> +		return "UWB preamble sense based on the packet with the multiplexed preamble";
> +	default:
> +		sprintf(cca_mode_buf, "Unknown CCA mode (%d)", cca_mode);
> +		return cca_mode_buf;
> +	}
> +}
> +
>  static const char *commands[NL802154_CMD_MAX + 1] = {
>  	[NL802154_CMD_UNSPEC] = "unspec",
>  	[NL802154_CMD_GET_WPAN_PHY] = "get_wpan_phy",
> @@ -235,23 +270,13 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
>  	}
>  
>  	if (tb_msg[NL802154_ATTR_CCA_MODE]) {
> +		enum nl802154_cca_opts cca_opt = 99;

use:

NL802154_CCA_OPT_ATTR_MAX

should always be invalid. Maybe there exists a better way to evaluate
the tb_msg[NL802154_ATTR_CCA_MODE] first when somewhere is a cca_mode
which have some option (like cca_mode == 3). I don't know how to do it
_exactly_ better now. I am fine with your current solution.

Can you try to add first a patch which updates the nl802154.h from the
kernel "just for syncing" and then replace the cca_mode printout. And
please fixup patch 2 with 1.

Then you have:

1. patch which sync the nl802154 header with the kernel
2. patch which replace the cca modes printout with your implementation.


Current behaviour would have some state in wpan-tools which we
havn't any cca output, this is the case when I apply patch 1 only in your
series.

- Alex

Re: [PATCH v3 wpan-tools 1/3] info: add cca mode descriptive text to output

[Christoffer Holmstedt]

On Wed, Jun 10, 2015 at 02:17:13PM +0200, Alexander Aring wrote:
> On Wed, Jun 10, 2015 at 01:18:07PM +0200, Christoffer Holmstedt wrote:
> > Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
> > ---
> >  src/info.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++--------------
> >  1 file changed, 63 insertions(+), 18 deletions(-)
> > 
> > diff --git a/src/info.c b/src/info.c
> > index 2434bef6a87f..364c02f718aa 100644
> > --- a/src/info.c
> > +++ b/src/info.c
> > @@ -146,6 +146,41 @@ static void print_freq_handler(int channel_page, int channel)
> >  	}
> >  }
> >  
> > +static char cca_mode_buf[100];
> > +
> > +const char *print_cca_mode_handler(enum nl802154_cca_modes cca_mode,
> > +				   enum nl802154_cca_opts cca_opt)
> > +{
> > +	switch (cca_mode) {
> > +	case NL802154_CCA_ENERGY:
> > +		return "Energy above threshold";
> > +	case NL802154_CCA_CARRIER:
> > +		return "Carrier sense only";
> > +	case NL802154_CCA_ENERGY_CARRIER:
> > +		switch (cca_opt) {
> > +		case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
> > +			return "Carrier sense with energy above threshold (logical operator is 'and')";
> > +		case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
> > +			return "Carrier sense with energy above threshold (logical operator is 'or')";
> > +		default:
> > +			sprintf(cca_mode_buf,
> > +				"Unknown CCA option (%d) for CCA mode (%d)",
> > +				cca_opt,
> > +				cca_mode);
> > +			return cca_mode_buf;
> > +		}
> > +	case NL802154_CCA_ALOHA:
> > +		return "ALOHA";
> > +	case NL802154_CCA_UWB_SHR:
> > +		return "UWB preamble sense based on the SHR of a frame";
> > +	case NL802154_CCA_UWB_MULTIPEXED:
> > +		return "UWB preamble sense based on the packet with the multiplexed preamble";
> > +	default:
> > +		sprintf(cca_mode_buf, "Unknown CCA mode (%d)", cca_mode);
> > +		return cca_mode_buf;
> > +	}
> > +}
> > +
> >  static const char *commands[NL802154_CMD_MAX + 1] = {
> >  	[NL802154_CMD_UNSPEC] = "unspec",
> >  	[NL802154_CMD_GET_WPAN_PHY] = "get_wpan_phy",
> > @@ -235,23 +270,13 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
> >  	}
> >  
> >  	if (tb_msg[NL802154_ATTR_CCA_MODE]) {
> > +		enum nl802154_cca_opts cca_opt = 99;
> 
> use:
> 
> NL802154_CCA_OPT_ATTR_MAX

Ok.

> 
> should always be invalid. Maybe there exists a better way to evaluate
> the tb_msg[NL802154_ATTR_CCA_MODE] first when somewhere is a cca_mode
> which have some option (like cca_mode == 3). I don't know how to do it
> _exactly_ better now. I am fine with your current solution.
> 
> Can you try to add first a patch which updates the nl802154.h from the
> kernel "just for syncing" and then replace the cca_mode printout. And
> please fixup patch 2 with 1.

Sure.

> 
> Then you have:
> 
> 1. patch which sync the nl802154 header with the kernel
> 2. patch which replace the cca modes printout with your implementation.
> 
> 
> Current behaviour would have some state in wpan-tools which we
> havn't any cca output, this is the case when I apply patch 1 only in your
> series.
> 
> - Alex

Thanks for the feedback.

-- 
Christoffer Holmstedt

[PATCH wpan-tools] nl802154: fix misspelled enum

[Christoffer Holmstedt]

This patch updates nl802154 enum to match the one in bluetooth-next tree from
commit d446278c408b ("nl802154: fix misspelled enum").

Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
---
 src/nl802154.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/nl802154.h b/src/nl802154.h
index 6fc231e60920..b0ab530d28cd 100644
--- a/src/nl802154.h
+++ b/src/nl802154.h
@@ -187,7 +187,7 @@ enum nl802154_wpan_phy_capability_attr {
  * @NL802154_CCA_ENERGY_CARRIER: Carrier sense with energy above threshold
  * @NL802154_CCA_ALOHA: CCA shall always report an idle medium
  * @NL802154_CCA_UWB_SHR: UWB preamble sense based on the SHR of a frame
- * @NL802154_CCA_UWB_MULTIPEXED: UWB preamble sense based on the packet with
+ * @NL802154_CCA_UWB_MULTIPLEXED: UWB preamble sense based on the packet with
  *	the multiplexed preamble
  * @__NL802154_CCA_ATTR_AFTER_LAST: Internal
  * @NL802154_CCA_ATTR_MAX: Maximum CCA attribute number
@@ -199,7 +199,7 @@ enum nl802154_cca_modes {
 	NL802154_CCA_ENERGY_CARRIER,
 	NL802154_CCA_ALOHA,
 	NL802154_CCA_UWB_SHR,
-	NL802154_CCA_UWB_MULTIPEXED,
+	NL802154_CCA_UWB_MULTIPLEXED,
 
 	/* keep last */
 	__NL802154_CCA_ATTR_AFTER_LAST,
-- 
2.1.4

Re: [PATCH wpan-tools] nl802154: fix misspelled enum

[Alexander Aring]

On Wed, Jun 10, 2015 at 02:39:18PM +0200, Christoffer Holmstedt wrote:
> This patch updates nl802154 enum to match the one in bluetooth-next tree from
> commit d446278c408b ("nl802154: fix misspelled enum").
> 
> Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>

Applied, thanks.

- Alex

[PATCH v4 wpan-tools] info: update CCA mode and CCA options output

[Christoffer Holmstedt]

Instead of printing out CCA mode and CCA options in integers descriptive
text is now printed to the user when "iwpan list" is run.

Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
---
I think I got all the changes right now as requested.

Changes v3 -> v4:
* Sent "nl802154: fix misspelled enum" patch separately (already merged).
* Did a "fixup" of remaining two patches so it is only one now.
* Changed "random" value of 99 to NL802154_CCA_OPT_ATTR_MAX.

Changes v2 -> v3:
* Fixed misspelled enum in nl802154

Changes v1 -> v2:
* cca_mode number is removed from "current cca_mode", only text is printed as
an example:

        cca_mode: Energy above threshold
        was
        cca_mode: 1, Energy above threshold

* Changed "enum" to string mapping in print_cca_mode_handler. Instead of
printing out the strings in the function I return a tempbuf that is formatted
by the caller.

* Added default value to cca_opt when printing current cca_mode and cca_opt. It
is set to the "random" value of 99 as the enum only has 2 values, is this a
proper solution? (or should we use limits.h with MAX_INT perhaps?)


 src/info.c | 100 +++++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 61 insertions(+), 39 deletions(-)

diff --git a/src/info.c b/src/info.c
index 2434bef6a87f..9b0780df0262 100644
--- a/src/info.c
+++ b/src/info.c
@@ -146,6 +146,41 @@ static void print_freq_handler(int channel_page, int channel)
 	}
 }
 
+static char cca_mode_buf[100];
+
+const char *print_cca_mode_handler(enum nl802154_cca_modes cca_mode,
+				   enum nl802154_cca_opts cca_opt)
+{
+	switch (cca_mode) {
+	case NL802154_CCA_ENERGY:
+		return "Energy above threshold";
+	case NL802154_CCA_CARRIER:
+		return "Carrier sense only";
+	case NL802154_CCA_ENERGY_CARRIER:
+		switch (cca_opt) {
+		case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
+			return "Carrier sense with energy above threshold (logical operator is 'and')";
+		case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
+			return "Carrier sense with energy above threshold (logical operator is 'or')";
+		default:
+			sprintf(cca_mode_buf,
+				"Unknown CCA option (%d) for CCA mode (%d)",
+				cca_opt,
+				cca_mode);
+			return cca_mode_buf;
+		}
+	case NL802154_CCA_ALOHA:
+		return "ALOHA";
+	case NL802154_CCA_UWB_SHR:
+		return "UWB preamble sense based on the SHR of a frame";
+	case NL802154_CCA_UWB_MULTIPLEXED:
+		return "UWB preamble sense based on the packet with the multiplexed preamble";
+	default:
+		sprintf(cca_mode_buf, "Unknown CCA mode (%d)", cca_mode);
+		return cca_mode_buf;
+	}
+}
+
 static const char *commands[NL802154_CMD_MAX + 1] = {
 	[NL802154_CMD_UNSPEC] = "unspec",
 	[NL802154_CMD_GET_WPAN_PHY] = "get_wpan_phy",
@@ -235,23 +270,13 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
 	}
 
 	if (tb_msg[NL802154_ATTR_CCA_MODE]) {
+		enum nl802154_cca_opts cca_opt = NL802154_CCA_OPT_ATTR_MAX;
+		if (tb_msg[NL802154_ATTR_CCA_OPT])
+			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
+
 		cca_mode = nla_get_u32(tb_msg[NL802154_ATTR_CCA_MODE]);
-		printf("cca_mode: %d", cca_mode);
-		if (cca_mode == NL802154_CCA_ENERGY_CARRIER) {
-			enum nl802154_cca_opts cca_opt;
 
-			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
-			switch (cca_opt) {
-			case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
-				printf(" logical and ");
-				break;
-			case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
-				printf(" logical or ");
-				break;
-			default:
-				printf(" logical op mode unkown ");
-			}
-		}
+		printf("cca_mode: %s", print_cca_mode_handler(cca_mode, cca_opt));
 		printf("\n");
 	}
 
@@ -361,33 +386,30 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
 			printf("\tcca_modes: ");
 			nla_for_each_nested(nl_cca_modes,
 					    tb_caps[NL802154_CAP_ATTR_CCA_MODES],
-					    rem_cca_modes)
-				printf("%d,", nla_type(nl_cca_modes));
-			/* TODO */
-			printf("\b \n");
-		}
-
-		if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
-			struct nlattr *nl_cca_opts;
-			int rem_cca_opts;
-
-			printf("\tcca_opts: ");
-			nla_for_each_nested(nl_cca_opts,
-					    tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
-					    rem_cca_opts) {
-				printf("%d", nla_type(nl_cca_opts));
-				switch (nla_type(nl_cca_opts)) {
-				case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
-				case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
-					printf("(cca_mode: 3),");
-					break;
-				default:
-					printf("unkown\n");
-					break;
+					    rem_cca_modes) {
+				if (nla_type(nl_cca_modes) != NL802154_CCA_ENERGY_CARRIER) {
+					printf("\n\t\t(%d) %s",
+						nla_type(nl_cca_modes),
+						print_cca_mode_handler(nla_type(nl_cca_modes), 0));
+				} else {
+					if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
+						struct nlattr *nl_cca_opts;
+						int rem_cca_opts;
+
+						nla_for_each_nested(nl_cca_opts,
+								    tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
+								    rem_cca_opts) {
+
+							printf("\n\t\t(%d, cca_opt: %d) %s",
+								nla_type(nl_cca_modes),
+								nla_type(nl_cca_opts),
+								print_cca_mode_handler(nla_type(nl_cca_modes), nla_type(nl_cca_opts)));
+						}
+					}
 				}
 			}
 			/* TODO */
-			printf("\b \n");
+			printf("\n");
 		}
 
 		if (tb_caps[NL802154_CAP_ATTR_MIN_MINBE] &&
-- 
2.1.4

Re: [PATCH v4 wpan-tools] info: update CCA mode and CCA options output

[Alexander Aring]

On Wed, Jun 10, 2015 at 03:07:55PM +0200, Christoffer Holmstedt wrote:
> Instead of printing out CCA mode and CCA options in integers descriptive
> text is now printed to the user when "iwpan list" is run.
> 
> Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
> ---
> I think I got all the changes right now as requested.
> 
> Changes v3 -> v4:
> * Sent "nl802154: fix misspelled enum" patch separately (already merged).
> * Did a "fixup" of remaining two patches so it is only one now.
> * Changed "random" value of 99 to NL802154_CCA_OPT_ATTR_MAX.
> 
> Changes v2 -> v3:
> * Fixed misspelled enum in nl802154
> 
> Changes v1 -> v2:
> * cca_mode number is removed from "current cca_mode", only text is printed as
> an example:
> 
>         cca_mode: Energy above threshold
>         was
>         cca_mode: 1, Energy above threshold
> 
> * Changed "enum" to string mapping in print_cca_mode_handler. Instead of
> printing out the strings in the function I return a tempbuf that is formatted
> by the caller.
> 
> * Added default value to cca_opt when printing current cca_mode and cca_opt. It
> is set to the "random" value of 99 as the enum only has 2 values, is this a
> proper solution? (or should we use limits.h with MAX_INT perhaps?)
> 
> 
>  src/info.c | 100 +++++++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 61 insertions(+), 39 deletions(-)
> 
> diff --git a/src/info.c b/src/info.c
> index 2434bef6a87f..9b0780df0262 100644
> --- a/src/info.c
> +++ b/src/info.c
> @@ -146,6 +146,41 @@ static void print_freq_handler(int channel_page, int channel)
>  	}
>  }
>  
> +static char cca_mode_buf[100];
> +
> +const char *print_cca_mode_handler(enum nl802154_cca_modes cca_mode,
> +				   enum nl802154_cca_opts cca_opt)
> +{
> +	switch (cca_mode) {
> +	case NL802154_CCA_ENERGY:
> +		return "Energy above threshold";
> +	case NL802154_CCA_CARRIER:
> +		return "Carrier sense only";
> +	case NL802154_CCA_ENERGY_CARRIER:
> +		switch (cca_opt) {
> +		case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
> +			return "Carrier sense with energy above threshold (logical operator is 'and')";
> +		case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
> +			return "Carrier sense with energy above threshold (logical operator is 'or')";
> +		default:
> +			sprintf(cca_mode_buf,
> +				"Unknown CCA option (%d) for CCA mode (%d)",
> +				cca_opt,
> +				cca_mode);
> +			return cca_mode_buf;
> +		}
> +	case NL802154_CCA_ALOHA:
> +		return "ALOHA";
> +	case NL802154_CCA_UWB_SHR:
> +		return "UWB preamble sense based on the SHR of a frame";
> +	case NL802154_CCA_UWB_MULTIPLEXED:
> +		return "UWB preamble sense based on the packet with the multiplexed preamble";
> +	default:
> +		sprintf(cca_mode_buf, "Unknown CCA mode (%d)", cca_mode);
> +		return cca_mode_buf;
> +	}
> +}
> +

sorry, I see this now. Why not simple making this return values void of
this function and doing instead of:

return "Carrier sense with energy above threshold (logical operator is
'or')";

a:

sprintf(cca_mode_buf, "%s", "Carrier sense with energy above threshold
(logical operator is 'or')");



and always use the cca_mode_buf afterwards for printing.
Is this possible?


Then you can directly use the parameters for baking the cca_mode string,
like:
sprintf(cca_mode, "mode %d, %s", cca_mode, "cca mode description");

After calling that function you can call:
printf("additional stuff %s\n", cca_mode_buf);


The function print_cca_mode_handler should be a helper function only to
place the cca description inside the cca_mode_buf and it can be used
until the next call of print_cca_mode_handler by printf or something
else.

- Alex

Re: [PATCH v4 wpan-tools] info: update CCA mode and CCA options output

[Christoffer Holmstedt]

On Wed, Jun 10, 2015 at 03:45:31PM +0200, Alexander Aring wrote:
> On Wed, Jun 10, 2015 at 03:07:55PM +0200, Christoffer Holmstedt wrote:
> > Instead of printing out CCA mode and CCA options in integers descriptive
> > text is now printed to the user when "iwpan list" is run.
> > 
> > Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
> > ---
> > I think I got all the changes right now as requested.
> > 
> > Changes v3 -> v4:
> > * Sent "nl802154: fix misspelled enum" patch separately (already merged).
> > * Did a "fixup" of remaining two patches so it is only one now.
> > * Changed "random" value of 99 to NL802154_CCA_OPT_ATTR_MAX.
...cut...
> > +	case NL802154_CCA_UWB_SHR:
> > +		return "UWB preamble sense based on the SHR of a frame";
> > +	case NL802154_CCA_UWB_MULTIPLEXED:
> > +		return "UWB preamble sense based on the packet with the multiplexed preamble";
> > +	default:
> > +		sprintf(cca_mode_buf, "Unknown CCA mode (%d)", cca_mode);
> > +		return cca_mode_buf;
> > +	}
> > +}
> > +
> 
> sorry, I see this now. Why not simple making this return values void of
> this function and doing instead of:
> 

No need to say sorry, I appreciate all the feedback I can get. I hope I can
return the favor in the future when I know the code base a little bit more.

> return "Carrier sense with energy above threshold (logical operator is
> 'or')";
> 
> a:
> 
> sprintf(cca_mode_buf, "%s", "Carrier sense with energy above threshold
> (logical operator is 'or')");
> and always use the cca_mode_buf afterwards for printing.
> Is this possible?
> 
Yes it is possible, I just prepared a patch for the change (haven't sent it to
the list yet), will send it later today. One question first, should the
declaration of the "cca_mode_buf" (file scope) be put at the top of the file or
just above the helper function using it?

> 
> Then you can directly use the parameters for baking the cca_mode string,
> like:
> sprintf(cca_mode, "mode %d, %s", cca_mode, "cca mode description");
> 
> After calling that function you can call:
> printf("additional stuff %s\n", cca_mode_buf);
> 
> 
> The function print_cca_mode_handler should be a helper function only to
> place the cca description inside the cca_mode_buf and it can be used
> until the next call of print_cca_mode_handler by printf or something
> else.
> 
> - Alex

This new solution makes much more sense to me. The first implementation was
inspired by *iftype_name() from interface.c, perhaps that should be refactored
as well.

Regards
-- 
Christoffer Holmstedt

Re: [PATCH v4 wpan-tools] info: update CCA mode and CCA options output

[Alexander Aring]

On Thu, Jun 11, 2015 at 08:16:33AM +0200, Christoffer Holmstedt wrote:
> On Wed, Jun 10, 2015 at 03:45:31PM +0200, Alexander Aring wrote:
> > On Wed, Jun 10, 2015 at 03:07:55PM +0200, Christoffer Holmstedt wrote:
> > > Instead of printing out CCA mode and CCA options in integers descriptive
> > > text is now printed to the user when "iwpan list" is run.
> > > 
> > > Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
> > > ---
> > > I think I got all the changes right now as requested.
> > > 
> > > Changes v3 -> v4:
> > > * Sent "nl802154: fix misspelled enum" patch separately (already merged).
> > > * Did a "fixup" of remaining two patches so it is only one now.
> > > * Changed "random" value of 99 to NL802154_CCA_OPT_ATTR_MAX.
> ...cut...
> > > +	case NL802154_CCA_UWB_SHR:
> > > +		return "UWB preamble sense based on the SHR of a frame";
> > > +	case NL802154_CCA_UWB_MULTIPLEXED:
> > > +		return "UWB preamble sense based on the packet with the multiplexed preamble";
> > > +	default:
> > > +		sprintf(cca_mode_buf, "Unknown CCA mode (%d)", cca_mode);
> > > +		return cca_mode_buf;
> > > +	}
> > > +}
> > > +
> > 
> > sorry, I see this now. Why not simple making this return values void of
> > this function and doing instead of:
> > 
> 
> No need to say sorry, I appreciate all the feedback I can get. I hope I can
> return the favor in the future when I know the code base a little bit more.
> 
> > return "Carrier sense with energy above threshold (logical operator is
> > 'or')";
> > 
> > a:
> > 
> > sprintf(cca_mode_buf, "%s", "Carrier sense with energy above threshold
> > (logical operator is 'or')");
> > and always use the cca_mode_buf afterwards for printing.
> > Is this possible?
> > 
> Yes it is possible, I just prepared a patch for the change (haven't sent it to
> the list yet), will send it later today. One question first, should the
> declaration of the "cca_mode_buf" (file scope) be put at the top of the file or
> just above the helper function using it?
> 
> > 
> > Then you can directly use the parameters for baking the cca_mode string,
> > like:
> > sprintf(cca_mode, "mode %d, %s", cca_mode, "cca mode description");
> > 
> > After calling that function you can call:
> > printf("additional stuff %s\n", cca_mode_buf);
> > 
> > 
> > The function print_cca_mode_handler should be a helper function only to
> > place the cca description inside the cca_mode_buf and it can be used
> > until the next call of print_cca_mode_handler by printf or something
> > else.
> > 
> > - Alex
> 
> This new solution makes much more sense to me. The first implementation was
> inspired by *iftype_name() from interface.c, perhaps that should be refactored
> as well.
> 

then better maybe would be to leave the return value as "const char *"
there and returning the buffer. Just if you need to "bake" some string
with an format string, then use the temp buffer and build your string and
return the buffer.

If it's just a "const char *" then just 'return "foobar";'  but this
isn't your case here because you want to put some numbers in there, for
cca handling.

It's just:
The return of the temporary buffer allows us to use the helper printout
functionality also from another c-file when the buffer is declared as
static buffer.

- Alex

Re: [PATCH v4 wpan-tools] info: update CCA mode and CCA options output

[Christoffer Holmstedt]

On Thu, Jun 11, 2015 at 09:42:10AM +0200, Alexander Aring wrote:
> On Thu, Jun 11, 2015 at 08:16:33AM +0200, Christoffer Holmstedt wrote:
> > On Wed, Jun 10, 2015 at 03:45:31PM +0200, Alexander Aring wrote:
...cut...
> > > 
> > > The function print_cca_mode_handler should be a helper function only to
> > > place the cca description inside the cca_mode_buf and it can be used
> > > until the next call of print_cca_mode_handler by printf or something
> > > else.
> > > 
> > > - Alex
> > 
> > This new solution makes much more sense to me. The first implementation was
> > inspired by *iftype_name() from interface.c, perhaps that should be refactored
> > as well.
> > 
> 
> then better maybe would be to leave the return value as "const char *"
> there and returning the buffer. Just if you need to "bake" some string
> with an format string, then use the temp buffer and build your string and
> return the buffer.
> 
> If it's just a "const char *" then just 'return "foobar";'  but this
> isn't your case here because you want to put some numbers in there, for
> cca handling.
> 
> It's just:
> The return of the temporary buffer allows us to use the helper printout
> functionality also from another c-file when the buffer is declared as
> static buffer.
> 
> - Alex

aah, yes accessing the value from other files.

Not sure about the first two paragraphs though...a patch is coming up (separate
email) for info.c and if needed interface.c can be discussed yet in another
thread.

-- 
Christoffer Holmstedt

[PATCH v5 wpan-tools] info: update CCA mode and CCA options output

[Christoffer Holmstedt]

Instead of printing out CCA mode and CCA options in integers descriptive
text is now printed to the user when "iwpan list" is run.

Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
---

Changes v4 -> v5:
* Changed print_cca_mode_handler to only alter cca_mode_buf instead of
returning strings.

Changes v3 -> v4:
* Sent "nl802154: fix misspelled enum" patch separately (already merged).
* Did a "fixup" of remaining two patches so it is only one now.
* Changed "random" value of 99 to NL802154_CCA_OPT_ATTR_MAX.

Changes v2 -> v3:
* Fixed misspelled enum in nl802154

Changes v1 -> v2:
* cca_mode number is removed from "current cca_mode", only text is printed as
an example:

        cca_mode: Energy above threshold
        was
        cca_mode: 1, Energy above threshold

* Changed "enum" to string mapping in print_cca_mode_handler. Instead of
printing out the strings in the function I return a tempbuf that is formatted
by the caller.

* Added default value to cca_opt when printing current cca_mode and cca_opt. It
is set to the "random" value of 99 as the enum only has 2 values, is this a
proper solution? (or should we use limits.h with MAX_INT perhaps?)


 src/info.c | 112 ++++++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 73 insertions(+), 39 deletions(-)

diff --git a/src/info.c b/src/info.c
index 2434bef6a87f..b6a452fb9ccd 100644
--- a/src/info.c
+++ b/src/info.c
@@ -146,6 +146,49 @@ static void print_freq_handler(int channel_page, int channel)
 	}
 }
 
+static char cca_mode_buf[100];
+
+void print_cca_mode_handler(enum nl802154_cca_modes cca_mode,
+			    enum nl802154_cca_opts cca_opt)
+{
+	switch (cca_mode) {
+	case NL802154_CCA_ENERGY:
+		sprintf(cca_mode_buf,"%s", "Energy above threshold");
+		break;
+	case NL802154_CCA_CARRIER:
+		sprintf(cca_mode_buf,"%s", "Carrier sense only");
+		break;
+	case NL802154_CCA_ENERGY_CARRIER:
+		switch (cca_opt) {
+		case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
+			sprintf(cca_mode_buf,"%s", "Carrier sense with energy above threshold (logical operator is 'and')");
+			break;
+		case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
+			sprintf(cca_mode_buf,"%s", "Carrier sense with energy above threshold (logical operator is 'or')");
+			break;
+		default:
+			sprintf(cca_mode_buf,
+				"Unknown CCA option (%d) for CCA mode (%d)",
+				cca_opt,
+				cca_mode);
+			break;
+		}
+		break;
+	case NL802154_CCA_ALOHA:
+		sprintf(cca_mode_buf,"%s", "ALOHA");
+		break;
+	case NL802154_CCA_UWB_SHR:
+		sprintf(cca_mode_buf,"%s", "UWB preamble sense based on the SHR of a frame");
+		break;
+	case NL802154_CCA_UWB_MULTIPLEXED:
+		sprintf(cca_mode_buf,"%s", "UWB preamble sense based on the packet with the multiplexed preamble");
+		break;
+	default:
+		sprintf(cca_mode_buf, "Unknown CCA mode (%d)", cca_mode);
+		break;
+	}
+}
+
 static const char *commands[NL802154_CMD_MAX + 1] = {
 	[NL802154_CMD_UNSPEC] = "unspec",
 	[NL802154_CMD_GET_WPAN_PHY] = "get_wpan_phy",
@@ -235,23 +278,14 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
 	}
 
 	if (tb_msg[NL802154_ATTR_CCA_MODE]) {
+		enum nl802154_cca_opts cca_opt = NL802154_CCA_OPT_ATTR_MAX;
+		if (tb_msg[NL802154_ATTR_CCA_OPT])
+			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
+
 		cca_mode = nla_get_u32(tb_msg[NL802154_ATTR_CCA_MODE]);
-		printf("cca_mode: %d", cca_mode);
-		if (cca_mode == NL802154_CCA_ENERGY_CARRIER) {
-			enum nl802154_cca_opts cca_opt;
 
-			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
-			switch (cca_opt) {
-			case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
-				printf(" logical and ");
-				break;
-			case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
-				printf(" logical or ");
-				break;
-			default:
-				printf(" logical op mode unkown ");
-			}
-		}
+		print_cca_mode_handler(cca_mode, cca_opt);
+		printf("cca_mode: %s", cca_mode_buf);
 		printf("\n");
 	}
 
@@ -361,33 +395,33 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
 			printf("\tcca_modes: ");
 			nla_for_each_nested(nl_cca_modes,
 					    tb_caps[NL802154_CAP_ATTR_CCA_MODES],
-					    rem_cca_modes)
-				printf("%d,", nla_type(nl_cca_modes));
-			/* TODO */
-			printf("\b \n");
-		}
-
-		if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
-			struct nlattr *nl_cca_opts;
-			int rem_cca_opts;
-
-			printf("\tcca_opts: ");
-			nla_for_each_nested(nl_cca_opts,
-					    tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
-					    rem_cca_opts) {
-				printf("%d", nla_type(nl_cca_opts));
-				switch (nla_type(nl_cca_opts)) {
-				case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
-				case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
-					printf("(cca_mode: 3),");
-					break;
-				default:
-					printf("unkown\n");
-					break;
+					    rem_cca_modes) {
+				if (nla_type(nl_cca_modes) != NL802154_CCA_ENERGY_CARRIER) {
+					print_cca_mode_handler(nla_type(nl_cca_modes), 0);
+					printf("\n\t\t(%d) %s",
+							nla_type(nl_cca_modes),
+							cca_mode_buf);
+				} else {
+					if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
+						struct nlattr *nl_cca_opts;
+						int rem_cca_opts;
+
+						nla_for_each_nested(nl_cca_opts,
+								    tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
+								    rem_cca_opts) {
+							print_cca_mode_handler(
+									nla_type(nl_cca_modes),
+									nla_type(nl_cca_opts));
+							printf("\n\t\t(%d, cca_opt: %d) %s",
+								nla_type(nl_cca_modes),
+								nla_type(nl_cca_opts),
+								cca_mode_buf);
+						}
+					}
 				}
 			}
 			/* TODO */
-			printf("\b \n");
+			printf("\n");
 		}
 
 		if (tb_caps[NL802154_CAP_ATTR_MIN_MINBE] &&
-- 
2.1.4

Re: [PATCH v4 wpan-tools] info: update CCA mode and CCA options output

[Alexander Aring]

On Wed, Jun 10, 2015 at 03:07:55PM +0200, Christoffer Holmstedt wrote:
> Instead of printing out CCA mode and CCA options in integers descriptive
> text is now printed to the user when "iwpan list" is run.
> 
> Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>

I would apply this patch now. This is the nearest solution which I would
ack. Reason see below. Tell me if you are fine with that.


> ---
> I think I got all the changes right now as requested.
> 
> Changes v3 -> v4:
> * Sent "nl802154: fix misspelled enum" patch separately (already merged).
> * Did a "fixup" of remaining two patches so it is only one now.
> * Changed "random" value of 99 to NL802154_CCA_OPT_ATTR_MAX.
> 
> Changes v2 -> v3:
> * Fixed misspelled enum in nl802154
> 
> Changes v1 -> v2:
> * cca_mode number is removed from "current cca_mode", only text is printed as
> an example:
> 
>         cca_mode: Energy above threshold
>         was
>         cca_mode: 1, Energy above threshold
> 

The reason why we have the helper function is too have the look and feel
everywhere the same for the one enum definition.

> * Changed "enum" to string mapping in print_cca_mode_handler. Instead of
> printing out the strings in the function I return a tempbuf that is formatted
> by the caller.
> 
> * Added default value to cca_opt when printing current cca_mode and cca_opt. It
> is set to the "random" value of 99 as the enum only has 2 values, is this a
> proper solution? (or should we use limits.h with MAX_INT perhaps?)
> 
> 
>  src/info.c | 100 +++++++++++++++++++++++++++++++++++++------------------------
>  1 file changed, 61 insertions(+), 39 deletions(-)
> 
> diff --git a/src/info.c b/src/info.c
> index 2434bef6a87f..9b0780df0262 100644
> --- a/src/info.c
> +++ b/src/info.c
> @@ -146,6 +146,41 @@ static void print_freq_handler(int channel_page, int channel)
>  	}
>  }
>  
> +static char cca_mode_buf[100];
> +
> +const char *print_cca_mode_handler(enum nl802154_cca_modes cca_mode,
> +				   enum nl802154_cca_opts cca_opt)
> +{
> +	switch (cca_mode) {
> +	case NL802154_CCA_ENERGY:
> +		return "Energy above threshold";

Don't add the enum numbers afterwards, do that here with sprintf and
return the cca_mode_buf. Like you did that for the default case of
cca_opt.

Example:

You do afterwards:
printf("\n\t\t(%d) %s"....);

then simple do a:

sprintf(cca_mode_buf, "(%d) %s", cca_mode, "Energy above threshold");
return cca_mode_buf;

If you _really_ like to remove the cca_mode _number_ (in case of current
cca), then make some bool argument with_numbers and change the format string
here.

> +	case NL802154_CCA_CARRIER:
> +		return "Carrier sense only";
> +	case NL802154_CCA_ENERGY_CARRIER:
> +		switch (cca_opt) {
> +		case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
> +			return "Carrier sense with energy above threshold (logical operator is 'and')";
> +		case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
> +			return "Carrier sense with energy above threshold (logical operator is 'or')";
> +		default:
> +			sprintf(cca_mode_buf,
> +				"Unknown CCA option (%d) for CCA mode (%d)",
> +				cca_opt,
> +				cca_mode);
> +			return cca_mode_buf;
> +		}
> +	case NL802154_CCA_ALOHA:
> +		return "ALOHA";
> +	case NL802154_CCA_UWB_SHR:
> +		return "UWB preamble sense based on the SHR of a frame";
> +	case NL802154_CCA_UWB_MULTIPLEXED:
> +		return "UWB preamble sense based on the packet with the multiplexed preamble";
> +	default:
> +		sprintf(cca_mode_buf, "Unknown CCA mode (%d)", cca_mode);
> +		return cca_mode_buf;
> +	}
> +}
> +
>  static const char *commands[NL802154_CMD_MAX + 1] = {
>  	[NL802154_CMD_UNSPEC] = "unspec",
>  	[NL802154_CMD_GET_WPAN_PHY] = "get_wpan_phy",
> @@ -235,23 +270,13 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
>  	}
>  
>  	if (tb_msg[NL802154_ATTR_CCA_MODE]) {
> +		enum nl802154_cca_opts cca_opt = NL802154_CCA_OPT_ATTR_MAX;
> +		if (tb_msg[NL802154_ATTR_CCA_OPT])
> +			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
> +
>  		cca_mode = nla_get_u32(tb_msg[NL802154_ATTR_CCA_MODE]);
> -		printf("cca_mode: %d", cca_mode);
> -		if (cca_mode == NL802154_CCA_ENERGY_CARRIER) {
> -			enum nl802154_cca_opts cca_opt;
>  
> -			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
> -			switch (cca_opt) {
> -			case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
> -				printf(" logical and ");
> -				break;
> -			case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
> -				printf(" logical or ");
> -				break;
> -			default:
> -				printf(" logical op mode unkown ");
> -			}
> -		}
> +		printf("cca_mode: %s", print_cca_mode_handler(cca_mode, cca_opt));
>  		printf("\n");
>  	}
>  
> @@ -361,33 +386,30 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
>  			printf("\tcca_modes: ");
>  			nla_for_each_nested(nl_cca_modes,
>  					    tb_caps[NL802154_CAP_ATTR_CCA_MODES],
> -					    rem_cca_modes)
> -				printf("%d,", nla_type(nl_cca_modes));
> -			/* TODO */
> -			printf("\b \n");
> -		}
> -
> -		if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
> -			struct nlattr *nl_cca_opts;
> -			int rem_cca_opts;
> -
> -			printf("\tcca_opts: ");
> -			nla_for_each_nested(nl_cca_opts,
> -					    tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
> -					    rem_cca_opts) {
> -				printf("%d", nla_type(nl_cca_opts));
> -				switch (nla_type(nl_cca_opts)) {
> -				case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
> -				case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
> -					printf("(cca_mode: 3),");
> -					break;
> -				default:
> -					printf("unkown\n");
> -					break;
> +					    rem_cca_modes) {
> +				if (nla_type(nl_cca_modes) != NL802154_CCA_ENERGY_CARRIER) {
> +					printf("\n\t\t(%d) %s",
> +						nla_type(nl_cca_modes),
> +						print_cca_mode_handler(nla_type(nl_cca_modes), 0));
> +				} else {
> +					if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
> +						struct nlattr *nl_cca_opts;
> +						int rem_cca_opts;
> +
> +						nla_for_each_nested(nl_cca_opts,
> +								    tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
> +								    rem_cca_opts) {
> +
> +							printf("\n\t\t(%d, cca_opt: %d) %s",
> +								nla_type(nl_cca_modes),
> +								nla_type(nl_cca_opts),
> +								print_cca_mode_handler(nla_type(nl_cca_modes), nla_type(nl_cca_opts)));

Replace this one then with:

printf("\n\t\t%s", print_cca_mode_handler(nla_type(nl_cca_modes), nla_type(nl_cca_opts)));

no special handling is needed here anymore for NL802154_CCA_ENERGY_CARRIER.

- Alex

Re: [PATCH v4 wpan-tools] info: update CCA mode and CCA options output

[Christoffer Holmstedt]

On Thu, Jun 11, 2015 at 12:14:20PM +0200, Alexander Aring wrote:
> On Wed, Jun 10, 2015 at 03:07:55PM +0200, Christoffer Holmstedt wrote:
> > Instead of printing out CCA mode and CCA options in integers descriptive
> > text is now printed to the user when "iwpan list" is run.
> > 
> > Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
> 
> I would apply this patch now. This is the nearest solution which I would
> ack. Reason see below. Tell me if you are fine with that.
> 

So v4 rather than v5?

(I will prepare version 6).

> 
> > ---
> > I think I got all the changes right now as requested.
> > 
> > Changes v3 -> v4:
> > * Sent "nl802154: fix misspelled enum" patch separately (already merged).
> > * Did a "fixup" of remaining two patches so it is only one now.
> > * Changed "random" value of 99 to NL802154_CCA_OPT_ATTR_MAX.
> > 
> > Changes v2 -> v3:
> > * Fixed misspelled enum in nl802154
> > 
> > Changes v1 -> v2:
> > * cca_mode number is removed from "current cca_mode", only text is printed as
> > an example:
> > 
> >         cca_mode: Energy above threshold
> >         was
> >         cca_mode: 1, Energy above threshold
> > 
> 
> The reason why we have the helper function is too have the look and feel
> everywhere the same for the one enum definition.
> 

I agree.

> > * Changed "enum" to string mapping in print_cca_mode_handler. Instead of
> > printing out the strings in the function I return a tempbuf that is formatted
> > by the caller.
> > 
> > * Added default value to cca_opt when printing current cca_mode and cca_opt. It
> > is set to the "random" value of 99 as the enum only has 2 values, is this a
> > proper solution? (or should we use limits.h with MAX_INT perhaps?)
> > 
> > 
> >  src/info.c | 100 +++++++++++++++++++++++++++++++++++++------------------------
> >  1 file changed, 61 insertions(+), 39 deletions(-)
> > 
> > diff --git a/src/info.c b/src/info.c
> > index 2434bef6a87f..9b0780df0262 100644
> > --- a/src/info.c
> > +++ b/src/info.c
> > @@ -146,6 +146,41 @@ static void print_freq_handler(int channel_page, int channel)
> >  	}
> >  }
> >  
> > +static char cca_mode_buf[100];
> > +
> > +const char *print_cca_mode_handler(enum nl802154_cca_modes cca_mode,
> > +				   enum nl802154_cca_opts cca_opt)
> > +{
> > +	switch (cca_mode) {
> > +	case NL802154_CCA_ENERGY:
> > +		return "Energy above threshold";
> 
> Don't add the enum numbers afterwards, do that here with sprintf and
> return the cca_mode_buf. Like you did that for the default case of
> cca_opt.
> 
> Example:
> 
> You do afterwards:
> printf("\n\t\t(%d) %s"....);
> 
> then simple do a:
> 
> sprintf(cca_mode_buf, "(%d) %s", cca_mode, "Energy above threshold");
> return cca_mode_buf;
> 
> If you _really_ like to remove the cca_mode _number_ (in case of current
> cca), then make some bool argument with_numbers and change the format string
> here.
> 

Yea, that is a much better solution. I will update with another patch but will
leave the optional exclusion of cca_mode integer for the future.

> > +	case NL802154_CCA_CARRIER:
> > +		return "Carrier sense only";
> > +	case NL802154_CCA_ENERGY_CARRIER:
> > +		switch (cca_opt) {
> > +		case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
> > +			return "Carrier sense with energy above threshold (logical operator is 'and')";
> > +		case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
> > +			return "Carrier sense with energy above threshold (logical operator is 'or')";
> > +		default:
> > +			sprintf(cca_mode_buf,
> > +				"Unknown CCA option (%d) for CCA mode (%d)",
> > +				cca_opt,
> > +				cca_mode);
> > +			return cca_mode_buf;
> > +		}
> > +	case NL802154_CCA_ALOHA:
> > +		return "ALOHA";
> > +	case NL802154_CCA_UWB_SHR:
> > +		return "UWB preamble sense based on the SHR of a frame";
> > +	case NL802154_CCA_UWB_MULTIPLEXED:
> > +		return "UWB preamble sense based on the packet with the multiplexed preamble";
> > +	default:
> > +		sprintf(cca_mode_buf, "Unknown CCA mode (%d)", cca_mode);
> > +		return cca_mode_buf;
> > +	}
> > +}
> > +
> >  static const char *commands[NL802154_CMD_MAX + 1] = {
> >  	[NL802154_CMD_UNSPEC] = "unspec",
> >  	[NL802154_CMD_GET_WPAN_PHY] = "get_wpan_phy",
> > @@ -235,23 +270,13 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
> >  	}
> >  
> >  	if (tb_msg[NL802154_ATTR_CCA_MODE]) {
> > +		enum nl802154_cca_opts cca_opt = NL802154_CCA_OPT_ATTR_MAX;
> > +		if (tb_msg[NL802154_ATTR_CCA_OPT])
> > +			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
> > +
> >  		cca_mode = nla_get_u32(tb_msg[NL802154_ATTR_CCA_MODE]);
> > -		printf("cca_mode: %d", cca_mode);
> > -		if (cca_mode == NL802154_CCA_ENERGY_CARRIER) {
> > -			enum nl802154_cca_opts cca_opt;
> >  
> > -			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
> > -			switch (cca_opt) {
> > -			case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
> > -				printf(" logical and ");
> > -				break;
> > -			case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
> > -				printf(" logical or ");
> > -				break;
> > -			default:
> > -				printf(" logical op mode unkown ");
> > -			}
> > -		}
> > +		printf("cca_mode: %s", print_cca_mode_handler(cca_mode, cca_opt));
> >  		printf("\n");
> >  	}
> >  
> > @@ -361,33 +386,30 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
> >  			printf("\tcca_modes: ");
> >  			nla_for_each_nested(nl_cca_modes,
> >  					    tb_caps[NL802154_CAP_ATTR_CCA_MODES],
> > -					    rem_cca_modes)
> > -				printf("%d,", nla_type(nl_cca_modes));
> > -			/* TODO */
> > -			printf("\b \n");
> > -		}
> > -
> > -		if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
> > -			struct nlattr *nl_cca_opts;
> > -			int rem_cca_opts;
> > -
> > -			printf("\tcca_opts: ");
> > -			nla_for_each_nested(nl_cca_opts,
> > -					    tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
> > -					    rem_cca_opts) {
> > -				printf("%d", nla_type(nl_cca_opts));
> > -				switch (nla_type(nl_cca_opts)) {
> > -				case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
> > -				case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
> > -					printf("(cca_mode: 3),");
> > -					break;
> > -				default:
> > -					printf("unkown\n");
> > -					break;
> > +					    rem_cca_modes) {
> > +				if (nla_type(nl_cca_modes) != NL802154_CCA_ENERGY_CARRIER) {
> > +					printf("\n\t\t(%d) %s",
> > +						nla_type(nl_cca_modes),
> > +						print_cca_mode_handler(nla_type(nl_cca_modes), 0));
> > +				} else {
> > +					if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
> > +						struct nlattr *nl_cca_opts;
> > +						int rem_cca_opts;
> > +
> > +						nla_for_each_nested(nl_cca_opts,
> > +								    tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
> > +								    rem_cca_opts) {
> > +
> > +							printf("\n\t\t(%d, cca_opt: %d) %s",
> > +								nla_type(nl_cca_modes),
> > +								nla_type(nl_cca_opts),
> > +								print_cca_mode_handler(nla_type(nl_cca_modes), nla_type(nl_cca_opts)));
> 
> Replace this one then with:
> 
> printf("\n\t\t%s", print_cca_mode_handler(nla_type(nl_cca_modes), nla_type(nl_cca_opts)));
> 
> no special handling is needed here anymore for NL802154_CCA_ENERGY_CARRIER.
> 
> - Alex

True. Will do that.

-- 
Christoffer Holmstedt

[PATCH v6 wpan-tools] info: update CCA mode and CCA options output

[Christoffer Holmstedt]

Instead of printing out CCA mode and CCA options in integers descriptive
text is now printed to the user when "iwpan list" is run.

Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>
---

Changes v5 -> v6:
* Reverted v2 changes. The CCA mode and CCA option value as integer is now
included in the descriptive text as well.
* Reverted v5 changes. The print_cca_mode_handler now returns the buffered
value as well.

Changes v4 -> v5:
* Changed print_cca_mode_handler to only alter cca_mode_buf instead of
returning strings.

Changes v3 -> v4:
* Sent "nl802154: fix misspelled enum" patch separately (already merged).
* Did a "fixup" of remaining two patches so it is only one now.
* Changed "random" value of 99 to NL802154_CCA_OPT_ATTR_MAX.

Changes v2 -> v3:
* Fixed misspelled enum in nl802154

Changes v1 -> v2:
* cca_mode number is removed from "current cca_mode", only text is printed as
an example:

        cca_mode: Energy above threshold
        was
        cca_mode: 1, Energy above threshold

* Changed "enum" to string mapping in print_cca_mode_handler. Instead of
printing out the strings in the function I return a tempbuf that is formatted
by the caller.

* Added default value to cca_opt when printing current cca_mode and cca_opt. It
is set to the "random" value of 99 as the enum only has 2 values, is this a
proper solution? (or should we use limits.h with MAX_INT perhaps?)


 src/info.c | 113 ++++++++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 75 insertions(+), 38 deletions(-)

diff --git a/src/info.c b/src/info.c
index 2434bef6a87f..fd75fa092ce7 100644
--- a/src/info.c
+++ b/src/info.c
@@ -146,6 +146,52 @@ static void print_freq_handler(int channel_page, int channel)
 	}
 }
 
+static char cca_mode_buf[100];
+
+const char *print_cca_mode_handler(enum nl802154_cca_modes cca_mode,
+				   enum nl802154_cca_opts cca_opt)
+{
+	switch (cca_mode) {
+	case NL802154_CCA_ENERGY:
+		sprintf(cca_mode_buf,"(%d) %s", cca_mode, "Energy above threshold");
+		break;
+	case NL802154_CCA_CARRIER:
+		sprintf(cca_mode_buf,"(%d) %s", cca_mode, "Carrier sense only");
+		break;
+	case NL802154_CCA_ENERGY_CARRIER:
+		switch (cca_opt) {
+		case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
+			sprintf(cca_mode_buf, "(%d, cca_opt: %d) %s", cca_mode, cca_opt,
+				"Carrier sense with energy above threshold (logical operator is 'and')");
+			break;
+		case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
+			sprintf(cca_mode_buf, "(%d, cca_opt: %d) %s", cca_mode, cca_opt,
+				"Carrier sense with energy above threshold (logical operator is 'or')");
+			break;
+		default:
+			sprintf(cca_mode_buf, "Unknown CCA option (%d) for CCA mode (%d)",
+				cca_opt, cca_mode);
+			break;
+		}
+		break;
+	case NL802154_CCA_ALOHA:
+		sprintf(cca_mode_buf,"(%d) %s", cca_mode, "ALOHA");
+		break;
+	case NL802154_CCA_UWB_SHR:
+		sprintf(cca_mode_buf,"(%d) %s", cca_mode,
+			"UWB preamble sense based on the SHR of a frame");
+		break;
+	case NL802154_CCA_UWB_MULTIPLEXED:
+		sprintf(cca_mode_buf,"(%d) %s", cca_mode,
+			"UWB preamble sense based on the packet with the multiplexed preamble");
+		break;
+	default:
+		sprintf(cca_mode_buf, "Unknown CCA mode (%d)", cca_mode);
+		break;
+	}
+	return cca_mode_buf;
+}
+
 static const char *commands[NL802154_CMD_MAX + 1] = {
 	[NL802154_CMD_UNSPEC] = "unspec",
 	[NL802154_CMD_GET_WPAN_PHY] = "get_wpan_phy",
@@ -235,23 +281,13 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
 	}
 
 	if (tb_msg[NL802154_ATTR_CCA_MODE]) {
+		enum nl802154_cca_opts cca_opt = NL802154_CCA_OPT_ATTR_MAX;
+		if (tb_msg[NL802154_ATTR_CCA_OPT])
+			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
+
 		cca_mode = nla_get_u32(tb_msg[NL802154_ATTR_CCA_MODE]);
-		printf("cca_mode: %d", cca_mode);
-		if (cca_mode == NL802154_CCA_ENERGY_CARRIER) {
-			enum nl802154_cca_opts cca_opt;
 
-			cca_opt = nla_get_u32(tb_msg[NL802154_ATTR_CCA_OPT]);
-			switch (cca_opt) {
-			case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
-				printf(" logical and ");
-				break;
-			case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
-				printf(" logical or ");
-				break;
-			default:
-				printf(" logical op mode unkown ");
-			}
-		}
+		printf("cca_mode: %s", print_cca_mode_handler(cca_mode, cca_opt));
 		printf("\n");
 	}
 
@@ -361,33 +397,34 @@ static int print_phy_handler(struct nl_msg *msg, void *arg)
 			printf("\tcca_modes: ");
 			nla_for_each_nested(nl_cca_modes,
 					    tb_caps[NL802154_CAP_ATTR_CCA_MODES],
-					    rem_cca_modes)
-				printf("%d,", nla_type(nl_cca_modes));
-			/* TODO */
-			printf("\b \n");
-		}
+					    rem_cca_modes) {
+				/* Loop through all CCA options only if it is a
+				 * CCA mode that takes CCA options into
+				 * consideration.
+				 */
+				if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS] &&
+				    nla_type(nl_cca_modes) == NL802154_CCA_ENERGY_CARRIER) {
+					struct nlattr *nl_cca_opts;
+					int rem_cca_opts;
+
+					nla_for_each_nested(nl_cca_opts,
+								tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
+								rem_cca_opts) {
+						printf("\n\t\t%s",
+							print_cca_mode_handler(
+								nla_type(nl_cca_modes),
+								nla_type(nl_cca_opts)));
+					}
+				} else {
+					printf("\n\t\t%s",
+						print_cca_mode_handler(
+							nla_type(nl_cca_modes),
+							NL802154_CCA_OPT_ATTR_MAX));
 
-		if (tb_caps[NL802154_CAP_ATTR_CCA_OPTS]) {
-			struct nlattr *nl_cca_opts;
-			int rem_cca_opts;
-
-			printf("\tcca_opts: ");
-			nla_for_each_nested(nl_cca_opts,
-					    tb_caps[NL802154_CAP_ATTR_CCA_OPTS],
-					    rem_cca_opts) {
-				printf("%d", nla_type(nl_cca_opts));
-				switch (nla_type(nl_cca_opts)) {
-				case NL802154_CCA_OPT_ENERGY_CARRIER_AND:
-				case NL802154_CCA_OPT_ENERGY_CARRIER_OR:
-					printf("(cca_mode: 3),");
-					break;
-				default:
-					printf("unkown\n");
-					break;
 				}
 			}
 			/* TODO */
-			printf("\b \n");
+			printf("\n");
 		}
 
 		if (tb_caps[NL802154_CAP_ATTR_MIN_MINBE] &&
-- 
2.1.4

Re: [PATCH v6 wpan-tools] info: update CCA mode and CCA options output

[Alexander Aring]

On Thu, Jun 11, 2015 at 02:25:38PM +0200, Christoffer Holmstedt wrote:
> Instead of printing out CCA mode and CCA options in integers descriptive
> text is now printed to the user when "iwpan list" is run.
> 
> Signed-off-by: Christoffer Holmstedt <christoffer@christofferholmstedt.se>

Applied, thanks.

- Alex