Hi,
This is the same patch set I sent last week, but with the changes based on the
comments you made here. I think I addressed them all. Let me know if you have
other comments.
Thanks!
Alex Deymo (6):
client: Add color modifiers to NEW, CHG and DEL events.
client: Right prompt management on agent input
client: "agent" command capability argument and autocompletion
client: Agent's DisplayPincode implementation
client: Agent's DisplayPasskey implementation
client: Agent's RequestPasskey implementation
client/agent.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
client/display.h | 7 ++-
client/main.c | 106 +++++++++++++++++++++++++++++++++++------
3 files changed, 229 insertions(+), 27 deletions(-)
--
1.8.1.3
Hi Alex,
On 13:55 Tue 19 Mar, Alex Deymo wrote:
> Registering an agent shares the user input interface with the normal console
> command interface. The way it is implemented (using rl_message, rl_save_prompt
> and rl_restore_prompt) conflicts with the rl_printf calls that may appear
> while waiting for user input, loosing the [bluetooth]# prompt.
> This patch fixes this and makes clear if the expected input is a command or an
> agent reply changing the color and text of the prompt.
Seems that there are some minor issues with formatting:
[bluetooth]# pair 40:98:4E:XX:XX:XX
Attempting to pair with 40:98:4E:XX:XX:XX
[CHG] Device 40:98:4E:XX:XX:XX Connected: yes
Request confirmation
[blue1m[agent] Confirm passkey 368383 (yes/no): yes
And I would suggest adding a patch to make bluetoothctl register itself as a
default agent by default. I needed to enable it manually before it could
handle incomming pairing requests.
Cheers,
--
Vinicius
Implements the uint32 RequestPasskey(object device) method.
---
client/agent.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/client/agent.c b/client/agent.c
index 00754df..29769fd 100644
--- a/client/agent.c
+++ b/client/agent.c
@@ -94,6 +94,20 @@ static void pincode_response(DBusConnection *conn, const char *input)
DBUS_TYPE_INVALID);
}
+static void passkey_response(DBusConnection *conn, const char *input)
+{
+ dbus_uint32_t passkey;
+ if (sscanf(input, "%u", &passkey) == 1)
+ g_dbus_send_reply(conn, pending_message, DBUS_TYPE_UINT32,
+ &passkey, DBUS_TYPE_INVALID);
+ else if (!strcmp(input, "no"))
+ g_dbus_send_error(conn, pending_message,
+ "org.bluez.Error.Rejected", NULL);
+ else
+ g_dbus_send_error(conn, pending_message,
+ "org.bluez.Error.Canceled", NULL);
+}
+
static void confirm_response(DBusConnection *conn, const char *input)
{
if (!strcmp(input, "yes"))
@@ -119,6 +133,8 @@ dbus_bool_t agent_input(DBusConnection *conn, const char *input)
if (!strcmp(member, "RequestPinCode"))
pincode_response(conn, input);
+ else if (!strcmp(member, "RequestPasskey"))
+ passkey_response(conn, input);
else if (!strcmp(member, "RequestConfirmation"))
confirm_response(conn, input);
else if (!strcmp(member, "RequestAuthorization"))
@@ -186,6 +202,23 @@ static DBusMessage *display_pincode(DBusConnection *conn,
return dbus_message_new_method_return(msg);
}
+static DBusMessage *request_passkey(DBusConnection *conn,
+ DBusMessage *msg, void *user_data)
+{
+ const char *device;
+
+ rl_printf("Request passkey\n");
+
+ dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &device,
+ DBUS_TYPE_INVALID);
+
+ agent_prompt("Enter passkey (number in 0-999999): ");
+
+ pending_message = dbus_message_ref(msg);
+
+ return NULL;
+}
+
static DBusMessage *display_passkey(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
@@ -289,6 +322,9 @@ static const GDBusMethodTable methods[] = {
{ GDBUS_METHOD("DisplayPinCode",
GDBUS_ARGS({ "device", "o" }, { "pincode", "s" }),
NULL, display_pincode) },
+ { GDBUS_ASYNC_METHOD("RequestPasskey",
+ GDBUS_ARGS({ "device", "o" }),
+ GDBUS_ARGS({ "passkey", "u" }), request_passkey) },
{ GDBUS_METHOD("DisplayPasskey",
GDBUS_ARGS({ "device", "o" }, { "passkey", "u" },
{ "entered", "q" }),
--
1.8.1.3
Implements the DisplayPasskey(object device, uint32 passkey, uint16 entered)
method.
---
client/agent.c | 29 +++++++++++++++++++++++++++++
client/display.h | 2 ++
2 files changed, 31 insertions(+)
diff --git a/client/agent.c b/client/agent.c
index 1414eba..00754df 100644
--- a/client/agent.c
+++ b/client/agent.c
@@ -186,6 +186,31 @@ static DBusMessage *display_pincode(DBusConnection *conn,
return dbus_message_new_method_return(msg);
}
+static DBusMessage *display_passkey(DBusConnection *conn,
+ DBusMessage *msg, void *user_data)
+{
+ const char *device;
+ dbus_uint32_t passkey;
+ dbus_uint16_t entered;
+ char passkey_full[7];
+
+ dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &device,
+ DBUS_TYPE_UINT32, &passkey, DBUS_TYPE_UINT16, &entered,
+ DBUS_TYPE_INVALID);
+
+ snprintf(passkey_full, sizeof(passkey_full), "%.6u", passkey);
+ passkey_full[6] = '\0';
+
+ if (entered > strlen(passkey_full))
+ entered = strlen(passkey_full);
+
+ rl_printf(AGENT_PROMPT "Passkey: "
+ COLOR_BOLDGRAY "%.*s" COLOR_BOLDWHITE "%s\n" COLOR_OFF,
+ entered, passkey_full, passkey_full + entered);
+
+ return dbus_message_new_method_return(msg);
+}
+
static DBusMessage *request_confirmation(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
@@ -264,6 +289,10 @@ static const GDBusMethodTable methods[] = {
{ GDBUS_METHOD("DisplayPinCode",
GDBUS_ARGS({ "device", "o" }, { "pincode", "s" }),
NULL, display_pincode) },
+ { GDBUS_METHOD("DisplayPasskey",
+ GDBUS_ARGS({ "device", "o" }, { "passkey", "u" },
+ { "entered", "q" }),
+ NULL, display_passkey) },
{ GDBUS_ASYNC_METHOD("RequestConfirmation",
GDBUS_ARGS({ "device", "o" }, { "passkey", "u" }),
NULL, request_confirmation) },
diff --git a/client/display.h b/client/display.h
index 957bdc6..91a0be9 100644
--- a/client/display.h
+++ b/client/display.h
@@ -26,5 +26,7 @@
#define COLOR_GREEN "\x1B[0;92m"
#define COLOR_YELLOW "\x1B[0;93m"
#define COLOR_BLUE "\x1B[0;94m"
+#define COLOR_BOLDGRAY "\x1B[1;30m"
+#define COLOR_BOLDWHITE "\x1B[1;37m"
void rl_printf(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
--
1.8.1.3
Implements the DisplayPinCode(object device, string pincode) method.
---
client/agent.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/client/agent.c b/client/agent.c
index dc27079..1414eba 100644
--- a/client/agent.c
+++ b/client/agent.c
@@ -172,6 +172,20 @@ static DBusMessage *request_pincode(DBusConnection *conn,
return NULL;
}
+static DBusMessage *display_pincode(DBusConnection *conn,
+ DBusMessage *msg, void *user_data)
+{
+ const char *device;
+ const char *pincode;
+
+ dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &device,
+ DBUS_TYPE_STRING, &pincode, DBUS_TYPE_INVALID);
+
+ rl_printf(AGENT_PROMPT "PIN code: %s\n", pincode);
+
+ return dbus_message_new_method_return(msg);
+}
+
static DBusMessage *request_confirmation(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
@@ -247,6 +261,9 @@ static const GDBusMethodTable methods[] = {
{ GDBUS_ASYNC_METHOD("RequestPinCode",
GDBUS_ARGS({ "device", "o" }),
GDBUS_ARGS({ "pincode", "s" }), request_pincode) },
+ { GDBUS_METHOD("DisplayPinCode",
+ GDBUS_ARGS({ "device", "o" }, { "pincode", "s" }),
+ NULL, display_pincode) },
{ GDBUS_ASYNC_METHOD("RequestConfirmation",
GDBUS_ARGS({ "device", "o" }, { "passkey", "u" }),
NULL, request_confirmation) },
--
1.8.1.3
This patch enables argument autocompletion for the agent command with the
list of capabilities an agent can have, adding also "on" (for the default "")
and "off". The command passes the argument (parsing and verifying it) to the
dbus method call.
---
client/main.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 70 insertions(+), 3 deletions(-)
diff --git a/client/main.c b/client/main.c
index 704cf46..30d7aa2 100644
--- a/client/main.c
+++ b/client/main.c
@@ -59,6 +59,17 @@ static GDBusProxy *default_ctrl;
static GList *ctrl_list;
static GList *dev_list;
+static const char* agent_arguments[] = {
+ "on",
+ "off",
+ "DisplayOnly",
+ "DisplayYesNo",
+ "KeyboardDisplay",
+ "KeyboardOnly",
+ "NoInputNoOutput",
+ NULL
+};
+
static void proxy_leak(gpointer data)
{
printf("Leaking proxy %p\n", data);
@@ -413,6 +424,39 @@ static gboolean parse_argument_on_off(const char *arg, dbus_bool_t *value)
return FALSE;
}
+static gboolean parse_argument_agent(const char *arg, dbus_bool_t *value,
+ const char **capability)
+{
+ const char **opt;
+
+ if (arg == NULL || strlen(arg) == 0) {
+ rl_printf("Missing on/off/capability argument\n");
+ return FALSE;
+ }
+
+ if (strcmp(arg, "on") == 0 || strcmp(arg, "yes") == 0) {
+ *value = TRUE;
+ *capability = "";
+ return TRUE;
+ }
+
+ if (strcmp(arg, "off") == 0 || strcmp(arg, "no") == 0) {
+ *value = FALSE;
+ return TRUE;
+ }
+
+ for (opt = agent_arguments; *opt; opt++) {
+ if (strcmp(arg, *opt) == 0) {
+ *value = TRUE;
+ *capability = *opt;
+ return TRUE;
+ }
+ }
+
+ rl_printf("Invalid argument %s\n", arg);
+ return FALSE;
+}
+
static void cmd_list(const char *arg)
{
GList *list;
@@ -610,13 +654,14 @@ static void cmd_discoverable(const char *arg)
static void cmd_agent(const char *arg)
{
dbus_bool_t enable;
+ const char* capability;
- if (parse_argument_on_off(arg, &enable) == FALSE)
+ if (parse_argument_agent(arg, &enable, &capability) == FALSE)
return;
if (enable == TRUE) {
g_free(auto_register_agent);
- auto_register_agent = g_strdup("");
+ auto_register_agent = g_strdup(capability);
if (agent_manager)
agent_register(dbus_conn, agent_manager,
@@ -968,6 +1013,26 @@ static char *dev_generator(const char *text, int state)
return generic_generator(text, state, dev_list, "Address");
}
+static char *capability_generator(const char *text, int state)
+{
+ static int index, len;
+ const char *arg;
+
+ if (!state) {
+ index = 0;
+ len = strlen(text);
+ }
+
+ while ((arg = agent_arguments[index])) {
+ index++;
+
+ if (!strncmp(arg, text, len))
+ return strdup(arg);
+ }
+
+ return NULL;
+}
+
static const struct {
const char *cmd;
const char *arg;
@@ -989,7 +1054,9 @@ static const struct {
"Set controller pairable mode" },
{ "discoverable", "<on/off>", cmd_discoverable,
"Set controller discoverable mode" },
- { "agent", "<on/off>", cmd_agent, "Enable/disable agent" },
+ { "agent", "<on/off/capability>", cmd_agent,
+ "Enable/disable agent with given capability",
+ capability_generator},
{ "default-agent",NULL, cmd_default_agent },
{ "scan", "<on/off>", cmd_scan, "Scan for devices" },
{ "info", "<dev>", cmd_info, "Device information",
--
1.8.1.3
Registering an agent shares the user input interface with the normal console
command interface. The way it is implemented (using rl_message, rl_save_prompt
and rl_restore_prompt) conflicts with the rl_printf calls that may appear
while waiting for user input, loosing the [bluetooth]# prompt.
This patch fixes this and makes clear if the expected input is a command or an
agent reply changing the color and text of the prompt.
---
client/agent.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++----------
client/display.h | 2 +-
client/main.c | 13 +++++++-----
3 files changed, 59 insertions(+), 17 deletions(-)
diff --git a/client/agent.c b/client/agent.c
index b0ac2f8..dc27079 100644
--- a/client/agent.c
+++ b/client/agent.c
@@ -26,6 +26,7 @@
#endif
#include <stdio.h>
+#include <stdlib.h>
#include <readline/readline.h>
#include <gdbus.h>
@@ -35,9 +36,49 @@
#define AGENT_PATH "/org/bluez/agent"
#define AGENT_INTERFACE "org.bluez.Agent1"
+#define AGENT_PROMPT COLOR_RED "[agent]" COLOR_OFF " "
+
static gboolean agent_registered = FALSE;
static const char *agent_capability = NULL;
static DBusMessage *pending_message = NULL;
+static char *agent_saved_prompt = NULL;
+static int agent_saved_point = 0;
+
+static void agent_prompt(const char* msg)
+{
+ char *prompt;
+
+ /* Normal use should not prompt for user input to the agent a second
+ * time before it releases the prompt, but we take a safe action. */
+ if (agent_saved_prompt)
+ return;
+
+ agent_saved_point = rl_point;
+ agent_saved_prompt = g_strdup(rl_prompt);
+
+ prompt = g_strdup_printf(AGENT_PROMPT "%s", msg);
+ rl_set_prompt(prompt);
+ g_free(prompt);
+
+ rl_replace_line("", 0);
+ rl_redisplay();
+}
+
+static void agent_release_prompt(void)
+{
+ if (!agent_saved_prompt)
+ return;
+
+ /* This will cause rl_expand_prompt to re-run over the last prompt, but
+ * our prompt doesn't expand anyway. */
+ rl_set_prompt(agent_saved_prompt);
+ rl_replace_line("", 0);
+ rl_point = agent_saved_point;
+ rl_redisplay();
+
+ g_free(agent_saved_prompt);
+ agent_saved_prompt = NULL;
+}
dbus_bool_t agent_completion(void)
{
@@ -69,11 +110,11 @@ dbus_bool_t agent_input(DBusConnection *conn, const char *input)
{
const char *member;
- rl_clear_message();
-
if (!pending_message)
return FALSE;
+ agent_release_prompt();
+
member = dbus_message_get_member(pending_message);
if (!strcmp(member, "RequestPinCode"))
@@ -97,9 +138,6 @@ dbus_bool_t agent_input(DBusConnection *conn, const char *input)
static DBusMessage *release_agent(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
- if (pending_message)
- rl_clear_message();
-
agent_registered = FALSE;
agent_capability = NULL;
@@ -110,6 +148,8 @@ static DBusMessage *release_agent(DBusConnection *conn,
pending_message = NULL;
}
+ agent_release_prompt();
+
g_dbus_unregister_interface(conn, AGENT_PATH, AGENT_INTERFACE);
return dbus_message_new_method_return(msg);
@@ -125,7 +165,7 @@ static DBusMessage *request_pincode(DBusConnection *conn,
dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &device,
DBUS_TYPE_INVALID);
- rl_message("Enter PIN code: ");
+ agent_prompt("Enter PIN code: ");
pending_message = dbus_message_ref(msg);
@@ -145,7 +185,7 @@ static DBusMessage *request_confirmation(DBusConnection *conn,
DBUS_TYPE_UINT32, &passkey, DBUS_TYPE_INVALID);
str = g_strdup_printf("Confirm passkey %06u (yes/no): ", passkey);
- rl_message(str);
+ agent_prompt(str);
g_free(str);
pending_message = dbus_message_ref(msg);
@@ -163,7 +203,7 @@ static DBusMessage *request_authorization(DBusConnection *conn,
dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &device,
DBUS_TYPE_INVALID);
- rl_message("Accept pairing (yes/no): ");
+ agent_prompt("Accept pairing (yes/no): ");
pending_message = dbus_message_ref(msg);
@@ -182,7 +222,7 @@ static DBusMessage *authorize_service(DBusConnection *conn,
DBUS_TYPE_STRING, &uuid, DBUS_TYPE_INVALID);
str = g_strdup_printf("Authorize service %s (yes/no): ", uuid);
- rl_message(str);
+ agent_prompt(str);
g_free(str);
pending_message = dbus_message_ref(msg);
@@ -193,10 +233,9 @@ static DBusMessage *authorize_service(DBusConnection *conn,
static DBusMessage *cancel_request(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
- rl_clear_message();
-
rl_printf("Request canceled\n");
+ agent_release_prompt();
dbus_message_unref(pending_message);
pending_message = NULL;
diff --git a/client/display.h b/client/display.h
index 9cb891a..957bdc6 100644
--- a/client/display.h
+++ b/client/display.h
@@ -25,6 +25,6 @@
#define COLOR_RED "\x1B[0;91m"
#define COLOR_GREEN "\x1B[0;92m"
#define COLOR_YELLOW "\x1B[0;93m"
-#define COLOR_BLUE "\x1B[0;34m"
+#define COLOR_BLUE "\x1B[0;94m"
void rl_printf(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
diff --git a/client/main.c b/client/main.c
index d8547c0..704cf46 100644
--- a/client/main.c
+++ b/client/main.c
@@ -46,6 +46,9 @@
#define COLORED_CHG COLOR_YELLOW "CHG" COLOR_OFF
#define COLORED_DEL COLOR_RED "DEL" COLOR_OFF
+#define PROMPT_ON COLOR_BLUE "[bluetooth]" COLOR_OFF "# "
+#define PROMPT_OFF "[bluetooth]# "
+
static GMainLoop *main_loop;
static DBusConnection *dbus_conn;
@@ -63,7 +66,7 @@ static void proxy_leak(gpointer data)
static void connect_handler(DBusConnection *connection, void *user_data)
{
- rl_set_prompt(COLOR_BLUE "[bluetooth]" COLOR_OFF "# ");
+ rl_set_prompt(PROMPT_ON);
printf("\r");
rl_on_new_line();
rl_redisplay();
@@ -71,7 +74,7 @@ static void connect_handler(DBusConnection *connection, void *user_data)
static void disconnect_handler(DBusConnection *connection, void *user_data)
{
- rl_set_prompt("[bluetooth]# ");
+ rl_set_prompt(PROMPT_OFF);
printf("\r");
rl_on_new_line();
rl_redisplay();
@@ -1283,11 +1286,11 @@ int main(int argc, char *argv[])
rl_erase_empty_line = 1;
rl_callback_handler_install(NULL, rl_handler);
- rl_set_prompt("[bluetooth]# ");
+ rl_set_prompt(PROMPT_OFF);
rl_redisplay();
- input = setup_standard_input();
- signal = setup_signalfd();
+ input = setup_standard_input();
+ signal = setup_signalfd();
client = g_dbus_client_new(dbus_conn, "org.bluez", "/org/bluez");
g_dbus_client_set_connect_watch(client, connect_handler, NULL);
--
1.8.1.3
Events like [NEW], [CHG] and [DEL] can appear in the command console at
any time, even when the user is typing a command. The last line is kept
making the event line appear just before it, something that can be
unnoticed. This patch add meaningful colors for those three event to make
it easier to see them.
---
client/display.h | 3 +++
client/main.c | 20 +++++++++++++-------
2 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/client/display.h b/client/display.h
index 393a3c8..9cb891a 100644
--- a/client/display.h
+++ b/client/display.h
@@ -22,6 +22,9 @@
*/
#define COLOR_OFF "\x1B[0m"
+#define COLOR_RED "\x1B[0;91m"
+#define COLOR_GREEN "\x1B[0;92m"
+#define COLOR_YELLOW "\x1B[0;93m"
#define COLOR_BLUE "\x1B[0;34m"
void rl_printf(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
diff --git a/client/main.c b/client/main.c
index 9a927a8..d8547c0 100644
--- a/client/main.c
+++ b/client/main.c
@@ -41,6 +41,11 @@
#include "agent.h"
#include "display.h"
+/* String display constants */
+#define COLORED_NEW COLOR_GREEN "NEW" COLOR_OFF
+#define COLORED_CHG COLOR_YELLOW "CHG" COLOR_OFF
+#define COLORED_DEL COLOR_RED "DEL" COLOR_OFF
+
static GMainLoop *main_loop;
static DBusConnection *dbus_conn;
@@ -252,7 +257,7 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
if (device_is_child(proxy, default_ctrl) == TRUE) {
dev_list = g_list_append(dev_list, proxy);
- print_device(proxy, "NEW");
+ print_device(proxy, COLORED_NEW);
}
} else if (!strcmp(interface, "org.bluez.Adapter1")) {
ctrl_list = g_list_append(ctrl_list, proxy);
@@ -260,7 +265,7 @@ static void proxy_added(GDBusProxy *proxy, void *user_data)
if (!default_ctrl)
default_ctrl = proxy;
- print_adapter(proxy, "NEW");
+ print_adapter(proxy, COLORED_NEW);
} else if (!strcmp(interface, "org.bluez.AgentManager1")) {
if (!agent_manager) {
agent_manager = proxy;
@@ -282,12 +287,12 @@ static void proxy_removed(GDBusProxy *proxy, void *user_data)
if (device_is_child(proxy, default_ctrl) == TRUE) {
dev_list = g_list_remove(dev_list, proxy);
- print_device(proxy, "DEL");
+ print_device(proxy, COLORED_DEL);
}
} else if (!strcmp(interface, "org.bluez.Adapter1")) {
ctrl_list = g_list_remove(ctrl_list, proxy);
- print_adapter(proxy, "DEL");
+ print_adapter(proxy, COLORED_DEL);
if (default_ctrl == proxy) {
default_ctrl = NULL;
@@ -319,8 +324,8 @@ static void property_changed(GDBusProxy *proxy, const char *name,
dbus_message_iter_get_basic(&addr_iter,
&address);
- str = g_strdup_printf("[CHG] Device %s ",
- address);
+ str = g_strdup_printf("[" COLORED_CHG
+ "] Device %s ", address);
} else
str = g_strdup("");
@@ -336,7 +341,8 @@ static void property_changed(GDBusProxy *proxy, const char *name,
const char *address;
dbus_message_iter_get_basic(&addr_iter, &address);
- str = g_strdup_printf("[CHG] Controller %s ", address);
+ str = g_strdup_printf("[" COLORED_CHG
+ "] Controller %s ", address);
} else
str = g_strdup("");
--
1.8.1.3