linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Stotland, Inga" <inga.stotland@intel.com>
To: "mike@mnmoran.org" <mike@mnmoran.org>,
	"linux-bluetooth@vger.kernel.org"
	<linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH BlueZ 1/1] mesh: Add strings for SIG Model IDs.
Date: Sun, 2 Aug 2020 07:04:39 +0000	[thread overview]
Message-ID: <b20b3a4a31a6393a1f1c035823fa20b7a778c1b2.camel@intel.com> (raw)
In-Reply-To: <d3547f5d-5dae-38fb-41c6-33454e8b1356@mnmoran.org>

Hi Michael,

Thanks for the patch. Few style tweaks.

First, no period at the end of commit title.

On Sat, 2020-08-01 at 00:28 -0400, Michael N. Moran wrote:
> This patch adds a utility function returning a string
> describing a SIG Model ID and uses this function
> for in the mesh-cfgclient in the list-nodes command
> output and in the display of received composition data.
> 
> ---
>   tools/mesh/cfgcli.c |  3 ++-
>   tools/mesh/remote.c |  5 ++--
>   tools/mesh/util.c   | 64 
> +++++++++++++++++++++++++++++++++++++++++++++
>   tools/mesh/util.h   |  1 +
>   4 files changed, 70 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/mesh/cfgcli.c b/tools/mesh/cfgcli.c
> index e36c8dca5..f6f465c6c 100644
> --- a/tools/mesh/cfgcli.c
> +++ b/tools/mesh/cfgcli.c
> @@ -263,7 +263,8 @@ static uint32_t print_mod_id(uint8_t 
> *data, bool vendor, const char *offset)
>    	if (!vendor) {
>   		mod_id = get_le16(data);
> -		bt_shell_printf("%sModel ID\t%4.4x\n", offset, mod_id);
> +		bt_shell_printf("%sModel ID\t%4.4x \"%s\"\n",
> +			offset, mod_id,sig_model_string(mod_id));

Space before sig_model_string.
Also, make sure that the  wrapped line
"offset,mod_id,sig_model_string(mod_id))" is
right adjusted, i.e., according to BlueZ coding style (as outlined in
doc/coding-style.txt):
"The referred style for line wrapping is to indent as far as possible
to the right without hitting the 80 columns limit."

>   		mod_id = VENDOR_ID_MASK | mod_id;
>   	} else {
>   		mod_id = get_le16(data + 2);
> diff --git a/tools/mesh/remote.c b/tools/mesh/remote.c
> index c74f0bec1..ba3e50d3f 100644
> --- a/tools/mesh/remote.c
> +++ b/tools/mesh/remote.c
> @@ -30,6 +30,7 @@
>   #include "tools/mesh/keys.h"
>   #include "tools/mesh/mesh-db.h"
>   #include "tools/mesh/remote.h"
> +#include "tools/mesh/util.h"

Line break between includes & defines

>    #define abs_diff(a, b) ((a) > (b) ? (a) - (b) : (b) - (a))
>   @@ -293,8 +294,8 @@ static void print_model(void *model, 
> void *user_data)
>    	if (mod_id >= VENDOR_ID_MASK) {
>   		mod_id &= ~VENDOR_ID_MASK;
> -		bt_shell_printf("\t\t\t" COLOR_GREEN "SIG model: %4.4x\n"
> -							COLOR_OFF, mod_id);
> +		bt_shell_printf("\t\t\t" COLOR_GREEN "SIG model: %4.4x 
> \"%s\"\n"
> +							COLOR_OFF, mod_id,sig_model_string(mod_id));

Space before sig_model_string.
Also, make sure that the  wrapped line
"offset,mod_id,sig_model_string(mod_id))" is
right adjusted, i.e., according to BlueZ coding style (as outlined in
doc/coding-style.txt):
"The referred style for line wrapping is to indent as far as possible
to the right without hitting the 80 columns limit."

>   		return;
>   	}
>   diff --git a/tools/mesh/util.c b/tools/mesh/util.c
> index 7176cc562..4603a6261 100644
> --- a/tools/mesh/util.c
> +++ b/tools/mesh/util.c
> @@ -138,3 +138,67 @@ void swap_u256_bytes(uint8_t *u256)
>   		u256[i] ^= u256[31 - i];
>   	}
>   }
> +
> +const char*	sig_model_string(uint16_t sig_model_id)

