test-case: add test jack detection playback #1280

arikgreen · 2025-07-07T13:20:08Z

Add a new test case to test jack detection during playback.
Verify jack detection functionality by simulating plugging and unplugging of audio jack using a USB relay switch.
The unplugging and plugging of the jack will be during playback operations to ensure the system can handle jack detection events correctly.
The test will check if the jack detection status is updated correctly when the jack is plugged in and unplugged.

For unplug/plug headset jack is using a USB relay. https://github.com/darrylb123/usbrelay

marc-hb

I didn't look at the code much but there is seems to be some duplication with #1278?

marc-hb · 2025-07-24T00:41:47Z

test-case/test-jack-detection-playback-capture.sh

+# Repeat for both headphone and headset jacks.
+# Repeat for all pipelines.
+
+source "$(dirname "${BASH_SOURCE[0]}")"/../case-lib/lib.sh


Missing this line, see other scripts.

shellcheck source=...

marc-hb · 2025-07-24T00:43:40Z

case-lib/lib.sh

+    local switch_name=$1
+    local state=$2
+    dlogi "Setting usbrelay switch $switch_name to $state."
+    usbrelay "$switch_name=$state" >/dev/null 2>&1 || {


Discarding stderr is still very wrong:

#1278 (comment)

yes, you right Marc I missed it

marc-hb

You are very close to fixing ALL shellcheck warnings!

marc-hb · 2025-07-24T17:10:57Z

test-case/test-jack-detection-playback-capture.sh

+    local expected_switch_state="$2"
+    local switch_state
+
+    switch_state=$(echo -e $(amixer -c 0 contents | grep -i "$switch_name .* *jack" -A 2) | sed -n '1s/^.*values=//p')


Use $SOFCARD instead of hardcoding to 0.

What does the echo -e achieve?

BTW: https://unix.stackexchange.com/questions/65803/why-is-printf-better-than-echo

But better use none if none needed in the first place.

marc-hb · 2025-07-24T17:15:28Z

test-case/test-jack-detection-playback-capture.sh

+    dlogi "Reset - plug jack audio"
+    usbrelay_switch "$relay" 0
+
+    for round in $(seq 1 "$round_cnt")


You can use while or shellcheck disable=SC2034

why should I use while?

Do you never use shellcheck?

Very strange: in https://github.com/thesofproject/sof-test/actions/runs/16506744373/job/46679195194 shellcheck complained like this:

In test-case/test-jack-detection-playback-capture.sh line 108: for round in $(seq 1 "$round_cnt") ^-^ SC2034 (warning): round appears unused. Verify use (or export if used externally).

But now it's gone in newer https://github.com/thesofproject/sof-test/actions/runs/16507901117/job/46683162054?pr=1280

EDIT: maybe that went away when you added shellcheck source=?

(The dubious echo on line 79 is flagged in both)

marc-hb · 2025-07-24T17:16:12Z

test-case/test-jack-detection-playback-capture.sh

+        done
+    done
+
+    sof-kernel-log-check.sh "$KERNEL_CHECKPOINT"


Maybe check after each iteration to "fail fast"?

marc-hb · 2025-07-24T17:17:54Z

test-case/test-jack-detection-playback-capture.sh

+    for round in $(seq 1 "$round_cnt")
+    do
+        for idx in $(seq 0 $((PIPELINE_COUNT - 1)))
+        do


It would be more readable to move this to some new test_one_idx() function (better name wanted), that would avoid a lot of indentation and a lot of distance between the start and end of the for loops.

marc-hb · 2025-07-24T22:32:26Z

case-lib/lib.sh

        die "Error: Unknown format: %s\n"
    fi
-}  
+}


To keep git clean, don't mix pure whitespace changes with code changes; especially not when it's far away in a completely different file.
https://wiki.openstack.org/wiki/GitCommitMessages#Things_to_avoid_when_creating_commits

marc-hb · 2025-07-24T22:36:09Z

case-lib/relay.sh

