code duplication in wireless scripts

[Nicolò Chieffo]

hello!
Ther's a code duplication in wireless scripts of /etc/acpi/wireless.sh , /etc/acpi/asus-wireless.sh and /usr/share/acpi-support/state-funcs this is a problem for you (or someone than would change wireless scripts) to mantain everything working. To follow software-engineering principles you should at least implement functions in /usr/share/acpi-support/state-funcs and use them in the other scripts!

simple proposal:

setLEDAsusWireless should be used in /etc/acpi/asus-wireless.sh
and /etc/wireless.sh should use a new function to be added in /usr/share/acpi-support/state-funcs which switches the status (which in its body uses the other function isAnyWirelessPoweredOn;)

If you follow these steps you'll have easier life whenever something has to be changed :)

[Nicolò Chieffo (yelo3)]

this could probably be a good proposal!
while writing it I've also considered bugs:
42820, 42387, 38354, 37010

please tell me if you're interested and if you need some explanation in my code

[Nicolò Chieffo (yelo3)]

there are some functions... you can include the file and use them, here's an example

state check led; (will check the state and set all leds (asus, ibm, toshiba))
state switch led; (will switch state and set all leds)
state switch; (will switch state without setting leds)
state init led; (will look for a default interface state configuration in /etc/default/wireless in this form: eth1=off)

[Nicolò Chieffo (yelo3)]

in this way all hotkeys actions could point to /etc/acpi/wireless.sh and this file can be

#!/bin/bash
. /usr/share/acpi-support/state-funcs
state switch led

or if we want better performance we could leave different action files for every laptop! for instance asus-wireless.sh could look like that:

#!/bin/bash
. /usr/share/acpi-support/state-funcs
STATE=`state switch`

asus_led $STATE

and at boot there should be a reference, in networking init.d script to:
state init led;

[Nicolò Chieffo (yelo3)]

I'm afraid I did a small error in my code:

when doing

echo $ON_STATE > $STATE_FILE
echo $OFF_STATE > $STATE_FILE

-n might be requered to work!

[Nicolò Chieffo (yelo3)]

added missing -n to echo

[Nicolò Chieffo (yelo3)]

In this way a script can simply include the library /usr/share/acpi-support/wireless-lib and do the following operations:
1) switch_wireless (which includes also the ability to adjust leds)
2) adjust_leds (to be executed at boot or during resume)

When a new method to control leds is "discovered" just add a new file in
/usr/share/acpi-support/wireless-led/

The same thing is valid for new methods to control wireless status
/usr/share/acpi-support/wireless/

these new files must (see attached README)
1) be executable
2) accept 2 parameters (if_name and {on|off|status} )
3) return 0 if the if is ON, 1 if OFF, and 2 if unknown for any reason

I think this is the simplest way to handle frequent changes...

[Nicolò Chieffo (yelo3)]

of course the led method files must only accept one parameter {on|off} there is no need of status (I think) or if_name (definitely not)

[Nicolò Chieffo (yelo3)]

Have you had a look at my proposals or are you going to do it "later"
(which measn feisty + 1)?

[Nicolò Chieffo (yelo3)]

closing this since Paul Sladen did a nice job

[Paul Sladen (sladen)]

 acpi-support (0.95) feisty; urgency=low
 .
   * Clean up issues with toggling multiple wireless cards.
     - Move variable initiation into loop inside 'wireless.sh'
     - Move contents of 'wireless.sh' into lib/state-funcs:toggleAllWirelessState()
     - Update '{ibm,tosh,asus}-wireless.sh' to use state-funcs
     (Closes: LP #96107)
     (Thanks to Nicolo Chieffo for assistance and testing)
   * Remove spurious 'echo HERE' from lib/state-funcs. D'oh.