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

Richard Hughes hughsient at gmail.com
Wed Mar 2 03:22:21 PST 2011


On 1 March 2011 23:00, Michael Sweet <msweet at apple.com> wrote:
> 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 and attach the patch, explaining what it does, etc.

I've created an account, and have also updated the patch and attached
it there: http://www.cups.org/str.php?L3808

>Due to the size of the change, there will also be some legal fun (which we can discuss offline) before we can incorporate it.

Sure, I figured there might be some copyright issues, which I'm sure
won't be any problem. I'm pretty easy going and can contribute the
code from myself rather than Red Hat if that's easier.

> You should also review the CUPS Developer Guide at:

Thanks. I've amended my patch to conform to those as closely as possible.

> 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 think putting the ColorSync code into colorsync is a very good idea.
I've amended my patch to included the printers.c stubs that would need
to be moved over.

I also don't think any of the DBus stuff is shared, it's quite
specific to colord in this instance.

> 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.

When would cupsdCmsStart and cupsdCmsStop be called? I couldn't see
anything in printers.c, although maybe I need to widen my focus a
little to other files. Advice welcome.

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

Fixed.

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

Both fixed, thanks!

> 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've added some #defines with the enums we're using, and added a link
to the developer documentation.

Thanks for your help so far,

Richard.





More information about the cups mailing list