+    local state=$2
+
+    dlogi "Setting usbrelay switch $switch_name to $state."
+    usbrelay "$switch_name=$state" > /dev/null || {


How verbose is usbrelay, can you share an example of what is getting discarded here?

This is test code; as long as their size stays reasonable, logs files should have as much information as possible. Imagine trying to make sense of test logs captured on a system you don't have access to.

BTW usbrelay has a --quiet option, why not just use it instead of /dev/null ? Only if needed.

ubuntu@gk-ptlh-rvp-05:~$ usbrelay HURTM_1=1 HURTM_1=0 HURTM_2=0

the verbose is unnecessary for us and it is confused because display previous state of the relay.

the verbose is unnecessary for us and it is confused because display previous state of the relay.

so just call it twice then: before and after to log the change is done (as it is at line 35 already)

the current state after switching is verified in this line
[[ "$current_state" == "$state" ]]

marc-hb · 2025-07-24T22:38:20Z

case-lib/relay.sh

+    sleep "$USBRELAY_SETTLE_TIME"
+
+    # Display current state of the switch
+    current_state=$(usbrelay | grep "$switch_name" | awk -F= '{print $2}')


awk has grep built-in.

Suggested change

current_state=$(usbrelay | grep "$switch_name" | awk -F= '{print $2}')

current_state=$(usbrelay | awk -F= "/$switch_name/ {print \$2}")

Or, if you prefer

Suggested change

current_state=$(usbrelay | grep "$switch_name" | awk -F= '{print $2}')

current_state=$(usbrelay | awk -F= "/$switch_name/"' {print $2}')

marc-hb · 2025-07-24T22:41:39Z

test-case/test-jack-detection-playback-capture.sh

+    dlogi "Reset - plug jack audio"
+    usbrelay_switch "$relay" 0
+
+    for round in $(seq 1 "$round_cnt")


Very strange: in https://github.com/thesofproject/sof-test/actions/runs/16506744373/job/46679195194 shellcheck complained like this:

In test-case/test-jack-detection-playback-capture.sh line 108: for round in $(seq 1 "$round_cnt") ^-^ SC2034 (warning): round appears unused. Verify use (or export if used externally).

But now it's gone in newer https://github.com/thesofproject/sof-test/actions/runs/16507901117/job/46683162054?pr=1280

EDIT: maybe that went away when you added shellcheck source=?

(The dubious echo on line 79 is flagged in both)

arikgreen · 2025-07-29T19:49:36Z

Thank you @marc-hb for your review, it was very helpful. I hope the code now meets your requirements.

marc-hb

it was very helpful.

My pleasure.

I hope the code now meets your requirements.

I don't have any requirement; I just try to spot common shell mistakes (after shellcheck caught most of them).

The shell is quirky, everyone keeps making the same mistakes.

marc-hb · 2025-07-29T22:53:20Z

test-case/test-jack-detection-playback-capture.sh

+    }
+
+    kill -9 $pid_playback > /dev/null 2>&1
+    wait $pid_playback 2>/dev/null || true


I hope we already have some code for this, maybe in case-lib/

(every script tries to re-invent this)

marc-hb · 2025-07-29T22:53:34Z

test-case/test-jack-detection-playback-capture.sh