"char*	sig" -> "char *sig"

> +{
> +	switch (sig_model_id) {
> +	case 0x0000: return "Configuration Server";
> +	case 0x0001: return "Configuration Client";
> +	case 0x0002: return "Health Server";
> +	case 0x0003: return "Health Client";
> +	case 0x1000: return "Generic OnOff Server";
> +	case 0x1001: return "Generic OnOff Client";
> +	case 0x1002: return "Generic Level Server";
> +	case 0x1003: return "Generic Level Client";
> +	case 0x1004: return "Generic Default Transition Time Server";
> +	case 0x1005: return "Generic Default Transition Time Client";
> +	case 0x1006: return "Generic Power OnOff Server";
> +	case 0x1007: return "Generic Power OnOff Setup Server";
> +	case 0x1008: return "Generic Power OnOff Client";
> +	case 0x1009: return "Generic Power Level Server";
> +	case 0x100A: return "Generic Power Level Setup Server";
> +	case 0x100B: return "Generic Power Level Client";
> +	case 0x100C: return "Generic Battery Server";
> +	case 0x100D: return "Generic Battery Client";
> +	case 0x100E: return "Generic Location Server";
> +	case 0x100F: return "Generic Location Setup Server";
> +	case 0x1010: return "Generic Location Client";
> +	case 0x1011: return "Generic Admin Property Server";
> +	case 0x1012: return "Generic Manufacturer Property Server";
> +	case 0x1013: return "Generic User Property Server";
> +	case 0x1014: return "Generic Client Property Server";
> +	case 0x1015: return "Generic Property Client";
> +	case 0x1100: return "Sensor Server";
> +	case 0x1101: return "Sensor Setup Server";
> +	case 0x1102: return "Sensor Client";
> +	case 0x1200: return "Time Server";
> +	case 0x1201: return "Time Setup Server";
> +	case 0x1202: return "Time Client";
> +	case 0x1203: return "Scene Server";
> +	case 0x1204: return "Scene Setup Server";
> +	case 0x1205: return "Scene Client";
> +	case 0x1206: return "Scheduler Server";
> +	case 0x1207: return "Scheduler Setup Server";
> +	case 0x1208: return "Scheduler Client";
> +	case 0x1300: return "Light Lightness Server";
> +	case 0x1301: return "Light Lightness Setup Server";
> +	case 0x1302: return "Light Lightness Client";
> +	case 0x1303: return "Light CTL Server";
> +	case 0x1304: return "Light CTL Setup Server";
> +	case 0x1305: return "Light CTL Client";
> +	case 0x1306: return "Light CTL Temperature Server";
> +	case 0x1307: return "Light HSL Server";
> +	case 0x1308: return "Light HSL Setup Server";
> +	case 0x1309: return "Light HSL Client";
> +	case 0x130A: return "Light HSL Hue Server";
> +	case 0x130B: return "Light HSL Saturation Server";
> +	case 0x130C: return "Light xyL Server";
> +	case 0x130D: return "Light xyL Setup Server";
> +	case 0x130E: return "Light xyL Client";
> +	case 0x130F: return "Light LC Server";
> +	case 0x1310: return "Light LC Setup Server";
> +	case 0x1311: return "Light LC Client";
> +
> +	default: return "Unknown";
> +	}
> +}
> diff --git a/tools/mesh/util.h b/tools/mesh/util.h
> index cca07cf96..7e966bc69 100644
> --- a/tools/mesh/util.h
> +++ b/tools/mesh/util.h
> @@ -27,3 +27,4 @@ uint16_t mesh_opcode_set(uint32_t opcode, 
> uint8_t *buf);
>   bool mesh_opcode_get(const uint8_t *buf, uint16_t sz, 
> uint32_t *opcode, int *n);
>   const char *mesh_status_str(uint8_t status);
>   void swap_u256_bytes(uint8_t *u256);
> +const char*	sig_model_string(uint16_t sig_model_id);

"char*	sig" -> "char *sig"


Regards,
Inga

      reply	other threads:[~2020-08-02  7:05 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-01  4:28 [PATCH BlueZ 1/1] mesh: Add strings for SIG Model IDs Michael N. Moran
2020-08-02  7:04 ` Stotland, Inga [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b20b3a4a31a6393a1f1c035823fa20b7a778c1b2.camel@intel.com \
    --to=inga.stotland@intel.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=mike@mnmoran.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).