Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 165456 - Review Request: fnfx - Enables hotkey functions for Toshiba laptop
Summary: Review Request: fnfx - Enables hotkey functions for Toshiba laptop
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Matthias Saou
QA Contact: David Lawrence
URL: http://fnfx.sourceforge.net
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2005-08-09 15:49 UTC by Andreas Bierfert
Modified: 2007-11-30 22:11 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2005-08-24 00:48:03 UTC
Type: ---
Embargoed:

Attachments (Terms of Use)
Spec file patch (2.52 KB, patch)
2005-08-19 09:55 UTC, Matthias Saou 		no flags Details | Diff
New init script (1006 bytes, text/plain)
2005-08-23 12:51 UTC, Matthias Saou 		no flags Details
View All

Description Andreas Bierfert 2005-08-09 15:49:33 UTC 
Spec Name or Url: http://fedora.lowlatency.de/review/fnfx.spec
SRPM Name or Url: http://fedora.lowlatency.de/review/fnfx-0.3-2.src.rpm
Description: 
FnFX enables owners of Toshiba laptops to change the LCD brightness,
control, the internal fan and use the special keys on their keyboard
(Fn-x combinations, hot-keys). The internal functions will give the
possibility to map the Fn-Keys to functions like volume up/down, mute,
suspend to disk, suspend to ram and switch LCD/CRT/TV-out. These
functions heavily depend on the system and/or kernel configuration.
You will need at least a kernel (v2.4.x, v2.5.x, v2.6.x) with ACPI and
Toshiba support (CONFIG_ACPI and CONFIG_ACPI_TOSHIBA).
Comment 1 Matthias Saou 2005-08-19 09:55:25 UTC 
Attached is a patch to the spec file to fix minor issues. The first real blocker
would have been that the _sysconfdir/fnfx/ directory was unowned by the package,
thus not removed upon uninstall. The second maybe the execution bit of the init
script after storing the file in CVS, as it wasn't explicitely set anywhere.

Now for the init script : Things seem wrong, as if the daemon crashes, then the
/var/lock/subsys/fnfxd will be left over, and given how the checks are made, it
will prevent it from being started again, and even forcing a stop or restart
won't get rid of the file.
Comment 2 Matthias Saou 2005-08-19 09:55:38 UTC 
Created attachment 117905 [details]
Spec file patch
Comment 3 Andreas Bierfert 2005-08-19 22:45:03 UTC 
I still don't know about the init script but the rest is fixed:

http://fedora.lowlatency.de/review/fnfx-0.3-3.src.rpm
http://fedora.lowlatency.de/review/fnfx.spec
Comment 4 Andreas Bierfert 2005-08-23 10:02:29 UTC 
ping?

I would say for the script part: Leave it as is and if problems arise find a way
to fix them. I think it is ok the way it is.
Comment 5 Matthias Saou 2005-08-23 12:51:29 UTC 
No, the init script is definitely not ok the way it is. There is also a
"RETVAL=$?" after "echo" (always true) instead of after the "killproc" call
which will have the script return a wrong exit status in some occasions.

Attached is a completely new init script. It fixes all the issues I've seen, but
is completely untested, so please give it a try in case I've missed something.
Note that I've enabled the service by default for runlevels 2345. This is
something we don't want to do often, but for such a specific service that
doesn't do any kind of network access, it should be considered ok, and will save
the end user the step of manually enabling the service.
Comment 6 Matthias Saou 2005-08-23 12:51:38 UTC 
Created attachment 118000 [details]
New init script
Comment 7 Andreas Bierfert 2005-08-23 14:21:29 UTC 
Thx for your work =) I am not into init scripts that much...

http://fedora.lowlatency.de/review/fnfx-0.3-4.src.rpm
http://fedora.lowlatency.de/review/fnfx.spec
Comment 8 Matthias Saou 2005-08-23 14:34:01 UTC 
Can you confirm that the init script is working? As soon as that's confirmed,
I'll approve the package :-)
Comment 9 Andreas Bierfert 2005-08-23 20:41:04 UTC 
The script looks good (in comparison to other scripts) and does what is expected
on my computer.. problem is: I don't have a toshiba laptop and this packages was
actually the reason for a friend of mine not to switch to fc (yet ;) ) so this
is why I initially put it up here... 

I will say it like this: It does not bail on anything I am doing but I can not
test the real thing here.
Comment 10 Matthias Saou 2005-08-24 00:48:03 UTC 
OK, then go ahead and import the package.
Comment 11 Ville Skyttä 2005-08-24 06:14:59 UTC 
Reminder: http://fedoraproject.org/wiki/Extras/NewPackageProcessMarkTwo  
  
Things are easier to track if they flow according to the process; a new 
submission should be closed (by the submitter) only after it's in CVS and 
successfully built.
Comment 12 Christian Iseli 2006-10-18 08:56:14 UTC 
Normalize summary field for easy parsing
Note You need to log in before you can comment on or make changes to this bug.