2017-09-22 04:59:58

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 0/4] Move common client code to bt_shell

This is first part of moving common code of client and meshctl to shared
directory. Next steps would be:
- functions for switching between menus (needed for meshctl)
- display functions
- common support for history


Marcin Kraglak (4):
shared/bt_shell: Add initial implementation
client: Use bt_shell to process commands.
shared/bt_shell: Add bt_shell_completion
client: Use bt_shell_completion

Makefile.tools | 3 +-
client/main.c | 93 +++---------------------------
src/shared/bt_shell.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++
src/shared/bt_shell.h | 43 ++++++++++++++
4 files changed, 204 insertions(+), 87 deletions(-)
create mode 100644 src/shared/bt_shell.c
create mode 100644 src/shared/bt_shell.h

--
2.13.5



2017-09-22 08:09:19

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 4/4] client: Use bt_shell_completion

Hi Marcin,

On Fri, Sep 22, 2017 at 11:00 AM, Marcin Kraglak
<[email protected]> wrote:
> Hi Luiz,
>
> On 22 September 2017 at 03:47, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Marcin,
>>
>> On Fri, Sep 22, 2017 at 8:00 AM, Marcin Kraglak
>> <[email protected]> wrote:
>>> ---
>>> client/main.c | 49 +------------------------------------------------
>>> 1 file changed, 1 insertion(+), 48 deletions(-)
>>>
>>> diff --git a/client/main.c b/client/main.c
>>> index 87019b463..86514f67e 100644
>>> --- a/client/main.c
>>> +++ b/client/main.c
>>> @@ -2564,61 +2564,14 @@ static const struct bt_shell_menu_entry cmd_table[] = {
>>> { }
>>> };
>>>
>>> -static char *cmd_generator(const char *text, int state)
>>> -{
>>> - static int index, len;
>>> - const char *cmd;
>>> -
>>> - if (!state) {
>>> - index = 0;
>>> - len = strlen(text);
>>> - }
>>> -
>>> - while ((cmd = cmd_table[index].cmd)) {
>>> - index++;
>>> -
>>> - if (!strncmp(cmd, text, len))
>>> - return strdup(cmd);
>>> - }
>>> -
>>> - return NULL;
>>> -}
>>> -
>>> static char **cmd_completion(const char *text, int start, int end)
>>> {
>>> - char **matches = NULL;
>>> -
>>> if (agent_completion() == TRUE) {
>>> rl_attempted_completion_over = 1;
>>> return NULL;
>>> }
>>>
>>> - if (start > 0) {
>>> - int i;
>>> - char *input_cmd;
>>> -
>>> - input_cmd = g_strndup(rl_line_buffer, start -1);
>>> - for (i = 0; cmd_table[i].cmd; i++) {
>>> - if (strcmp(cmd_table[i].cmd, input_cmd))
>>> - continue;
>>> -
>>> - if (!cmd_table[i].gen)
>>> - continue;
>>> -
>>> - rl_completion_display_matches_hook = cmd_table[i].disp;
>>> - matches = rl_completion_matches(text, cmd_table[i].gen);
>>> - break;
>>> - }
>>> - g_free(input_cmd);
>>> - } else {
>>> - rl_completion_display_matches_hook = NULL;
>>> - matches = rl_completion_matches(text, cmd_generator);
>>> - }
>>> -
>>> - if (!matches)
>>> - rl_attempted_completion_over = 1;
>>> -
>>> - return matches;
>>> + return bt_shell_completion(text, start, end);
>>> }
>>
>> While this is great as we reuse more code I think we better move the
>> whole input handling into the bt_shell so the application don't even
>> need to call bt_shell_completion, etc.

Not only that but the entire readline setup including
rl_callback_handler_install which means the tools don't really need to
use readline functions just bt_shell.
>> --
>> Luiz Augusto von Dentz
>
> You mean bt_shell should set rl_attempted_completion_function in
> bt_shell_init() (then bt_shell_completion is not exposed)?
>
>
>
> --
> BR
> Marcin Kraglak



--
Luiz Augusto von Dentz

2017-09-22 08:00:02

by Marcin Kraglak

[permalink] [raw]
Subject: Re: [PATCH 4/4] client: Use bt_shell_completion

Hi Luiz,

On 22 September 2017 at 03:47, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Marcin,
>
> On Fri, Sep 22, 2017 at 8:00 AM, Marcin Kraglak
> <[email protected]> wrote:
>> ---
>> client/main.c | 49 +------------------------------------------------
>> 1 file changed, 1 insertion(+), 48 deletions(-)
>>
>> diff --git a/client/main.c b/client/main.c
>> index 87019b463..86514f67e 100644
>> --- a/client/main.c
>> +++ b/client/main.c
>> @@ -2564,61 +2564,14 @@ static const struct bt_shell_menu_entry cmd_table[] = {
>> { }
>> };
>>
>> -static char *cmd_generator(const char *text, int state)
>> -{
>> - static int index, len;
>> - const char *cmd;
>> -
>> - if (!state) {
>> - index = 0;
>> - len = strlen(text);
>> - }
>> -
>> - while ((cmd = cmd_table[index].cmd)) {
>> - index++;
>> -
>> - if (!strncmp(cmd, text, len))
>> - return strdup(cmd);
>> - }
>> -
>> - return NULL;
>> -}
>> -
>> static char **cmd_completion(const char *text, int start, int end)
>> {
>> - char **matches = NULL;
>> -
>> if (agent_completion() == TRUE) {
>> rl_attempted_completion_over = 1;
>> return NULL;
>> }
>>
>> - if (start > 0) {
>> - int i;
>> - char *input_cmd;
>> -
>> - input_cmd = g_strndup(rl_line_buffer, start -1);
>> - for (i = 0; cmd_table[i].cmd; i++) {
>> - if (strcmp(cmd_table[i].cmd, input_cmd))
>> - continue;
>> -
>> - if (!cmd_table[i].gen)
>> - continue;
>> -
>> - rl_completion_display_matches_hook = cmd_table[i].disp;
>> - matches = rl_completion_matches(text, cmd_table[i].gen);
>> - break;
>> - }
>> - g_free(input_cmd);
>> - } else {
>> - rl_completion_display_matches_hook = NULL;
>> - matches = rl_completion_matches(text, cmd_generator);
>> - }
>> -
>> - if (!matches)
>> - rl_attempted_completion_over = 1;
>> -
>> - return matches;
>> + return bt_shell_completion(text, start, end);
>> }
>
> While this is great as we reuse more code I think we better move the
> whole input handling into the bt_shell so the application don't even
> need to call bt_shell_completion, etc.
>
> --
> Luiz Augusto von Dentz

You mean bt_shell should set rl_attempted_completion_function in
bt_shell_init() (then bt_shell_completion is not exposed)?



--
BR
Marcin Kraglak

2017-09-22 07:52:49

by Marcin Kraglak

[permalink] [raw]
Subject: Re: [PATCH 1/4] shared/bt_shell: Add initial implementation

