[cups.development] [patch] Add colord support to CUPS

[Michael Sweet]

On Mar 1, 2011, at 8:45 AM, Richard Hughes wrote:
> ...
> So, could someone please review my patch and give me some comments.
> I've used a local git-svn mirror to generate the patch against svn
> trunk, although if should apply fine if you use the patch -p1 option.
> To test, you need to be running colord 0.1.3. If colord isn't present
> then no devices or profiles are added and we continue like normal
> without errors. The patch also appears to apply against 1.4.

Richard,

Thanks for the patch!  In order for this to get into the base CUPS distribution, we'll want you to create a bug report here:

    http://www.cups.org/str.php

and attach the patch, explaining what it does, etc.  Due to the size of the change, there will also be some legal fun (which we can discuss offline) before we can incorporate it.

You should also review the CUPS Developer Guide at:

    http://www.cups.org/documentation.php/doc-1.4/spec-cmp.html

Among other things, it covers our coding standards so that the code you submit can be as close as possible in style to the existing stuff (it helps for readability and maintenance of the code...)

......

OK, all that said I suspect we will want to do some refactoring of your changes, along with separating out our current ColorSync code so that ipp.c calls into a common abstraction layer.  We should also see if there are common DBUS "helper" functions being used between cupsd and the dbus notifier, and put them in libcups as private API as needed.

I would like to see the DBUS connection opened and closed whenever the scheduler does a full reload - that will ensure we don't have old profile registrations lying around.  We can provide high-level cupsdStart/StopColorManagement (or something like that) functions that do the right thing for ColorSync and DBUS/colord.

There are several places where dbus_message_new_method_call is called with some constants - this should be made into a helper function.

The helper functions shouldn't use a dbus_ prefix (not from that library) and watch the speling mistaks! :)

Finally, I'd like to see some more background info and/or URLs to the documentation in the source code comments to make future maintenance a bit easier.  I can take care of doing the same for ColorSync...

________________________________________________________________________
Michael Sweet, Senior Printing System Engineer, PWG Chair