questions vanity lights

[Michael E Brown]

On Mon, Mar 26, 2007 at 09:31:54AM +0200, Oliver Eichler wrote:
> 
> Hi Michael
> 
> > Too cool. I'll have a look at it when I get back into the office.
> 
> I enhanced the code a little bit over the weekend.

First, I dont have any experience with dbus, so I cannot give any
dbus-related feedback. Might be useful to get somebody with qt/dbus
experience for more feedback.

Next, I had some slight difficulty compiling on my system. I have an FC6
system. I had to install qt4-devel to compile and run qmake-qt4. Not
sure if this is intended or not (vs qt3, etc) 

This looks interesting, my problem is currently what exactly to do with
it. It has dependencies outside of what I currently require (qt), so I
cannot put it directly into libsmbios tarball. It would have to go into
its own project. That means its own RPM and such. The problem is that I
have no easy way to test it. I would be willing to write a spec file and
make the RPM, host it for download, etc. I would probably even maintain
it for fedora and possibly opensuse (once I figure out the opensuse
contributor process). But, I would need contributors (you, possibly
others) to maintain it.

Let me know what you would like to do.

Here are a few things I saw:

-- code quality looks good. Code is readable and formatted nicely.
Builds and runs after some initial problems getting everything
installed. Gives a nice "LED not supported" message on my system (which
is accurate).

-- Versioning? Probably need to have the server export an API version
and check in the client to avoid incompatibilities.

-- your init script should probably modprobe dcdbas 
  -> if dcdbas is not loaded, daemon terminates with:
  terminate called after throwing an instance of
  'smbios::InternalErrorImpl'
    what():  Could not open file /sys/devices/platform/dcdbas/smi_data.
    Check that dcdbas driver is properly loaded.
  Aborted

-- probably want to make a x-platform init script that will work on
fedora/rh/suse. lsb maybe?  

-- init script needs fedora-style chkconfig lines as well as the suse
lines.

-- The service should probably check if lights are supported and exit
gracefully so it doesnt take up resources on systems without lights.
Any way to just have dbus return a nice error in this case without
keeping the daemon running?

-- Seems like you would want the service to daemonize itself, with an
optional arg to tell it not to.

src/dellxpsd/CXPSAdaptor.cpp:
-- probably want to use enums in the switch() instead of hardcoded
values in setLEDColor()

--
Michael