Hi Luiz Eramoto,

On 22 September 2017 at 03:42, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Eramoto, Marcin,
>
> On Fri, Sep 22, 2017 at 9:27 AM, ERAMOTO Masaya
> <[email protected]> wrote:
>> Hi Marcin,
>>
>>> +#ifdef HAVE_CONFIG_H
>>> +#include <config.h>
>>> +#endif
>>> +
>>> +#include <stdio.h>
>>> +#include "src/shared/util.h"
>>> +#include "src/shared/queue.h"
>>> +#include "src/shared/bt_shell.h"
>>> +
>>> +#define CMD_LENGTH 48
>>> +
>>> +static struct {
>>> + const struct bt_shell_menu_entry *current;
>>> +} bt_shell_data;
>>> +
>>> +bool bt_shell_init(const struct bt_shell_menu_entry *menu)
>>> +{
>>> + if (bt_shell_data.current || !menu)
>>> + return false;
>>> +
>>> + bt_shell_data.current = menu;
>>> +
>>> + return true;
>>> +}
>>> +
>>> +void bt_shell_cleanup(void)
>>> +{
>>> + bt_shell_data.current = NULL;
>>> +}
>>> +
>>> +void bt_shell_process(const char *cmd, const char *arg)
>>> +{
>>> + const struct bt_shell_menu_entry *entry;
>>> +
>>> + if (!bt_shell_data.current || !cmd)
>>> + return;
>>> +
>>> + for (entry = bt_shell_data.current; entry->cmd; entry++) {
>>> + if (strcmp(cmd, entry->cmd))
>>> + continue;
>>> +
>>> + if (entry->func) {
>>> + entry->func(arg);
>>> + return;
>>> + }
>>> + }
>>> +
>>> + if (strcmp(cmd, "help")) {
>>> + printf("Invalid command\n");
>>> + return;
>>> + }
>>> +
>>> + bt_shell_print_menu();
>>> +}
>>> +
>>> +void bt_shell_print_menu(void)
>>> +{
>>> + const struct bt_shell_menu_entry *entry;
>>> +
>>> + if (!bt_shell_data.current)
>>> + return;
>>> +
>>> + printf("Available commands:\n");
>>> + for (entry = bt_shell_data.current; entry->cmd; entry++) {
>>> + printf(" %s %-*s %s\n", entry->cmd,
>>> + (int)(CMD_LENGTH - strlen(entry->cmd)),
>>> + entry->arg ? : "", entry->desc ? : "");
>>
>>
>> I think that it is better to
>> - add some white-spaces
>> or
>> - add a new line
>> between the argument string and the description string, because it is a little
>> difficult for the help of register-characteristic to read as below:
>>
>> [bluetooth]# help
>> Available commands:
>> ...
>> unregister-service <UUID/object> Unregister application service
>> register-characteristic <UUID> <Flags=read,write,notify...> Register application characteristic
>> unregister-characteristic <UUID/object> Unregister application characteristic
>> register-descriptor <UUID> <Flags=read,write...> Register application descriptor
>
> It should probably have the same formatting as bluetoothctl:
>
> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/client/main.c#n2677
>
> --
> Luiz Augusto von Dentz

Yes, I'll correct it

--
BR
Marcin Kraglak

2017-09-22 07:47:02

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 4/4] client: Use bt_shell_completion

Hi Marcin,

On Fri, Sep 22, 2017 at 8:00 AM, Marcin Kraglak
<[email protected]> wrote:
> ---
> client/main.c | 49 +------------------------------------------------
> 1 file changed, 1 insertion(+), 48 deletions(-)
>
> diff --git a/client/main.c b/client/main.c
> index 87019b463..86514f67e 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -2564,61 +2564,14 @@ static const struct bt_shell_menu_entry cmd_table[] = {
> { }
> };
>
> -static char *cmd_generator(const char *text, int state)
> -{
> - static int index, len;
> - const char *cmd;
> -
> - if (!state) {
> - index = 0;
> - len = strlen(text);
> - }
> -
> - while ((cmd = cmd_table[index].cmd)) {
> - index++;
> -
> - if (!strncmp(cmd, text, len))
> - return strdup(cmd);
> - }
> -
> - return NULL;
> -}
> -
> static char **cmd_completion(const char *text, int start, int end)
> {
> - char **matches = NULL;
> -
> if (agent_completion() == TRUE) {
> rl_attempted_completion_over = 1;
> return NULL;
> }
>
> - if (start > 0) {
> - int i;
> - char *input_cmd;
> -
> - input_cmd = g_strndup(rl_line_buffer, start -1);
> - for (i = 0; cmd_table[i].cmd; i++) {
> - if (strcmp(cmd_table[i].cmd, input_cmd))
> - continue;
> -
> - if (!cmd_table[i].gen)
> - continue;
> -
> - rl_completion_display_matches_hook = cmd_table[i].disp;
> - matches = rl_completion_matches(text, cmd_table[i].gen);
> - break;
> - }
> - g_free(input_cmd);
> - } else {
> - rl_completion_display_matches_hook = NULL;
> - matches = rl_completion_matches(text, cmd_generator);
> - }
> -
> - if (!matches)
> - rl_attempted_completion_over = 1;
> -
> - return matches;
> + return bt_shell_completion(text, start, end);
> }

While this is great as we reuse more code I think we better move the
whole input handling into the bt_shell so the application don't even
need to call bt_shell_completion, etc.

--
Luiz Augusto von Dentz

2017-09-22 07:42:20

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/4] shared/bt_shell: Add initial implementation

Hi Eramoto, Marcin,

