Re: Script update

[Robert Elz <kre%munnari.OZ.AU@localhost>]

    Date:        Thu, 26 Mar 2026 04:47:00 -0300
    From:        Isac =?UTF-8?B?TW9uw6fDo28=?= <isacmoncao%disroot.org@localhost>
    Message-ID:  <20260326044700.52582862@moncao>

A few comments on your script - and please don't take these
the wrong way, everyone needs to learn.


  | ```sh
  | # Script to easily manipulate audio volume in NetBSD
  | # Author:           Isac Mon=C3=A7=C3=A3o <isacmoncao%disroot.org@localhost>
  | # Last Modified:    26/03/2026 04:09 AM -03
  | # Requirements:     echo, mixerctl, sed, bc=20
  | # License:          ISC
  | #
  | # Contributions:
  | #  - DTB <trinity@tebibyte.media>: Multiple if statements refactored
  | into a single case statement.
  | #=20

That trailing space is probably an accident.

  | # =E9=A0=91=E5=BC=B5=E3=81=A3=E3=81=9F

(there's no point in me decoding the quoted-printable for that,
this is just an artifact of my MUA).

  | #
  | #!/bin/sh

The #! line must be the first line of the file (the # and ! must be
the first two characters of the file) if it is to do anything at all,
otherwise you might as well just remove it.

  | INCREMENT="1"

The quotes there achieve nothing, but the value also probably should
not be fixed in the script

  | MIXER_VARIABLE=outputs.master

Instead

	INCREMENT=$(mixerctl -v "${MIXER_VARIABLE}")
	case $INCREMENT in
	*delta=*)
		INCREMENT=${INCREMENT##*delta=}
		;;
	*)	INCREMENT=1		# default in case we aren't told.
		;;
	esac

and once you have that, then:

	MAXVOL=$(( 256 - INCREMENT ))

(for later)

  | usage() {
  |   echo "usage: vol up"
  |   echo "       vol down"
  |   echo "       vol get"
  | }

Avoid echo, while as used there it is safe, it is better to
get into the habit of using printf - besides, printf is easier
to use (no need to carefully insert the right number of spaces).

Further, allow people to call the script whatever they want.

	ME=${0##*/}		# This line is often the first executed,
			# so the ME var is set for all errors/usage ...

	usage()
	{
		printf "%-7s${ME} %s\\n" Usage: up '' down '' get
	}


  | get_vol() {
  |   pattern="$MIXER_VARIABLE=3D[0-9][0-9]?[0-9]?,"
  |
  |   current_vol=$(mixerctl "$MIXER_VARIABLE" | sed -E s"/$pattern//")
  |   perc_val=$(echo "(100 * $current_vol) / 248" | bc)
  |   echo "$perc_val%"
  | }

You don't need sed, and definitely not bc, for that (bc would help
if you wanted fractional percentages, but for this, I agree, a simple
integer is better.

	get_vol()
	{
		volume=$(mixerctl "${MIXER_VARIABLE}") ||
			fail 'Unable to get volume'
		case $volume in
		"${MIXER_VARIABLE}="*,*)
			volume=${volume#"${MIXER_VARIABLE}="*,}
			;;
		"${MIXER_VARIABLE}="*)
			volume=${volume#"${MIXER_VARIABLE}="}
			;;
		*)
			fail 'Unrecognisable mixerctl output' "${volume}"
			;;
		esac
		printf '%d%%\n' "$(( (100 * volume) / MAXVOL ))"
	}

sed (regular expressions) are better at matching precise patterns than
shell patterns are, but as long as you're assuming the output format,
and you were, and doing nothing if it isn't what you're expecting, there's
no advantage to be gained at all.  The shell version will "work" if the
result from requesting the value was
	
		outputs.master=unknown

or something, the patterns could be made better to handle that, but
not to the extreme of what sed can do ... but good enough.

Of course, you also now need a "fail" function, something like

	fail()
	{
		IFS=' '
		printf >&2 '%s: %s\n' "${ME}" "$*"
		exit 1
	}

  | case "$1" in
  |    up)   mixerctl -w "$MIXER_VARIABLE+=$INCREMENT" ;;
  |    down) mixerctl -w "$MIXER_VARIABLE-=$INCREMENT" ;;
  |    get)  get_vol ;;
  |    *)    usage; exit 64;; # sysexits(3) EX_USAGE
  | esac


exit 1 is good enough.   The sysexits() values are designed for
applications that work as part of a group of commands which co-operate
for exit values (those were originally designed for sendmail to work
with other mail applications).

In general, it is better (and safer) if you always get into the
habit of using { } in variable expansions, when the variable name
is longer than one character (${x} is OK, but $x is easier, provided
what follows next can't be part of a var name).

You could add an " || fail ...." after the two mixerctl invocations
as well, or just rely upon any error message mixerctl produces.

kre