[PATCH] Get IEEE1284 for a single printer

[PATCH] Get IEEE1284 for a single printer

[Bastien Nocera]

[-- Attachment #1: Type: text/plain, Size: 714 bytes --]

Heya,

Discussed printer setup integration with Tim Waugh, and we came to the
idea that the current framework for USB printers could be reused for
Bluetooth ones.

The differences would be:
- add code to fetch a single printer's IEEE1284 device ID to the
Bluetooth cups backend (the attached patch)
- add special handling in udev-configure-printer for Bluetooth printers
(we get the IEEE1284 device ID from the cups backend helper)
https://fedorahosted.org/system-config-printer/ticket/207
- Add plugin to bluetoothd to call udev-configure-printer when printers
are paired, or unpaired (not written yet)

Let me know what you think. The patch below could still be useful for
debugging (in the worst case).

Cheers

[-- Attachment #2: 0001-cups-Add-ability-to-print-IEEE1284-device-ID.patch --]
[-- Type: text/x-patch, Size: 3209 bytes --]

>From da8e0589d687cb3f24eab46259d179fd830cd088 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Sun, 6 Jun 2010 15:48:26 +0100
Subject: [PATCH] (cups) Add ability to print IEEE1284 device ID

Add ability to print IEEE1284 device ID for Bluetooth
printers to allow auto-configuration once paired.
---
 cups/main.c |   78 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 78 insertions(+), 0 deletions(-)

diff --git a/cups/main.c b/cups/main.c
index 9659a11..00933b6 100644
--- a/cups/main.c
+++ b/cups/main.c
@@ -605,6 +605,75 @@ static gboolean list_printers(void)
 	return TRUE;
 }
 
+static gboolean print_ieee1284(const char *bdaddr)
+{
+	DBusMessage *message, *reply, *adapter_reply;
+	DBusMessageIter iter;
+	char *object_path = NULL;
+	char *adapter;
+	char *id;
+
+	adapter_reply = NULL;
+
+	conn = g_dbus_setup_bus(DBUS_BUS_SYSTEM, NULL, NULL);
+	if (conn == NULL)
+		return FALSE;
+
+	message = dbus_message_new_method_call("org.bluez", "/",
+					       "org.bluez.Manager",
+					       "DefaultAdapter");
+
+	adapter_reply = dbus_connection_send_with_reply_and_block(conn,
+								  message, -1, NULL);
+
+	dbus_message_unref(message);
+
+	if (dbus_message_get_args(adapter_reply, NULL, DBUS_TYPE_OBJECT_PATH, &adapter, DBUS_TYPE_INVALID) == FALSE)
+		return FALSE;
+
+	message = dbus_message_new_method_call("org.bluez", adapter,
+					       "org.bluez.Adapter",
+					       "FindDevice");
+	dbus_message_iter_init_append(message, &iter);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &bdaddr);
+
+	if (adapter_reply != NULL)
+		dbus_message_unref(adapter_reply);
+
+	reply = dbus_connection_send_with_reply_and_block(conn,
+							message, -1, NULL);
+
+	dbus_message_unref(message);
+
+	if (!reply) {
+		message = dbus_message_new_method_call("org.bluez", adapter,
+						       "org.bluez.Adapter",
+						       "CreateDevice");
+		dbus_message_iter_init_append(message, &iter);
+		dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &bdaddr);
+
+		reply = dbus_connection_send_with_reply_and_block(conn,
+								  message, -1, NULL);
+
+		dbus_message_unref(message);
+
+		if (!reply)
+			return FALSE;
+	}
+	if (dbus_message_get_args(reply, NULL, DBUS_TYPE_OBJECT_PATH, &object_path,
+				  DBUS_TYPE_INVALID) == FALSE) {
+		return FALSE;
+	}
+
+	id = device_get_ieee1284_id(adapter, object_path);
+	if (id == NULL)
+		return FALSE;
+	printf("%s", id);
+	g_free(id);
+
+	return TRUE;
+}
+
 /*
  *  Usage: printer-uri job-id user title copies options [file]
  *
@@ -642,10 +711,19 @@ int main(int argc, char *argv[])
 			return CUPS_BACKEND_OK;
 		else
 			return CUPS_BACKEND_FAILED;
+	} else if (argc == 3 && strcmp(argv[1], "--get-deviceid") == 0) {
+		if (bachk(argv[2]) < 0) {
+			fprintf(stderr, "Invalid Bluetooth address '%s'\n", argv[2]);
+			return CUPS_BACKEND_FAILED;
+		}
+		if (print_ieee1284(argv[2]) == FALSE)
+			return CUPS_BACKEND_FAILED;
+		return CUPS_BACKEND_OK;
 	}
 
 	if (argc < 6 || argc > 7) {
 		fprintf(stderr, "Usage: bluetooth job-id user title copies options [file]\n");
+		fprintf(stderr, "       bluetooth --get-deviceid [bdaddr]\n");
 		return CUPS_BACKEND_FAILED;
 	}
 
-- 
1.7.0.1

Re: [PATCH] Get IEEE1284 for a single printer

[Johan Hedberg]

Hi Bastien,

On Sun, Jun 06, 2010, Bastien Nocera wrote:
> Let me know what you think.

Sounds ok'ish to me, but how exactly would udev-configure-printer be
called? If it's directly executed (as opposed to e.g. using D-Bus) then
privilege and environment inheritance needs to be considered (i.e. is it
fine that it gets run with the same privileges and environment as
bluetoothd itself).

> The patch below could still be useful for debugging (in the worst
> case).

I have nothing against pushing the patch upstream as long as its coding
style issues are fixed:

> +	message = dbus_message_new_method_call("org.bluez", "/",
> +					       "org.bluez.Manager",
> +					       "DefaultAdapter");

Mixed tabs and spaces for indentation.

> +	adapter_reply = dbus_connection_send_with_reply_and_block(conn,
> +								  message, -1, NULL);

Same.

> +	if (dbus_message_get_args(adapter_reply, NULL, DBUS_TYPE_OBJECT_PATH, &adapter, DBUS_TYPE_INVALID) == FALSE)

Longer line than 79 characters.

> +	message = dbus_message_new_method_call("org.bluez", adapter,
> +					       "org.bluez.Adapter",
> +					       "FindDevice");

Mixed tabs and spaces.

> +		message = dbus_message_new_method_call("org.bluez", adapter,
> +						       "org.bluez.Adapter",
> +						       "CreateDevice");

Mixed tabs and spaces.

> +		dbus_message_iter_init_append(message, &iter);
> +		dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &bdaddr);

Longer line than 79 characters.

> +		reply = dbus_connection_send_with_reply_and_block(conn,
> +								  message, -1, NULL);

Mixed tabs and spaces as well as overlong line.

> +		if (!reply)
> +			return FALSE;
> +	}
> +	if (dbus_message_get_args(reply, NULL, DBUS_TYPE_OBJECT_PATH, &object_path,

Missing empty line after } as well as too long line.

> +				  DBUS_TYPE_INVALID) == FALSE) {

Mixed tabs and spaces.

> +			fprintf(stderr, "Invalid Bluetooth address '%s'\n", argv[2]);

Too long line.

Johan

Re: [PATCH] Get IEEE1284 for a single printer

[Bastien Nocera]

On Tue, 2010-06-08 at 15:09 +0800, Johan Hedberg wrote:
> Hi Bastien,
> 
> On Sun, Jun 06, 2010, Bastien Nocera wrote:
> > Let me know what you think.
> 
> Sounds ok'ish to me, but how exactly would udev-configure-printer be
> called? If it's directly executed (as opposed to e.g. using D-Bus) then
> privilege and environment inheritance needs to be considered (i.e. is it
> fine that it gets run with the same privileges and environment as
> bluetoothd itself).
> 
> > The patch below could still be useful for debugging (in the worst
> > case).
> 
> I have nothing against pushing the patch upstream as long as its coding
> style issues are fixed:
<snip>
> Mixed tabs and spaces.

You do realise that all those are cut'n'paste from another function in
the same source file?

> > +			fprintf(stderr, "Invalid Bluetooth address '%s'\n", argv[2]);
> 
> Too long line.

Right, I'll fix that.

Re: [PATCH] Get IEEE1284 for a single printer

[Johan Hedberg]

Hi Bastien,

On Tue, Jun 08, 2010, Bastien Nocera wrote:
> > I have nothing against pushing the patch upstream as long as its coding
> > style issues are fixed:
> <snip>
> > Mixed tabs and spaces.
> 
> You do realise that all those are cut'n'paste from another function in
> the same source file?

Nope, didn't realize that. So the whole file needs coding style cleanups
then.

> > > +			fprintf(stderr, "Invalid Bluetooth address '%s'\n", argv[2]);
> > 
> > Too long line.
> 
> Right, I'll fix that.

Thanks.

Johan

Re: [PATCH] Get IEEE1284 for a single printer

[Bastien Nocera]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

On Tue, 2010-06-08 at 17:26 +0800, Johan Hedberg wrote:
> Hi Bastien,
> 
> On Tue, Jun 08, 2010, Bastien Nocera wrote:
> > > I have nothing against pushing the patch upstream as long as its coding
> > > style issues are fixed:
> > <snip>
> > > Mixed tabs and spaces.
> > 
> > You do realise that all those are cut'n'paste from another function in
> > the same source file?
> 
> Nope, didn't realize that. So the whole file needs coding style cleanups
> then.

I've only fixed the style problems in the patch itself. I'm happy to do
the rest of the file, if you have a "indent" magic incantation for it.

> > > > +			fprintf(stderr, "Invalid Bluetooth address '%s'\n", argv[2]);
> > > 
> > > Too long line.
> > 
> > Right, I'll fix that.

Done.

Updated patch attached.

[-- Attachment #2: 0001-cups-Add-ability-to-print-IEEE1284-device-ID.patch --]
[-- Type: text/x-patch, Size: 3158 bytes --]

>From 35c984cfbe0a17bb3dd7231450f72854b3ffb7d0 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Sun, 6 Jun 2010 15:48:26 +0100
Subject: [PATCH] (cups) Add ability to print IEEE1284 device ID

Add ability to print IEEE1284 device ID for Bluetooth
printers to allow auto-configuration once paired.
---
 cups/main.c |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 83 insertions(+), 0 deletions(-)

diff --git a/cups/main.c b/cups/main.c
index 9659a11..b420643 100644
--- a/cups/main.c
+++ b/cups/main.c
@@ -605,6 +605,79 @@ static gboolean list_printers(void)
 	return TRUE;
 }
 
+static gboolean print_ieee1284(const char *bdaddr)
+{
+	DBusMessage *message, *reply, *adapter_reply;
+	DBusMessageIter iter;
+	char *object_path = NULL;
+	char *adapter;
+	char *id;
+
+	adapter_reply = NULL;
+
+	conn = g_dbus_setup_bus(DBUS_BUS_SYSTEM, NULL, NULL);
+	if (conn == NULL)
+		return FALSE;
+
+	message = dbus_message_new_method_call("org.bluez", "/",
+			"org.bluez.Manager",
+			"DefaultAdapter");
+
+	adapter_reply = dbus_connection_send_with_reply_and_block(conn,
+			message, -1, NULL);
+
+	dbus_message_unref(message);
+
+	if (dbus_message_get_args(adapter_reply, NULL,
+			DBUS_TYPE_OBJECT_PATH, &adapter,
+			DBUS_TYPE_INVALID) == FALSE)
+		return FALSE;
+
+	message = dbus_message_new_method_call("org.bluez", adapter,
+			"org.bluez.Adapter",
+			"FindDevice");
+	dbus_message_iter_init_append(message, &iter);
+	dbus_message_iter_append_basic(&iter, DBUS_TYPE_STRING, &bdaddr);
+
+	if (adapter_reply != NULL)
+		dbus_message_unref(adapter_reply);
+
+	reply = dbus_connection_send_with_reply_and_block(conn,
+			message, -1, NULL);
+
+	dbus_message_unref(message);
+
+	if (!reply) {
+		message = dbus_message_new_method_call("org.bluez", adapter,
+				"org.bluez.Adapter",
+				"CreateDevice");
+		dbus_message_iter_init_append(message, &iter);
+		dbus_message_iter_append_basic(&iter,
+				DBUS_TYPE_STRING, &bdaddr);
+
+		reply = dbus_connection_send_with_reply_and_block(conn,
+				message, -1, NULL);
+
+		dbus_message_unref(message);
+
+		if (!reply)
+			return FALSE;
+	}
+	if (dbus_message_get_args(reply, NULL,
+			DBUS_TYPE_OBJECT_PATH, &object_path,
+			DBUS_TYPE_INVALID) == FALSE) {
+		return FALSE;
+	}
+
+	id = device_get_ieee1284_id(adapter, object_path);
+	if (id == NULL)
+		return FALSE;
+	printf("%s", id);
+	g_free(id);
+
+	return TRUE;
+}
+
 /*
  *  Usage: printer-uri job-id user title copies options [file]
  *
@@ -642,10 +715,20 @@ int main(int argc, char *argv[])
 			return CUPS_BACKEND_OK;
 		else
 			return CUPS_BACKEND_FAILED;
+	} else if (argc == 3 && strcmp(argv[1], "--get-deviceid") == 0) {
+		if (bachk(argv[2]) < 0) {
+			fprintf(stderr, "Invalid Bluetooth address '%s'\n",
+					argv[2]);
+			return CUPS_BACKEND_FAILED;
+		}
+		if (print_ieee1284(argv[2]) == FALSE)
+			return CUPS_BACKEND_FAILED;
+		return CUPS_BACKEND_OK;
 	}
 
 	if (argc < 6 || argc > 7) {
 		fprintf(stderr, "Usage: bluetooth job-id user title copies options [file]\n");
+		fprintf(stderr, "       bluetooth --get-deviceid [bdaddr]\n");
 		return CUPS_BACKEND_FAILED;
 	}
 
-- 
1.7.0.1

Re: [PATCH] Get IEEE1284 for a single printer

[Johan Hedberg]

Hi Bastien,

On Wed, Sep 01, 2010, Bastien Nocera wrote:
> From: Bastien Nocera <hadess@hadess.net>
> Date: Sun, 6 Jun 2010 15:48:26 +0100
> Subject: [PATCH] (cups) Add ability to print IEEE1284 device ID
> 
> Add ability to print IEEE1284 device ID for Bluetooth
> printers to allow auto-configuration once paired.
> ---
>  cups/main.c |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 83 insertions(+), 0 deletions(-)

Thanks for the updated patch. It's now pushed upstream.

> I've only fixed the style problems in the patch itself. I'm happy to do
> the rest of the file, if you have a "indent" magic incantation for it.

In my experience indent can produce quite a mess compared to fixing
style issues manually. Anyway, I went ahead and did the cleanup myself
for the issues that I could quickly spot and pushed the fixes as a
separate patch upstream.

Johan

Re: [PATCH] Get IEEE1284 for a single printer

[Bastien Nocera]

On Wed, 2010-09-01 at 21:49 +0300, Johan Hedberg wrote:
> Hi Bastien,
> 
> On Wed, Sep 01, 2010, Bastien Nocera wrote:
> > From: Bastien Nocera <hadess@hadess.net>
> > Date: Sun, 6 Jun 2010 15:48:26 +0100
> > Subject: [PATCH] (cups) Add ability to print IEEE1284 device ID
> > 
> > Add ability to print IEEE1284 device ID for Bluetooth
> > printers to allow auto-configuration once paired.
> > ---
> >  cups/main.c |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 83 insertions(+), 0 deletions(-)
> 
> Thanks for the updated patch. It's now pushed upstream.

Thanks.

> > I've only fixed the style problems in the patch itself. I'm happy to do
> > the rest of the file, if you have a "indent" magic incantation for it.
> 
> In my experience indent can produce quite a mess compared to fixing
> style issues manually. Anyway, I went ahead and did the cleanup myself
> for the issues that I could quickly spot and pushed the fixes as a
> separate patch upstream.

OK. Indent should be able to do this without making a mess as long as
you know the exact parameters. Mind, some of the restrictions feel like
they're from COBOL days (79 characters line width?).

I prefer readable code to mashed-up code that looks that way because of
hard constraints.