On Fri, Sep 22, 2017 at 9:27 AM, ERAMOTO Masaya
<[email protected]> wrote:
> Hi Marcin,
>
>> +#ifdef HAVE_CONFIG_H
>> +#include <config.h>
>> +#endif
>> +
>> +#include <stdio.h>
>> +#include "src/shared/util.h"
>> +#include "src/shared/queue.h"
>> +#include "src/shared/bt_shell.h"
>> +
>> +#define CMD_LENGTH 48
>> +
>> +static struct {
>> + const struct bt_shell_menu_entry *current;
>> +} bt_shell_data;
>> +
>> +bool bt_shell_init(const struct bt_shell_menu_entry *menu)
>> +{
>> + if (bt_shell_data.current || !menu)
>> + return false;
>> +
>> + bt_shell_data.current = menu;
>> +
>> + return true;
>> +}
>> +
>> +void bt_shell_cleanup(void)
>> +{
>> + bt_shell_data.current = NULL;
>> +}
>> +
>> +void bt_shell_process(const char *cmd, const char *arg)
>> +{
>> + const struct bt_shell_menu_entry *entry;
>> +
>> + if (!bt_shell_data.current || !cmd)
>> + return;
>> +
>> + for (entry = bt_shell_data.current; entry->cmd; entry++) {
>> + if (strcmp(cmd, entry->cmd))
>> + continue;
>> +
>> + if (entry->func) {
>> + entry->func(arg);
>> + return;
>> + }
>> + }
>> +
>> + if (strcmp(cmd, "help")) {
>> + printf("Invalid command\n");
>> + return;
>> + }
>> +
>> + bt_shell_print_menu();
>> +}
>> +
>> +void bt_shell_print_menu(void)
>> +{
>> + const struct bt_shell_menu_entry *entry;
>> +
>> + if (!bt_shell_data.current)
>> + return;
>> +
>> + printf("Available commands:\n");
>> + for (entry = bt_shell_data.current; entry->cmd; entry++) {
>> + printf(" %s %-*s %s\n", entry->cmd,
>> + (int)(CMD_LENGTH - strlen(entry->cmd)),
>> + entry->arg ? : "", entry->desc ? : "");
>
>
> I think that it is better to
> - add some white-spaces
> or
> - add a new line
> between the argument string and the description string, because it is a little
> difficult for the help of register-characteristic to read as below:
>
> [bluetooth]# help
> Available commands:
> ...
> unregister-service <UUID/object> Unregister application service
> register-characteristic <UUID> <Flags=read,write,notify...> Register application characteristic
> unregister-characteristic <UUID/object> Unregister application characteristic
> register-descriptor <UUID> <Flags=read,write...> Register application descriptor

It should probably have the same formatting as bluetoothctl:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/client/main.c#n2677

--
Luiz Augusto von Dentz

2017-09-22 06:27:14

by ERAMOTO Masaya

[permalink] [raw]
Subject: Re: [PATCH 1/4] shared/bt_shell: Add initial implementation

Hi Marcin,

> +#ifdef HAVE_CONFIG_H
> +#include <config.h>
> +#endif
> +
> +#include <stdio.h>
> +#include "src/shared/util.h"
> +#include "src/shared/queue.h"
> +#include "src/shared/bt_shell.h"
> +
> +#define CMD_LENGTH 48
> +
> +static struct {
> + const struct bt_shell_menu_entry *current;
> +} bt_shell_data;
> +
> +bool bt_shell_init(const struct bt_shell_menu_entry *menu)
> +{
> + if (bt_shell_data.current || !menu)
> + return false;
> +
> + bt_shell_data.current = menu;
> +
> + return true;
> +}
> +
> +void bt_shell_cleanup(void)
> +{
> + bt_shell_data.current = NULL;
> +}
> +
> +void bt_shell_process(const char *cmd, const char *arg)
> +{
> + const struct bt_shell_menu_entry *entry;
> +
> + if (!bt_shell_data.current || !cmd)
> + return;
> +
> + for (entry = bt_shell_data.current; entry->cmd; entry++) {
> + if (strcmp(cmd, entry->cmd))
> + continue;
> +
> + if (entry->func) {
> + entry->func(arg);
> + return;
> + }
> + }
> +
> + if (strcmp(cmd, "help")) {
> + printf("Invalid command\n");
> + return;
> + }
> +
> + bt_shell_print_menu();
> +}
> +
> +void bt_shell_print_menu(void)
> +{
> + const struct bt_shell_menu_entry *entry;
> +
> + if (!bt_shell_data.current)
> + return;
> +
> + printf("Available commands:\n");
> + for (entry = bt_shell_data.current; entry->cmd; entry++) {
> + printf(" %s %-*s %s\n", entry->cmd,
> + (int)(CMD_LENGTH - strlen(entry->cmd)),
> + entry->arg ? : "", entry->desc ? : "");


I think that it is better to
- add some white-spaces
or
- add a new line
between the argument string and the description string, because it is a little
difficult for the help of register-characteristic to read as below:

[bluetooth]# help
Available commands:
...
unregister-service <UUID/object> Unregister application service
register-characteristic <UUID> <Flags=read,write,notify...> Register application characteristic
unregister-characteristic <UUID/object> Unregister application characteristic
register-descriptor <UUID> <Flags=read,write...> Register application descriptor


Regards,
Eramoto


2017-09-22 05:00:01

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 3/4] shared/bt_shell: Add bt_shell_completion

This function may be used as rl_attempted_completion_function
in applications using readline.
---
src/shared/bt_shell.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++
src/shared/bt_shell.h | 1 +
2 files changed, 61 insertions(+)

diff --git a/src/shared/bt_shell.c b/src/shared/bt_shell.c
index 78d23c7ea..d27020b7f 100644
--- a/src/shared/bt_shell.c
+++ b/src/shared/bt_shell.c
@@ -26,6 +26,7 @@
#endif

#include <stdio.h>
+#include <readline/readline.h>
#include "src/shared/util.h"
#include "src/shared/queue.h"
#include "src/shared/bt_shell.h"
@@ -90,3 +91,62 @@ void bt_shell_print_menu(void)
entry->arg ? : "", entry->desc ? : "");
}
}
+
+static char *cmd_generator(const char *text, int state)
+{
+ const struct bt_shell_menu_entry *entry;
+ static int index, len;
+ const char *cmd;
+
+ entry = bt_shell_data.current;
+
+ if (!state) {
+ index = 0;
+ len = strlen(text);
+ }
+
+ while ((cmd = entry[index].cmd)) {
+ index++;
+
+ if (!strncmp(cmd, text, len))
+ return strdup(cmd);
+ }
+
+ return NULL;
+}
+
+char **bt_shell_completion(const char *text, int start, int end)
+{
+ char **matches = NULL;
+
+ if (!bt_shell_data.current)
+ return NULL;
+
+ if (start > 0) {
+ const struct bt_shell_menu_entry *entry;
+ char *input_cmd;
+
+ input_cmd = strndup(rl_line_buffer, start - 1);
+ for (entry = bt_shell_data.current; entry->cmd; entry++) {
+ if (strcmp(entry->cmd, input_cmd))
+ continue;
+
+ if (!entry->gen)
+ continue;
+
+ rl_completion_display_matches_hook = entry->disp;
+ matches = rl_completion_matches(text, entry->gen);
+ break;
+ }
+
+ free(input_cmd);
+ } else {
+ rl_completion_display_matches_hook = NULL;
+ matches = rl_completion_matches(text, cmd_generator);
+ }
+
+ if (!matches)
+ rl_attempted_completion_over = 1;
+
+ return matches;
+}
diff --git a/src/shared/bt_shell.h b/src/shared/bt_shell.h
index 93d7ed771..c30a3dcbd 100644
--- a/src/shared/bt_shell.h
+++ b/src/shared/bt_shell.h
@@ -39,4 +39,5 @@ bool bt_shell_init(const struct bt_shell_menu_entry *menu);
void bt_shell_cleanup(void);

void bt_shell_process(const char *cmd, const char *arg);
+char **bt_shell_completion(const char *text, int start, int end);
void bt_shell_print_menu(void);
--
2.13.5


2017-09-22 05:00:02

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 4/4] client: Use bt_shell_completion

---
client/main.c | 49 +------------------------------------------------
1 file changed, 1 insertion(+), 48 deletions(-)

