On Sat, Oct 13, 2012 at 3:03 PM, Иван Шаповалов <intelfx100@gmail.com> wrote:
2012/10/12 Jouke Witteveen <j.witteveen@gmail.com>:
On Mon, Oct 08, 2012 at 08:44:27PM +0400, Ivan Shapovalov wrote:
On Sunday 07 October 2012 22:19:47 Jouke Witteveen wrote:
I'm not quite comfortable with the new look of the code. For one thing it would be nice to upgrade to the new ABI in one go: http://www.kernel.org/doc/Documentation/ABI/stable/sysfs-class-rfkill
I'll try to do that. The only thing we use from the obsolete API is the "state" file, which is replaced with "hard" and "soft" files containing respective states.
Can someone point me at some official statement about the location of rfkill stuff under /sys/class/net/$INTERFACE ? If there is no official location, I think it is perhaps best to make RFKILL_NAME mandatory and only look under /sys/class/rfkill .
There seems to be no such statement, as the sysfs ABI is officially unstable. OTOH, I think we should not sacrifice usability for the sake of "code cleanness".
An example: on my system (Atheros AR5B97, driver ath9k) I need to remove the driver module before suspending and to restore it afterwards, otherwise network performance drops noticeably. And when a driver is reinserted, the rfkill's name/index is changed (was phyN, becomes phyN+1).
In general I would like src/rfkill to be simple. This patch makes it hackish in my opinion.
- Jouke
Thanks, Ivan.
What about the following? Would everybody be happy with that? I decided to go with find instead of globbing, since a recursive glob would hang on the cyclic links. Using non-recursive globs requires multiple patterns and multiple checks to see whether we are dealing with actual directories. The results of find are in Breadth First order, a simple 'head -n 1' suffices.
This code has not been tested by me.
Regards, - Jouke
--- diff --git a/src/rfkill b/src/rfkill index 12e1832..4ce3ca3 100644 --- a/src/rfkill +++ b/src/rfkill @@ -8,10 +8,10 @@ set_rf_state() { local path=$(get_rf_path "$INTERFACE" "$RFKILL_NAME") || return 1 case "$state" in enabled) - echo 1 > "$path/state" + echo 0 > "$path/soft" ;; disabled) - echo 0 > "$path/state" + echo 1 > "$path/soft" ;; esac } @@ -28,8 +28,8 @@ get_rf_path() { done report_fail "no rfkill switch with name $RFKILL_NAME" else - path="/sys/class/net/$INTERFACE/rfkill" - if [[ -d "$path" ]]; then + path=$(find -L "/sys/class/net/$INTERFACE/" -maxdepth 2 -type d -name "rfkill*" 2> /dev/null | head -n 1) + if [[ -n "$path" ]]; then echo "$path" return 0 fi @@ -38,35 +38,21 @@ get_rf_path() { return 1 }
-get_rf_state() { - local INTERFACE="$1" PROFILE="$2" path state - - path=$(get_rf_path "$INTERFACE" "$RFKILL_NAME") || return 1 - read state < "$path/state" - - case "$state" in - 0|2) - echo "disabled";; - 1) - echo "enabled";; - *) - echo "$state";; - esac -} - enable_rf() { - local INTERFACE="$1" RFKILL="$2" RFKILL_NAME="$3" + local INTERFACE="$1" RFKILL="$2" RFKILL_NAME="$3" path hard soft + # Enable rfkill if necessary, or fail if it is hardware if [[ -n "$RFKILL" ]]; then - local state=$(get_rf_state "$INTERFACE") || return 1 - if [[ "$state" != "enabled" ]]; then - if [[ "$RFKILL" == "soft" ]]; then - set_rf_state "$INTERFACE" enabled $RFKILL_NAME - sleep 1 - else - report_fail "radio is disabled on $INTERFACE" - return 1 - fi + path=$(get_rf_path "$INTERFACE" "$RFKILL_NAME") || return 1 + read hard < "$path/hard" + read soft < "$path/soft" + + if (( hard )); then + report_fail "radio is disabled on $INTERFACE" + return 1 + elif (( soft )); then + set_rf_state "$INTERFACE" enabled $RFKILL_NAME || return 1 + timeout_wait 1 "(( ! \$(< \"$path/soft\") ))" fi fi } -- 1.7.12.2
It seems ok; and then one shall mark "$RFKILL" as unused...
Regards, Ivan.
RFKILL is still used on lines 4 and 60. When set_rf_state is called from enable_rf on line 64, the declaration of line 58 survives. That's the way local variables work in bash. The difference with the old implementation is that we no longer explicitly check for 'soft', but only that it is not 'hard'. Regards, - Jouke