+    sleep $TWO_SECONDS
+
+    # check if the aplay process is still running after unplugging the jack
+    ps -p "$pid_playback" > /dev/null || {


I hope we already have some code for this, maybe in case-lib/

IMO ps -p is more explicit about checking for a process

golowanow · 2025-08-07T06:28:08Z

test-case/test-jack-detection-playback-capture.sh

+
+##
+## Preconditions
+# 1. Runtime PM status is on.


Suggested change

# 1. Runtime PM status is on.

# 1. Runtime PM status is on.

please elaborate what is PM here

PM is power management but this point was required in previous version. Should be removed.

golowanow · 2025-08-07T06:29:08Z

test-case/test-jack-detection-playback-capture.sh

+## Preconditions
+# 1. Runtime PM status is on.
+# 2. aplay (playback) is running.
+# 3. USB relay switch is available and configured.


Can it be any USB relay switch or of some particular type is required ?
How it should be configured ?

It must be same type of USB relay switch
https://github.com/darrylb123/usbrelay
if change type of USB relay switch we will must update the usbrelay_switch() function.

golowanow · 2025-08-07T06:31:28Z

test-case/test-jack-detection-playback-capture.sh

+## Test Description
+# Verify jack detection functionality by simulating plugging and unplugging
+# of audio jack using a USB relay switch.
+# The unplugging and plugging of the jack will be during playback operations


Should the test also try jack detection while no playback is running and its handling is correct in this mode ?

this scenario will test in a different test - test_jack_detection_during_suspend

golowanow · 2025-08-07T06:40:06Z

case-lib/relay.sh

+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2021-2025 Intel Corporation. All rights reserved.
+
+# test-mic_privacy.sh needs to control mic privacy settings (on/off)


Suggested change

# test-mic_privacy.sh needs to control mic privacy settings (on/off)

golowanow · 2025-08-07T06:41:28Z

case-lib/relay.sh

+    # Declare a constant for the relay settle time
+    USBRELAY_SETTLE_TIME=0.5
+
+    local switch_name=$1


does it run with $1 == --debug ?

yes
https://github.com/arikgreen/sof-test/blob/13d9d4e61b71c5601869bc5b66ab78494287182e/test-case/test-jack-detection-playback-capture.sh#L185

golowanow · 2025-08-07T06:42:35Z

case-lib/relay.sh

+    }
+
+    # Declare a constant for the relay settle time
+    USBRELAY_SETTLE_TIME=0.5


Suggested change

USBRELAY_SETTLE_TIME=0.5

local USBRELAY_SETTLE_TIME=0.5

golowanow · 2025-08-07T06:44:57Z

case-lib/relay.sh