diff --git a/client/main.c b/client/main.c
index 87019b463..86514f67e 100644
--- a/client/main.c
+++ b/client/main.c
@@ -2564,61 +2564,14 @@ static const struct bt_shell_menu_entry cmd_table[] = {
{ }
};

-static char *cmd_generator(const char *text, int state)
-{
- static int index, len;
- const char *cmd;
-
- if (!state) {
- index = 0;
- len = strlen(text);
- }
-
- while ((cmd = cmd_table[index].cmd)) {
- index++;
-
- if (!strncmp(cmd, text, len))
- return strdup(cmd);
- }
-
- return NULL;
-}
-
static char **cmd_completion(const char *text, int start, int end)
{
- char **matches = NULL;
-
if (agent_completion() == TRUE) {
rl_attempted_completion_over = 1;
return NULL;
}

- if (start > 0) {
- int i;
- char *input_cmd;
-
- input_cmd = g_strndup(rl_line_buffer, start -1);
- for (i = 0; cmd_table[i].cmd; i++) {
- if (strcmp(cmd_table[i].cmd, input_cmd))
- continue;
-
- if (!cmd_table[i].gen)
- continue;
-
- rl_completion_display_matches_hook = cmd_table[i].disp;
- matches = rl_completion_matches(text, cmd_table[i].gen);
- break;
- }
- g_free(input_cmd);
- } else {
- rl_completion_display_matches_hook = NULL;
- matches = rl_completion_matches(text, cmd_generator);
- }
-
- if (!matches)
- rl_attempted_completion_over = 1;
-
- return matches;
+ return bt_shell_completion(text, start, end);
}

static void rl_handler(char *input)
--
2.13.5


2017-09-22 05:00:00

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 2/4] client: Use bt_shell to process commands.

Use bt_shell to process and print available commands.
---
Makefile.tools | 3 ++-
client/main.c | 44 ++++++--------------------------------------
2 files changed, 8 insertions(+), 39 deletions(-)

diff --git a/Makefile.tools b/Makefile.tools
index 561302fa1..4893acd8a 100644
--- a/Makefile.tools
+++ b/Makefile.tools
@@ -8,7 +8,8 @@ client_bluetoothctl_SOURCES = client/main.c \
client/advertising.h \
client/advertising.c \
client/gatt.h client/gatt.c \
- monitor/uuid.h monitor/uuid.c
+ monitor/uuid.h monitor/uuid.c \
+ src/shared/bt_shell.h src/shared/bt_shell.c
client_bluetoothctl_LDADD = gdbus/libgdbus-internal.la src/libshared-glib.la \
@GLIB_LIBS@ @DBUS_LIBS@ -lreadline
endif
diff --git a/client/main.c b/client/main.c
index 91b728a12..87019b463 100644
--- a/client/main.c
+++ b/client/main.c
@@ -38,6 +38,7 @@
#include <readline/history.h>
#include <glib.h>

+#include "src/shared/bt_shell.h"
#include "src/shared/util.h"
#include "gdbus/gdbus.h"
#include "monitor/uuid.h"
@@ -2435,14 +2436,7 @@ static void cmd_set_advertise_appearance(const char *arg)
ad_advertise_local_appearance(dbus_conn, value);
}

