current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

This is a script I made while having trouble with xautolock and dimming of the screen right before locking it. I would like some tips for making it a bit more robust and to know of any disadvantages of it. I don't know how it works with multiple screens but I suspect it does not.

#!/bin/bash

# Lock screen in:
minutes=15
seconds=0

# How many seconds it takes to dim
dim_secs=5

idle_secs=$(( $minutes * 60 + $seconds )) # Screen timeout in seconds
idle_ms=$(( $idle_secs * 1000 ))

timeout=$idle_secs

while true; do
    sleep $timeout

    if pgrep xflock4 > /dev/null; then
        timeout=$idle_secs
        continue
    fi

    # After sleeping, check user idle time
    if [ $(xprintidle) -ge $idle_ms ]; then
        WINDOW=$(echo $(xwininfo -id $(xdotool getactivewindow) -stats | \
                        egrep '(Width|Height):' | \
                        awk '{print $NF}') | \
                sed -e 's/ /x/')
        SCREEN=$(xdpyinfo | grep -m1 dimensions | awk '{print $2}')

        # If greater than timeout check if something's not in fullscreen and active
        if [ "$WINDOW" != "$SCREEN" ]; then
            interrupted=false # If dimming's been interrupted

            screen=$(xrandr -q | grep " connected" | awk '{print $1;}') # Connected screen
            brightness=$(xrandr --verbose | grep -i brightness | cut -f2 -d ' ') # Current brightness

            start_time=$(date "+%s.%N") # Current time
            while true; do
                # Current brightness(0 to $brigtness), decided by time since dimming started
                br=$(echo "$brightness-$brightness*(($(date "+%s.%N")-$start_time)/$dim_secs)" | bc -l)
                if [ $(echo "$br >= 0" | bc) -eq 1  ]; then
                    # Set the current brightness and sleep for a short while
                    xrandr --output $screen --brightness $br
                    sleep 0.01
                else
                    # If brightness is less than 0 then it means that the
                    # time since starting dimming is greater than $dim_secs
                    break
                fi

                # If the user gave an input that's less than $idle_ms
                # Will always be less than $idle_ms if it was interrupted during dimming
                if [ $(xprintidle) -lt $idle_ms ]; then
                    interrupted=true
                    break
                fi
            done
            if $interrupted; then
                # If it was interrupted during dimming, reset brightness
                xrandr --output $screen --brightness $brightness
            else
                xset dpms force off # Turn screen off
                xrandr --output $screen --brightness $brightness # Reset brightness

                xflock4 # Lock screen
            fi
        fi
    fi

    # Timeout before checking idle time again
    timeout=$(echo "$idle_secs - $(xprintidle) / 1000 " | bc -l)
done
share|improve this question

1 Answer 1

up vote 2 down vote

You don't need to prefix variables with $ inside $(( ... )), this will work just as well:

idle_secs=$(( minutes * 60 + seconds ))  # Screen timeout in seconds
idle_ms=$(( idle_secs * 1000 ))

In many places you pipe to a grep and then pipe to awk. Note that awk can perform many of the functions of grep, and using one process (awk) instead of two (grep + awk) would be more optimal.

Here are some examples. Instead of:

... | egrep '(Width|Height):' | awk '{print $NF}'

This is equivalent:

... | awk '/(Width|Height):/ {print $NF}'

Instead of:

... | grep -m1 dimensions | awk '{print $2}'

This is equivalent:

... | awk '/dimensions/ {print $2; exit}'

And so on, similarly at other places too, you get the idea.

Another common pattern I see at many places:

echo "$br >= 0" | bc -l

That is, using echo to pass a string to a command. A better way is using here-strings, like this:

bc -l <<< "$br >= 0"

This long line appears at multiple places, sometimes with minor variations:

# If it was interrupted during dimming, reset brightness
xrandr --output $screen --brightness $brightness

It would be better to move this to a function, for example reset_brightness, to reduce duplication of logic.

share|improve this answer
    
I get "pgrep: invalid option -- 'q'" –  thabubble Nov 25 '14 at 21:05
    
Ah... Worked in my system (BSD version), looks like GNU version doesn't have it. Thanks, removed that point. –  janos♦ Nov 25 '14 at 21:10

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.