updates for first review comments

The volume node naming in the manual page is changed to be
consistent with the bluealsa(1) page, with the plugin control
values in parentheses with double-quotes.

Error checking of command line is done the "bluealsa way", and
bash completion is included (I completely forgot about that!)

Verbose output is improved in line with your recommendations.

doc/bluealsa-aplay.1.rst

@@ -128,8 +128,9 @@ OPTIONS
       some other application to apply remote volume change requests.
 
     - **software** - **bluealsa-aplay** will force the BlueALSA PCM volume mode
-      setting to ``software`` and then will not operate its configured ALSA
-      mixer.
+      setting to soft-volume ("software") and then will not operate its
+      configured ALSA mixer. This can be used to enable remote volume control
+      without using an ALSA mixer.
 
     See `Volume control`_ in the **NOTES** section below for more information
     on volume control.
@@ -183,14 +184,14 @@ NOTES
 Volume control
 --------------
 
-If the Bluetooth PCM is using software volume control, then volume
+If the Bluetooth PCM is using BlueALSA soft-volume volume control, then volume
 adjustment will have been applied to the PCM stream within the **bluealsa**
 daemon; so **bluealsa-aplay** does not operate the mixer control in this case.
 
 When using ``--volume=none`` or ``--volume=software``, then the mixer options
 ``--mixer-device``, ``--mixer-name`` and ``--mixer-index`` are ignored, and
 **bluealsa-aplay** will not operate any mixer controls, even if some other
-application changes the PCM volume mode while in use.
+application changes the PCM volume mode to native while in use.
 
 When using ``--volume=auto`` or ``--volume=mixer`` the ALSA mixer control will
 be operated only when the PCM stream is active, (i.e. the remote device is

misc/bash-completion/bluealsa

@@ -265,6 +265,11 @@ _bluealsa_aplay() {
 			__ltrim_colon_completions "$cur"
 			return
 			;;
+		--volume)
+			readarray -t list < <(_bluealsa_enum_values "$1" "$prev")
+			readarray -t COMPREPLY < <(compgen -W "${list[*]}" -- "$cur")
+			return
+			;;
 		--*)
 			[[ ${COMP_WORDS[COMP_CWORD]} = = ]] && return
 			;;

utils/aplay/Makefile.am

@@ -10,6 +10,7 @@ bluealsa_aplay_SOURCES = \
 	../../src/shared/dbus-client.c \
 	../../src/shared/ffb.c \
 	../../src/shared/log.c \
+	../../src/shared/nv.c \
 	alsa-mixer.c \
 	alsa-pcm.c \
 	dbus.c \

utils/aplay/aplay.c

@@ -35,6 +35,7 @@
 #include "shared/defs.h"
 #include "shared/ffb.h"
 #include "shared/log.h"
+#include "shared/nv.h"
 #include "alsa-mixer.h"
 #include "alsa-pcm.h"
 #include "dbus.h"
@@ -1115,10 +1116,26 @@ int main(int argc, char *argv[]) {
 			pcm_period_time = atoi(optarg);
 			break;
 
-		case '8' /* --volume */ :
+		case '8' /* --volume */ : {
+			static const nv_entry_t values[] = {
+				{ "auto", .v.ui = VOL_TYPE_AUTO },
+				{ "mixer", .v.ui = VOL_TYPE_MIXER },
+				{ "software", .v.ui = VOL_TYPE_SOFTWARE },
+				{ "none", .v.ui = VOL_TYPE_NONE },
+				{ 0 },
+			};
+
+			const nv_entry_t *entry;
+			if ((entry = nv_find(values, optarg)) == NULL) {
+				error("Invalid volume control type {%s}: %s",
+						nv_join_names(values), optarg);
+				return EXIT_FAILURE;
+			}
+
 			volume_type_str = optarg;
+			volume_type = entry->v.ui;
 			break;
-
+		}
 		case 'M' /* --mixer-device=NAME */ :
 			mixer_device = optarg;
 			break;
@@ -1174,18 +1191,6 @@ int main(int argc, char *argv[]) {
 		return EXIT_FAILURE;
 	}
 
-	if (strcmp(volume_type_str, "mixer") == 0)
-		volume_type = VOL_TYPE_MIXER;
-	else if (strcmp(volume_type_str, "software") == 0)
-		volume_type = VOL_TYPE_SOFTWARE;
-	else if (strcmp(volume_type_str, "none") == 0)
-		volume_type = VOL_TYPE_NONE;
-	else if (strcmp(volume_type_str, "auto") == 0)
-		volume_type = VOL_TYPE_AUTO;
-	else {
-		fprintf(stderr, "Invalid volume control type '%s'. Try '%s --help' for more information.\n", volume_type_str, argv[0]);
-		return EXIT_FAILURE;
-	}
 	if (volume_type == VOL_TYPE_NONE || volume_type == VOL_TYPE_SOFTWARE)
 		mixer_device = NULL;
 
@@ -1198,26 +1203,26 @@ int main(int argc, char *argv[]) {
 		for (i = 0; i < ba_addrs_count; i++, tmp += 19)
 			ba2str(&ba_addrs[i], stpcpy(tmp, ", "));
 
-		char index_str[sizeof(",NNN")];
-		snprintf(index_str, sizeof(index_str), ",%u", mixer_elem_index % 1000);
+		char mixer_element[128] = "(not used)";
+		if (mixer_device != NULL)
+			snprintf(mixer_element, sizeof(mixer_element), "'%s',%u",
+					mixer_elem_name, mixer_elem_index);
+
 		info("Selected configuration:\n"
 				"  BlueALSA service: %s\n"
 				"  ALSA PCM device: %s\n"
 				"  ALSA PCM buffer time: %u us\n"
 				"  ALSA PCM period time: %u us\n"
 				"  Volume control type: %s\n"
 				"  ALSA mixer device: %s\n"
-				"  ALSA mixer element: %s%s%s%s\n"
+				"  ALSA mixer element: %s\n"
 				"  Bluetooth device(s): %s\n"
 				"  Profile: %s",
 				dbus_ba_service,
 				pcm_device, pcm_buffer_time, pcm_period_time,
 				volume_type_str,
 				mixer_device ? mixer_device : "(not used)",
-				mixer_device ? "'" : "",
-				mixer_device ? mixer_elem_name : "(not used)",
-				mixer_device ? "'" : "",
-				mixer_device ? index_str : "",
+				mixer_element,
 				ba_addr_any ? "ANY" : &ba_str[2],
 				ba_profile_a2dp ? "A2DP" : "SCO");