-static const struct {
- const char *cmd;
- const char *arg;
- void (*func) (const char *arg);
- const char *desc;
- char * (*gen) (const char *text, int state);
- void (*disp) (char **matches, int num_matches, int max_length);
-} cmd_table[] = {
+static const struct bt_shell_menu_entry cmd_table[] = {
{ "list", NULL, cmd_list, "List available controllers" },
{ "show", "[ctrl]", cmd_show, "Controller information",
ctrl_generator },
@@ -2630,7 +2624,6 @@ static char **cmd_completion(const char *text, int start, int end)
static void rl_handler(char *input)
{
char *cmd, *arg;
- int i;

if (!input) {
rl_insert_text("quit");
@@ -2659,41 +2652,14 @@ static void rl_handler(char *input)
arg[len - 1] = '\0';
}

- for (i = 0; cmd_table[i].cmd; i++) {
- if (strcmp(cmd, cmd_table[i].cmd))
- continue;
-
- if (cmd_table[i].func) {
- cmd_table[i].func(arg);
- goto done;
- }
- }
-
- printf("Invalid command\n");
+ bt_shell_process(cmd, arg);
done:
free(input);
}

static void cmd_help(const char *arg)
{
- int i;
-
- printf("Available commands:\n");
-
- for (i = 0; cmd_table[i].cmd; i++) {
- if ((int)strlen(cmd_table[i].arg? : "") <=
- (int)(25 - strlen(cmd_table[i].cmd)))
- printf(" %s %-*s %s\n", cmd_table[i].cmd,
- (int)(25 - strlen(cmd_table[i].cmd)),
- cmd_table[i].arg ? : "",
- cmd_table[i].desc ? : "");
- else
- printf(" %s %-s\n" " %s %-25s %s\n",
- cmd_table[i].cmd,
- cmd_table[i].arg ? : "",
- "", "",
- cmd_table[i].desc ? : "");
- }
+ bt_shell_print_menu();
}

static gboolean signal_handler(GIOChannel *channel, GIOCondition condition,
@@ -2844,6 +2810,7 @@ int main(int argc, char *argv[])
dbus_conn = g_dbus_setup_bus(DBUS_BUS_SYSTEM, NULL, NULL);
g_dbus_attach_object_manager(dbus_conn);

+ bt_shell_init(cmd_table);
setlinebuf(stdout);
rl_attempted_completion_function = cmd_completion;

@@ -2874,6 +2841,7 @@ int main(int argc, char *argv[])

rl_message("");
rl_callback_handler_remove();
+ bt_shell_cleanup();

dbus_connection_unref(dbus_conn);
g_main_loop_unref(main_loop);
--
2.13.5


2017-09-22 04:59:59

by Marcin Kraglak

[permalink] [raw]
Subject: [PATCH 1/4] shared/bt_shell: Add initial implementation

This module will handle command line in applications like bluetoothctl and meshctl.
---
src/shared/bt_shell.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++++
src/shared/bt_shell.h | 42 +++++++++++++++++++++++
2 files changed, 134 insertions(+)
create mode 100644 src/shared/bt_shell.c
create mode 100644 src/shared/bt_shell.h

diff --git a/src/shared/bt_shell.c b/src/shared/bt_shell.c
new file mode 100644
index 000000000..78d23c7ea
--- /dev/null
+++ b/src/shared/bt_shell.c
@@ -0,0 +1,92 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2017 Intel Corporation. All rights reserved.
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include "src/shared/util.h"
+#include "src/shared/queue.h"
+#include "src/shared/bt_shell.h"
+
+#define CMD_LENGTH 48
+
+static struct {
+ const struct bt_shell_menu_entry *current;
+} bt_shell_data;
+
+bool bt_shell_init(const struct bt_shell_menu_entry *menu)
+{
+ if (bt_shell_data.current || !menu)
+ return false;
+
+ bt_shell_data.current = menu;
+
+ return true;
+}
+
+void bt_shell_cleanup(void)
+{
+ bt_shell_data.current = NULL;
+}
+
+void bt_shell_process(const char *cmd, const char *arg)
+{
+ const struct bt_shell_menu_entry *entry;
+
+ if (!bt_shell_data.current || !cmd)
+ return;
+
+ for (entry = bt_shell_data.current; entry->cmd; entry++) {
+ if (strcmp(cmd, entry->cmd))
+ continue;
+
+ if (entry->func) {
+ entry->func(arg);
+ return;
+ }
+ }
+
+ if (strcmp(cmd, "help")) {
+ printf("Invalid command\n");
+ return;
+ }
+
+ bt_shell_print_menu();
+}
+
+void bt_shell_print_menu(void)
+{
+ const struct bt_shell_menu_entry *entry;
+
+ if (!bt_shell_data.current)
+ return;
+
+ printf("Available commands:\n");
+ for (entry = bt_shell_data.current; entry->cmd; entry++) {
+ printf(" %s %-*s %s\n", entry->cmd,
+ (int)(CMD_LENGTH - strlen(entry->cmd)),
+ entry->arg ? : "", entry->desc ? : "");
+ }
+}
diff --git a/src/shared/bt_shell.h b/src/shared/bt_shell.h
new file mode 100644
index 000000000..93d7ed771
--- /dev/null
+++ b/src/shared/bt_shell.h
@@ -0,0 +1,42 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2017 Intel Corporation. All rights reserved.
+ *
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+typedef void (*bt_shell_menu_cb_t)(const char *arg);
+typedef char * (*bt_shell_menu_gen_t)(const char *text, int state);
+typedef void (*bt_shell_menu_disp_t) (char **matches, int num_matches,
+ int max_length);
+
+struct bt_shell_menu_entry {
+ const char *cmd;
+ const char *arg;
+ bt_shell_menu_cb_t func;
+ const char *desc;
+ bt_shell_menu_gen_t gen;
+ bt_shell_menu_disp_t disp;
+};
+
+bool bt_shell_init(const struct bt_shell_menu_entry *menu);
+void bt_shell_cleanup(void);
+
+void bt_shell_process(const char *cmd, const char *arg);
+void bt_shell_print_menu(void);
--
2.13.5


2017-09-22 04:48:34

by Marcin Kraglak

[permalink] [raw]
Subject: Re: [PATCH 0/4] Move common client code to bt_shell

Hi Marcel,

On 21 September 2017 at 14:36, Marcel Holtmann <[email protected]> wrote:
> Hi Marcin,
>
>> This is first part of moving common code of client and meshctl to shared
>> directory. Next steps would be:
>> - functions for switching between menus (needed for meshctl)
>> - display functions
>> - common support for history
>>
>> Marcin Kraglak (4):
>> shared/shell/bt_shell: Add initial implementation
>> client: Use bt_shell to process commands.
>> shared/shell/bt_shell: Add bt_shell_completion
>> client: Use bt_shell_completion
>>
>> Makefile.tools | 3 +-
>> client/main.c | 93 ++-------------------------
>> src/shared/shell/bt_shell.c | 152 ++++++++++++++++++++++++++++++++++++++++++++
>> src/shared/shell/bt_shell.h | 43 +++++++++++++
>
> I am not a big fan of this deep directory structure. So either src/shell/ or put it all src/shared/. However I am sure tab-completion will be a pain with src/shell and src/shared ;)
Right, I'll move it to src/shared.
>
> Regards
>
> Marcel
>



--
BR
Marcin Kraglak

2017-09-21 18:36:20

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 0/4] Move common client code to bt_shell

Hi Marcin,

> This is first part of moving common code of client and meshctl to shared
> directory. Next steps would be:
> - functions for switching between menus (needed for meshctl)
> - display functions
> - common support for history
>
> Marcin Kraglak (4):
> shared/shell/bt_shell: Add initial implementation
> client: Use bt_shell to process commands.
> shared/shell/bt_shell: Add bt_shell_completion
> client: Use bt_shell_completion
>
> Makefile.tools | 3 +-
> client/main.c | 93 ++-------------------------
> src/shared/shell/bt_shell.c | 152 ++++++++++++++++++++++++++++++++++++++++++++
> src/shared/shell/bt_shell.h | 43 +++++++++++++

I am not a big fan of this deep directory structure. So either src/shell/ or put it all src/shared/. However I am sure tab-completion will be a pain with src/shell and src/shared ;)

Regards

Marcel


2017-10-23 11:42:02

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/4] shared/bt_shell: Add initial implementation

Hi Marcin,