+    local state=$2
+
+    dlogi "Setting usbrelay switch $switch_name to $state."
+    usbrelay "$switch_name=$state" > /dev/null || {


the verbose is unnecessary for us and it is confused because display previous state of the relay.

so just call it twice then: before and after to log the change is done (as it is at line 35 already)

golowanow · 2025-08-07T06:51:08Z

test-case/test-jack-detection-playback-capture.sh

+    # Check if usbrelay tool is installed
+    command -v usbrelay || {
+        # If usbrelay package is not installed
+        skip_test "usbrelay command not found. Please install usbrelay package."
+    }
+
+    # display current status of relays
+    usbrelay --debug || die "usbrelay actual state read failed."


looks like a good function at relay.sh

golowanow · 2025-08-07T06:56:50Z

test-case/test-jack-detection-playback-capture.sh

+        for i in $(seq 1 "$loop_cnt")
+        do
+            testing_one_pcm
+            sof-kernel-log-check.sh "$KERNEL_CHECKPOINT"


I think in these two nested loops it should either die on the first errors here, or set new kernel check point on each iteration.

Copilot

Pull Request Overview

This PR adds a new test case to verify jack detection functionality during audio playback operations using a USB relay switch to simulate physical plugging/unplugging of audio jacks.

Adds test script that uses USB relay to simulate jack plug/unplug events during playback
Creates relay utility library for controlling USB relay switches
Updates environment check to include usbrelay package dependency

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
test-case/test-jack-detection-playback-capture.sh	Main test script implementing jack detection testing during playback
env-check.sh	Adds usbrelay package check to environment validation
case-lib/relay.sh	Utility library for controlling USB relay switches

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-03T11:20:59Z

test-case/test-jack-detection-playback-capture.sh

+    # check if the aplay process is still running after unplugging the jack
+    ps -p "$pid_playback" > /dev/null || {
+        func_lib_lsof_error_dump "$snd"
+        die "Playback process terminated unexpectedly after unplugging the jack."


The error message refers to 'unplugging' but this code path is executed after plugging the jack (line 136). The message should say 'after plugging the jack' to match the actual context.

Suggested change

die "Playback process terminated unexpectedly after unplugging the jack."

die "Playback process terminated unexpectedly after plugging the jack."

Copilot · 2025-09-03T11:20:59Z

case-lib/relay.sh

+# param1: --debug
+# param1: switch name
+# param2: switch state


The parameter documentation is inconsistent. Both lines refer to 'param1' but they describe different parameters. The first should be 'param1: --debug' and the second should be 'param2: switch name' or document the overloaded behavior more clearly.

Suggested change

# param1: --debug

# param1: switch name

# param2: switch state

# param1: Either '--debug' to show relay status, or switch name to set state

# param2: switch state (required if param1 is switch name)

redzynix · 2025-09-03T11:11:53Z

case-lib/relay.sh

+usbrelay_switch()
+{
+    [[ "$1" == "--debug" ]] && {
+        dlogi "Debug mode: Current status of all relays is:"


Suggested change

dlogi "Debug mode: Current status of all relays is:"

dlogi "Debug mode: Current status of all relays:"

redzynix · 2025-09-03T11:15:41Z

test-case/test-jack-detection-playback-capture.sh

+#
+# Repeat steps 3-11 for all pcm jack playback devices.
+
+TESTDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)


Suggested change

TESTDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)

TESTDIR=$(realpath -e "$(dirname "${BASH_SOURCE[0]}")/..")

redzynix · 2025-09-03T11:17:32Z

test-case/test-jack-detection-playback-capture.sh

+OPT_NAME['t']='tplg'     OPT_DESC['t']="tplg file, default value is env TPLG: $TPLG"
+OPT_HAS_ARG['t']=1         OPT_VAL['t']="$TPLG"
+
+OPT_NAME['l']='loop'     OPT_DESC['l']='loop count'
+OPT_HAS_ARG['l']=1         OPT_VAL['l']=1
+
+OPT_NAME['s']='sof-logger'   OPT_DESC['s']="Open sof-logger trace the data will store at $LOG_ROOT"
+OPT_HAS_ARG['s']=0             OPT_VAL['s']=1
+
+OPT_NAME['u']='relay'     OPT_DESC['u']='name of usbrelay switch, default value is HURTM_2'
+OPT_HAS_ARG['u']=1         OPT_VAL['u']="HURTM_2"


inconsistent indents between OPTS.

marc-hb · 2025-09-05T02:02:57Z

This has the same relay.sh file as #1278 but it's not added in a separate commit.

The file is added in the separate PR 1280: thesofproject#1280 Signed-off-by: Artur Wilczak <arturx.wilczak@intel.com>

Add a new test case to test jack detection during playback. The test checks all playback Jack PCM devices For unplug/plug headset jack is using a USB relay. https://github.com/darrylb123/usbrelay Signed-off-by: Artur Wilczak <arturx.wilczak@intel.com>

The file is added in the separate PR 1278: thesofproject#1278 Signed-off-by: Artur Wilczak <arturx.wilczak@intel.com>

The file is added in the separate PR 1280: #1280 Signed-off-by: Artur Wilczak <arturx.wilczak@intel.com>

arikgreen requested review from a team, golowanow, lgirdwood and marc-hb as code owners July 7, 2025 13:20

arikgreen force-pushed the awilczax/add-test-jack-detection branch 2 times, most recently from b5952dd to 61a5222 Compare July 14, 2025 21:24

arikgreen force-pushed the awilczax/add-test-jack-detection branch 2 times, most recently from fd37446 to 2859a70 Compare July 23, 2025 09:12

marc-hb reviewed Jul 24, 2025

View reviewed changes

arikgreen force-pushed the awilczax/add-test-jack-detection branch from 2859a70 to 6b859f9 Compare July 24, 2025 12:31

marc-hb reviewed Jul 24, 2025

View reviewed changes

arikgreen force-pushed the awilczax/add-test-jack-detection branch 3 times, most recently from 4fcce37 to 4ecdc5c Compare July 24, 2025 21:08

marc-hb reviewed Jul 24, 2025

View reviewed changes

arikgreen force-pushed the awilczax/add-test-jack-detection branch from 4ecdc5c to 5f74c00 Compare July 29, 2025 19:04

marc-hb reviewed Jul 29, 2025

View reviewed changes

arikgreen force-pushed the awilczax/add-test-jack-detection branch 3 times, most recently from f4090f4 to 13d9d4e Compare August 4, 2025 17:51

golowanow reviewed Aug 7, 2025

View reviewed changes

arikgreen force-pushed the awilczax/add-test-jack-detection branch from 13d9d4e to 739afef Compare August 7, 2025 19:19

arikgreen requested a review from majunkier September 2, 2025 08:37

arikgreen force-pushed the awilczax/add-test-jack-detection branch from 739afef to 4651e82 Compare September 3, 2025 11:06

arikgreen requested review from Copilot and redzynix September 3, 2025 11:20

Copilot AI reviewed Sep 3, 2025

View reviewed changes

redzynix reviewed Sep 3, 2025

View reviewed changes

arikgreen force-pushed the awilczax/add-test-jack-detection branch from 4651e82 to ae2db23 Compare September 4, 2025 12:51

arikgreen force-pushed the awilczax/add-test-jack-detection branch from ae2db23 to 60ed279 Compare September 4, 2025 12:55

redzynix approved these changes Sep 4, 2025

View reviewed changes

arikgreen requested a review from KamilxPaszkiet September 4, 2025 12:56

arikgreen force-pushed the awilczax/add-test-jack-detection branch from 60ed279 to 91dfa5c Compare September 4, 2025 13:10

redzynix approved these changes Sep 4, 2025

View reviewed changes

marc-hb mentioned this pull request Sep 5, 2025

test-case: add new test case for check mic privacy #1278

Merged

KamilxPaszkiet approved these changes Sep 5, 2025

View reviewed changes

arikgreen added a commit to arikgreen/sof-test that referenced this pull request Sep 5, 2025

test-case: add lib file for test-mic-privacy

6ecdb54

The file is added in the separate PR 1280: thesofproject#1280 Signed-off-by: Artur Wilczak <arturx.wilczak@intel.com>

arikgreen requested a review from cgturner1 September 8, 2025 15:06

arikgreen added 2 commits September 8, 2025 17:09

test-case: add lib file for test-mic-privacy

ce0d396

The file is added in the separate PR 1278: thesofproject#1278 Signed-off-by: Artur Wilczak <arturx.wilczak@intel.com>

arikgreen force-pushed the awilczax/add-test-jack-detection branch from 91dfa5c to ce0d396 Compare September 8, 2025 15:11

cgturner1 pushed a commit that referenced this pull request Sep 8, 2025

test-case: add lib file for test-mic-privacy

5e71e20

The file is added in the separate PR 1280: #1280 Signed-off-by: Artur Wilczak <arturx.wilczak@intel.com>

cgturner1 approved these changes Sep 8, 2025

View reviewed changes

cgturner1 merged commit 09130c9 into thesofproject:main Sep 8, 2025
3 of 8 checks passed

	current_state=$(usbrelay \| grep "$switch_name" \| awk -F= '{print $2}')
	current_state=$(usbrelay \| awk -F= "/$switch_name/ {print \$2}")

	die "Playback process terminated unexpectedly after unplugging the jack."
	die "Playback process terminated unexpectedly after plugging the jack."

	dlogi "Debug mode: Current status of all relays is:"
	dlogi "Debug mode: Current status of all relays:"

	TESTDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)
	TESTDIR=$(realpath -e "$(dirname "${BASH_SOURCE[0]}")/..")

test-case: add test jack detection playback #1280

test-case: add test jack detection playback #1280

Uh oh!

Conversation

arikgreen commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marc-hb left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marc-hb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marc-hb Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marc-hb Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arikgreen Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marc-hb Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marc-hb Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arikgreen commented Jul 29, 2025

Uh oh!

marc-hb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

arikgreen commented Jul 7, 2025 •

edited

Loading

marc-hb left a comment •

edited

Loading

marc-hb Jul 24, 2025 •

edited

Loading

marc-hb Jul 24, 2025 •

edited

Loading

arikgreen Aug 7, 2025 •

edited

Loading

marc-hb Jul 24, 2025 •

edited

Loading

marc-hb Jul 24, 2025 •

edited

Loading

redzynix Sep 3, 2025 •

edited

Loading