On Fri, Sep 22, 2017 at 10:52 AM, Marcin Kraglak
<[email protected]> wrote:
> Hi Luiz Eramoto,
>
> On 22 September 2017 at 03:42, Luiz Augusto von Dentz
> <[email protected]> wrote:
>> Hi Eramoto, Marcin,
>>
>> On Fri, Sep 22, 2017 at 9:27 AM, ERAMOTO Masaya
>> <[email protected]> wrote:
>>> Hi Marcin,
>>>
>>>> +#ifdef HAVE_CONFIG_H
>>>> +#include <config.h>
>>>> +#endif
>>>> +
>>>> +#include <stdio.h>
>>>> +#include "src/shared/util.h"
>>>> +#include "src/shared/queue.h"
>>>> +#include "src/shared/bt_shell.h"
>>>> +
>>>> +#define CMD_LENGTH 48
>>>> +
>>>> +static struct {
>>>> + const struct bt_shell_menu_entry *current;
>>>> +} bt_shell_data;
>>>> +
>>>> +bool bt_shell_init(const struct bt_shell_menu_entry *menu)
>>>> +{
>>>> + if (bt_shell_data.current || !menu)
>>>> + return false;
>>>> +
>>>> + bt_shell_data.current = menu;
>>>> +
>>>> + return true;
>>>> +}
>>>> +
>>>> +void bt_shell_cleanup(void)
>>>> +{
>>>> + bt_shell_data.current = NULL;
>>>> +}
>>>> +
>>>> +void bt_shell_process(const char *cmd, const char *arg)
>>>> +{
>>>> + const struct bt_shell_menu_entry *entry;
>>>> +
>>>> + if (!bt_shell_data.current || !cmd)
>>>> + return;
>>>> +
>>>> + for (entry = bt_shell_data.current; entry->cmd; entry++) {
>>>> + if (strcmp(cmd, entry->cmd))
>>>> + continue;
>>>> +
>>>> + if (entry->func) {
>>>> + entry->func(arg);
>>>> + return;
>>>> + }
>>>> + }
>>>> +
>>>> + if (strcmp(cmd, "help")) {
>>>> + printf("Invalid command\n");
>>>> + return;
>>>> + }
>>>> +
>>>> + bt_shell_print_menu();
>>>> +}
>>>> +
>>>> +void bt_shell_print_menu(void)
>>>> +{
>>>> + const struct bt_shell_menu_entry *entry;
>>>> +
>>>> + if (!bt_shell_data.current)
>>>> + return;
>>>> +
>>>> + printf("Available commands:\n");
>>>> + for (entry = bt_shell_data.current; entry->cmd; entry++) {
>>>> + printf(" %s %-*s %s\n", entry->cmd,
>>>> + (int)(CMD_LENGTH - strlen(entry->cmd)),
>>>> + entry->arg ? : "", entry->desc ? : "");
>>>
>>>
>>> I think that it is better to
>>> - add some white-spaces
>>> or
>>> - add a new line
>>> between the argument string and the description string, because it is a little
>>> difficult for the help of register-characteristic to read as below:
>>>
>>> [bluetooth]# help
>>> Available commands:
>>> ...
>>> unregister-service <UUID/object> Unregister application service
>>> register-characteristic <UUID> <Flags=read,write,notify...> Register application characteristic
>>> unregister-characteristic <UUID/object> Unregister application characteristic
>>> register-descriptor <UUID> <Flags=read,write...> Register application descriptor
>>
>> It should probably have the same formatting as bluetoothctl:
>>
>> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/client/main.c#n2677
>>
>> --
>> Luiz Augusto von Dentz
>
> Yes, I'll correct it

Are you still working on this?

--
Luiz Augusto von Dentz

2017-11-06 13:59:00

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/4] shared/bt_shell: Add initial implementation

Hi Marcel,

On Mon, Nov 6, 2017 at 3:52 PM, Marcel Holtmann <[email protected]> wrote:
> Hi Luiz,
>
>>>>>>>> +#ifdef HAVE_CONFIG_H
>>>>>>>> +#include <config.h>
>>>>>>>> +#endif
>>>>>>>> +
>>>>>>>> +#include <stdio.h>
>>>>>>>> +#include "src/shared/util.h"
>>>>>>>> +#include "src/shared/queue.h"
>>>>>>>> +#include "src/shared/bt_shell.h"
>>>>>>>> +
>>>>>>>> +#define CMD_LENGTH 48
>>>>>>>> +
>>>>>>>> +static struct {
>>>>>>>> + const struct bt_shell_menu_entry *current;
>>>>>>>> +} bt_shell_data;
>>>>>>>> +
>>>>>>>> +bool bt_shell_init(const struct bt_shell_menu_entry *menu)
>>>>>>>> +{
>>>>>>>> + if (bt_shell_data.current || !menu)
>>>>>>>> + return false;
>>>>>>>> +
>>>>>>>> + bt_shell_data.current = menu;
>>>>>>>> +
>>>>>>>> + return true;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +void bt_shell_cleanup(void)
>>>>>>>> +{
>>>>>>>> + bt_shell_data.current = NULL;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +void bt_shell_process(const char *cmd, const char *arg)
>>>>>>>> +{
>>>>>>>> + const struct bt_shell_menu_entry *entry;
>>>>>>>> +
>>>>>>>> + if (!bt_shell_data.current || !cmd)
>>>>>>>> + return;
>>>>>>>> +
>>>>>>>> + for (entry = bt_shell_data.current; entry->cmd;
>>>>>>>> entry++) {
>>>>>>>> + if (strcmp(cmd, entry->cmd))
>>>>>>>> + continue;
>>>>>>>> +
>>>>>>>> + if (entry->func) {
>>>>>>>> + entry->func(arg);
>>>>>>>> + return;
>>>>>>>> + }
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + if (strcmp(cmd, "help")) {
>>>>>>>> + printf("Invalid command\n");
>>>>>>>> + return;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> + bt_shell_print_menu();
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +void bt_shell_print_menu(void)
>>>>>>>> +{
>>>>>>>> + const struct bt_shell_menu_entry *entry;
>>>>>>>> +
>>>>>>>> + if (!bt_shell_data.current)
>>>>>>>> + return;
>>>>>>>> +
>>>>>>>> + printf("Available commands:\n");
>>>>>>>> + for (entry = bt_shell_data.current; entry->cmd;
>>>>>>>> entry++) {
>>>>>>>> + printf(" %s %-*s %s\n", entry->cmd,
>>>>>>>> + (int)(CMD_LENGTH -
>>>>>>>> strlen(entry->cmd)),
>>>>>>>> + entry->arg ? : "",
>>>>>>>> entry->desc ? : "");
>>>>>>>
>>>>>>>
>>>>>>> I think that it is better to
>>>>>>> - add some white-spaces
>>>>>>> or
>>>>>>> - add a new line
>>>>>>> between the argument string and the description string, because
>>>>>>> it is a little
>>>>>>> difficult for the help of register-characteristic to read as
>>>>>>> below:
>>>>>>>
>>>>>>> [bluetooth]# help
>>>>>>> Available commands:
>>>>>>> ...
>>>>>>> unregister-service
>>>>>>> <UUID/object> Unregister application service
>>>>>>> register-characteristic <UUID> <Flags=read,write,notify...>
>>>>>>> Register application characteristic
>>>>>>> unregister-characteristic
>>>>>>> <UUID/object> Unregister application characteristic
>>>>>>> register-descriptor <UUID> <Flags=read,write...> Register
>>>>>>> application descriptor
>>>>>>
>>>>>> It should probably have the same formatting as bluetoothctl:
>>>>>>
>>>>>> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/client/ma
>>>>>> in.c#n2677
>>>>>>
>>>>>> --
>>>>>> Luiz Augusto von Dentz
>>>>>
>>>>> Yes, I'll correct it
>>>>
>>>> Are you still working on this?
>>>>
>>> No, unfortunatelly I don't have enough time to finish it.
>>
>> Ok, so I will start working on it myself.
>
> just a side note, lets do src/shared/shell.[ch] and not start with bt_ prefixes in file names.

Yep, Ive just did that. Ive also started replacing GChannel usage to
struct io, etc, and moved the whole rl_handler into the shell so it
will automatically setup the readline internals so in the future all
we have to do to replace readline is to rework the internals of
bt_shell.

> Regards
>
> Marcel
>



--
Luiz Augusto von Dentz

2017-11-06 13:52:29

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 1/4] shared/bt_shell: Add initial implementation

Hi Luiz,

>>>>>>> +#ifdef HAVE_CONFIG_H
>>>>>>> +#include <config.h>
>>>>>>> +#endif
>>>>>>> +
>>>>>>> +#include <stdio.h>
>>>>>>> +#include "src/shared/util.h"
>>>>>>> +#include "src/shared/queue.h"
>>>>>>> +#include "src/shared/bt_shell.h"
>>>>>>> +
>>>>>>> +#define CMD_LENGTH 48
>>>>>>> +
>>>>>>> +static struct {
>>>>>>> + const struct bt_shell_menu_entry *current;
>>>>>>> +} bt_shell_data;
>>>>>>> +
>>>>>>> +bool bt_shell_init(const struct bt_shell_menu_entry *menu)
>>>>>>> +{
>>>>>>> + if (bt_shell_data.current || !menu)
>>>>>>> + return false;
>>>>>>> +
>>>>>>> + bt_shell_data.current = menu;
>>>>>>> +
>>>>>>> + return true;
>>>>>>> +}
>>>>>>> +
>>>>>>> +void bt_shell_cleanup(void)
>>>>>>> +{
>>>>>>> + bt_shell_data.current = NULL;
>>>>>>> +}
>>>>>>> +
>>>>>>> +void bt_shell_process(const char *cmd, const char *arg)
>>>>>>> +{
>>>>>>> + const struct bt_shell_menu_entry *entry;
>>>>>>> +
>>>>>>> + if (!bt_shell_data.current || !cmd)
>>>>>>> + return;
>>>>>>> +
>>>>>>> + for (entry = bt_shell_data.current; entry->cmd;
>>>>>>> entry++) {
>>>>>>> + if (strcmp(cmd, entry->cmd))
>>>>>>> + continue;
>>>>>>> +
>>>>>>> + if (entry->func) {
>>>>>>> + entry->func(arg);
>>>>>>> + return;
>>>>>>> + }
>>>>>>> + }
>>>>>>> +
>>>>>>> + if (strcmp(cmd, "help")) {
>>>>>>> + printf("Invalid command\n");
>>>>>>> + return;
>>>>>>> + }
>>>>>>> +
>>>>>>> + bt_shell_print_menu();
>>>>>>> +}
>>>>>>> +
>>>>>>> +void bt_shell_print_menu(void)
>>>>>>> +{
>>>>>>> + const struct bt_shell_menu_entry *entry;
>>>>>>> +
>>>>>>> + if (!bt_shell_data.current)
>>>>>>> + return;
>>>>>>> +
>>>>>>> + printf("Available commands:\n");
>>>>>>> + for (entry = bt_shell_data.current; entry->cmd;
>>>>>>> entry++) {
>>>>>>> + printf(" %s %-*s %s\n", entry->cmd,
>>>>>>> + (int)(CMD_LENGTH -
>>>>>>> strlen(entry->cmd)),
>>>>>>> + entry->arg ? : "",
>>>>>>> entry->desc ? : "");
>>>>>>
>>>>>>
>>>>>> I think that it is better to
>>>>>> - add some white-spaces
>>>>>> or
>>>>>> - add a new line
>>>>>> between the argument string and the description string, because
>>>>>> it is a little
>>>>>> difficult for the help of register-characteristic to read as
>>>>>> below:
>>>>>>
>>>>>> [bluetooth]# help
>>>>>> Available commands:
>>>>>> ...
>>>>>> unregister-service
>>>>>> <UUID/object> Unregister application service
>>>>>> register-characteristic <UUID> <Flags=read,write,notify...>
>>>>>> Register application characteristic
>>>>>> unregister-characteristic
>>>>>> <UUID/object> Unregister application characteristic
>>>>>> register-descriptor <UUID> <Flags=read,write...> Register
>>>>>> application descriptor
>>>>>
>>>>> It should probably have the same formatting as bluetoothctl:
>>>>>
>>>>> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/client/ma
>>>>> in.c#n2677
>>>>>
>>>>> --
>>>>> Luiz Augusto von Dentz
>>>>
>>>> Yes, I'll correct it
>>>
>>> Are you still working on this?
>>>
>> No, unfortunatelly I don't have enough time to finish it.
>
> Ok, so I will start working on it myself.

just a side note, lets do src/shared/shell.[ch] and not start with bt_ prefixes in file names.

Regards

Marcel


2017-11-06 13:24:08

by Marcin Kraglak

[permalink] [raw]
Subject: Re: [PATCH 1/4] shared/bt_shell: Add initial implementation

Hi Luiz,

On Mon, 2017-10-23 at 14:42 +0300, Luiz Augusto von Dentz wrote:
> Hi Marcin,
>
> On Fri, Sep 22, 2017 at 10:52 AM, Marcin Kraglak
> <[email protected]> wrote:
> > Hi Luiz Eramoto,
> >
> > On 22 September 2017 at 03:42, Luiz Augusto von Dentz
> > <[email protected]> wrote:
> > > Hi Eramoto, Marcin,
> > >
> > > On Fri, Sep 22, 2017 at 9:27 AM, ERAMOTO Masaya
> > > <[email protected]> wrote:
> > > > Hi Marcin,
> > > >
> > > > > +#ifdef HAVE_CONFIG_H
> > > > > +#include <config.h>
> > > > > +#endif
> > > > > +
> > > > > +#include <stdio.h>
> > > > > +#include "src/shared/util.h"
> > > > > +#include "src/shared/queue.h"
> > > > > +#include "src/shared/bt_shell.h"
> > > > > +
> > > > > +#define CMD_LENGTH 48
> > > > > +
> > > > > +static struct {
> > > > > + const struct bt_shell_menu_entry *current;
> > > > > +} bt_shell_data;
> > > > > +
> > > > > +bool bt_shell_init(const struct bt_shell_menu_entry *menu)
> > > > > +{
> > > > > + if (bt_shell_data.current || !menu)
> > > > > + return false;
> > > > > +
> > > > > + bt_shell_data.current = menu;
> > > > > +
> > > > > + return true;
> > > > > +}
> > > > > +
> > > > > +void bt_shell_cleanup(void)
> > > > > +{
> > > > > + bt_shell_data.current = NULL;
> > > > > +}
> > > > > +
> > > > > +void bt_shell_process(const char *cmd, const char *arg)
> > > > > +{
> > > > > + const struct bt_shell_menu_entry *entry;
> > > > > +
> > > > > + if (!bt_shell_data.current || !cmd)
> > > > > + return;
> > > > > +
> > > > > + for (entry = bt_shell_data.current; entry->cmd;
> > > > > entry++) {
> > > > > + if (strcmp(cmd, entry->cmd))
> > > > > + continue;
> > > > > +
> > > > > + if (entry->func) {
> > > > > + entry->func(arg);
> > > > > + return;
> > > > > + }
> > > > > + }
> > > > > +
> > > > > + if (strcmp(cmd, "help")) {
> > > > > + printf("Invalid command\n");
> > > > > + return;
> > > > > + }
> > > > > +
> > > > > + bt_shell_print_menu();
> > > > > +}
> > > > > +
> > > > > +void bt_shell_print_menu(void)
> > > > > +{
> > > > > + const struct bt_shell_menu_entry *entry;
> > > > > +
> > > > > + if (!bt_shell_data.current)
> > > > > + return;
> > > > > +
> > > > > + printf("Available commands:\n");
> > > > > + for (entry = bt_shell_data.current; entry->cmd;
> > > > > entry++) {
> > > > > + printf(" %s %-*s %s\n", entry->cmd,
> > > > > + (int)(CMD_LENGTH -
> > > > > strlen(entry->cmd)),
> > > > > + entry->arg ? : "",
> > > > > entry->desc ? : "");
> > > >
> > > >
> > > > I think that it is better to
> > > > - add some white-spaces
> > > > or
> > > > - add a new line
> > > > between the argument string and the description string, because
> > > > it is a little
> > > > difficult for the help of register-characteristic to read as
> > > > below:
> > > >
> > > > [bluetooth]# help
> > > > Available commands:
> > > > ...
> > > > unregister-service
> > > > <UUID/object> Unregister application service
> > > > register-characteristic <UUID> <Flags=read,write,notify...>
> > > > Register application characteristic
> > > > unregister-characteristic
> > > > <UUID/object> Unregister application characteristic
> > > > register-descriptor <UUID> <Flags=read,write...> Register
> > > > application descriptor
> > >
> > > It should probably have the same formatting as bluetoothctl:
> > >
> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/client/ma
> > > in.c#n2677
> > >
> > > --
> > > Luiz Augusto von Dentz
> >
> > Yes, I'll correct it
>
> Are you still working on this?
>
No, unfortunatelly I don't have enough time to finish it.

BR
Marcin

2017-11-06 13:25:34

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 1/4] shared/bt_shell: Add initial implementation

Hi Marcin,

On Mon, Nov 6, 2017 at 3:24 PM, Marcin Kraglak <[email protected]> wrote:
> Hi Luiz,
>
> On Mon, 2017-10-23 at 14:42 +0300, Luiz Augusto von Dentz wrote:
>> Hi Marcin,
>>
>> On Fri, Sep 22, 2017 at 10:52 AM, Marcin Kraglak
>> <[email protected]> wrote:
>> > Hi Luiz Eramoto,
>> >
>> > On 22 September 2017 at 03:42, Luiz Augusto von Dentz
>> > <[email protected]> wrote:
>> > > Hi Eramoto, Marcin,
>> > >
>> > > On Fri, Sep 22, 2017 at 9:27 AM, ERAMOTO Masaya
>> > > <[email protected]> wrote:
>> > > > Hi Marcin,
>> > > >
>> > > > > +#ifdef HAVE_CONFIG_H
>> > > > > +#include <config.h>
>> > > > > +#endif
>> > > > > +
>> > > > > +#include <stdio.h>
>> > > > > +#include "src/shared/util.h"
>> > > > > +#include "src/shared/queue.h"
>> > > > > +#include "src/shared/bt_shell.h"
>> > > > > +
>> > > > > +#define CMD_LENGTH 48
>> > > > > +
>> > > > > +static struct {
>> > > > > + const struct bt_shell_menu_entry *current;
>> > > > > +} bt_shell_data;
>> > > > > +
>> > > > > +bool bt_shell_init(const struct bt_shell_menu_entry *menu)
>> > > > > +{
>> > > > > + if (bt_shell_data.current || !menu)
>> > > > > + return false;
>> > > > > +
>> > > > > + bt_shell_data.current = menu;
>> > > > > +
>> > > > > + return true;
>> > > > > +}
>> > > > > +
>> > > > > +void bt_shell_cleanup(void)
>> > > > > +{
>> > > > > + bt_shell_data.current = NULL;
>> > > > > +}
>> > > > > +
>> > > > > +void bt_shell_process(const char *cmd, const char *arg)
>> > > > > +{
>> > > > > + const struct bt_shell_menu_entry *entry;
>> > > > > +
>> > > > > + if (!bt_shell_data.current || !cmd)
>> > > > > + return;
>> > > > > +
>> > > > > + for (entry = bt_shell_data.current; entry->cmd;
>> > > > > entry++) {
>> > > > > + if (strcmp(cmd, entry->cmd))
>> > > > > + continue;
>> > > > > +
>> > > > > + if (entry->func) {
>> > > > > + entry->func(arg);
>> > > > > + return;
>> > > > > + }
>> > > > > + }
>> > > > > +
>> > > > > + if (strcmp(cmd, "help")) {
>> > > > > + printf("Invalid command\n");
>> > > > > + return;
>> > > > > + }
>> > > > > +
>> > > > > + bt_shell_print_menu();
>> > > > > +}
>> > > > > +
>> > > > > +void bt_shell_print_menu(void)
>> > > > > +{
>> > > > > + const struct bt_shell_menu_entry *entry;
>> > > > > +
>> > > > > + if (!bt_shell_data.current)
>> > > > > + return;
>> > > > > +
>> > > > > + printf("Available commands:\n");
>> > > > > + for (entry = bt_shell_data.current; entry->cmd;
>> > > > > entry++) {
>> > > > > + printf(" %s %-*s %s\n", entry->cmd,
>> > > > > + (int)(CMD_LENGTH -
>> > > > > strlen(entry->cmd)),
>> > > > > + entry->arg ? : "",
>> > > > > entry->desc ? : "");
>> > > >
>> > > >
>> > > > I think that it is better to
>> > > > - add some white-spaces
>> > > > or
>> > > > - add a new line
>> > > > between the argument string and the description string, because
>> > > > it is a little
>> > > > difficult for the help of register-characteristic to read as
>> > > > below:
>> > > >
>> > > > [bluetooth]# help
>> > > > Available commands:
>> > > > ...
>> > > > unregister-service
>> > > > <UUID/object> Unregister application service
>> > > > register-characteristic <UUID> <Flags=read,write,notify...>
>> > > > Register application characteristic
>> > > > unregister-characteristic
>> > > > <UUID/object> Unregister application characteristic
>> > > > register-descriptor <UUID> <Flags=read,write...> Register
>> > > > application descriptor
>> > >
>> > > It should probably have the same formatting as bluetoothctl:
>> > >
>> > > https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/client/ma
>> > > in.c#n2677
>> > >
>> > > --
>> > > Luiz Augusto von Dentz
>> >
>> > Yes, I'll correct it
>>
>> Are you still working on this?
>>
> No, unfortunatelly I don't have enough time to finish it.

Ok, so I will start working on it myself.

--
Luiz Augusto von Dentz

2017-11-06 12:34:55

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH 0/4] Move common client code to bt_shell

Hi Marcin,

On Fri, Sep 22, 2017 at 7:59 AM, Marcin Kraglak
<[email protected]> wrote:
> This is first part of moving common code of client and meshctl to shared
> directory. Next steps would be:
> - functions for switching between menus (needed for meshctl)
> - display functions
> - common support for history
>
>
> Marcin Kraglak (4):
> shared/bt_shell: Add initial implementation
> client: Use bt_shell to process commands.
> shared/bt_shell: Add bt_shell_completion
> client: Use bt_shell_completion
>
> Makefile.tools | 3 +-
> client/main.c | 93 +++---------------------------
> src/shared/bt_shell.c | 152 ++++++++++++++++++++++++++++++++++++++++++++++++++
> src/shared/bt_shell.h | 43 ++++++++++++++
> 4 files changed, 204 insertions(+), 87 deletions(-)
> create mode 100644 src/shared/bt_shell.c
> create mode 100644 src/shared/bt_shell.h
>
> --
> 2.13.5

Would you be able to continue working on this?

--
Luiz Augusto von Dentz