Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

check-pause-resume: handle volume MAX output in expect #931

Closed
wants to merge 1 commit into from
Closed

check-pause-resume: handle volume MAX output in expect #931

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions test-case/check-pause-release-suspend-resume.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ rm -rf /tmp/sof-test.lock
# catch: Evaluate script and trap exceptional returns
# after ms: Ms must be an integer giving a time in milliseconds.
# The command sleeps for ms milliseconds and then returns.
# FIXME: this test was broken by 46dadd0 ("Add mutual exclusion and journalctl logging")
# because it never was compatible with is_subtest()
# FIXME: Need to handle more of aplay/arecord output. Need to apply similar fix with
# check-pause-resume.sh, multiple-pause-resume.sh
Copy link
Member

@plbossart plbossart Jul 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why volume is MAX is separate question

No, that's why this test failed in the first place.
If the ALSA settings are not correct, this PR is not useful.

Fix the ALSA settings first, then we can refine expect scripts.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix the ALSA settings first, then we can refine expect scripts.

As usual, keep the broken ALSA settings in your backpocket to "test the test" locally. A test that can't fail is useless (green failure).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why volume is MAX is separate question

No, that's why this test failed in the first place.

I said suddenly seeing Volume MAX is sperate issue, mostly it is due to alsa settings. This PR doesn't fix that.

If the ALSA settings are not correct, this PR is not useful.

Fix the ALSA settings first, then we can refine expect scripts.

Agreed, that's why I restored alsa setting from working device from day 1, although I see some weird values. I can re-set if @libinyang can confirm.

expect <<AUDIO
spawn $cmd -D $pcm -r $rate -c $channel -f $fmt -vv -i $dummy_file -q
set i 1
Expand Down
22 changes: 20 additions & 2 deletions test-case/check-pause-resume.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,18 +96,21 @@ do
dlogi "Entering expect script with:
$cmd $SOF_ALSA_OPTS $cmd_opts -D $dev -r $rate -c $channel -f $fmt -vv -i $file_name -q"

# NOTE: Purposely we don't handle 'Volume MAX' case. So that the MAX output can fall into
# 'anything else' case and error out.
# FIXME: share this expect script as a common function
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fredoh9 I re-submitted these comments in new #1222. I think all the rest has been superseded by merged #1218. OK to abandon this?

expect <<END
spawn $cmd $SOF_ALSA_OPTS $cmd_opts -D $dev -r $rate -c $channel -f $fmt -vv -i $file_name -q
set i 1
expect {
"*#*+*\%" {
-re "#.*\%\r" {
set sleep_t [expr int([expr rand() * $rnd_range]) + $rnd_min ]
puts "\r(\$i/$repeat_count) Wait for \$sleep_t ms before pause"
send " "
after \$sleep_t
exp_continue
}
"*PAUSE*" {
"=== PAUSE ===" {
set sleep_t [expr int([expr rand() * $rnd_range]) + $rnd_min ]
puts "\r(\$i/$repeat_count) Wait for \$sleep_t ms before resume"
send " "
Expand All @@ -116,13 +119,28 @@ expect {
if { \$i > $repeat_count } { exit 0 }
exp_continue
}
-re ".*\:.*\n"|"Hardware PCM card.*\n" {
puts "\rexpect ignore dump-hw-params"
after 10
exp_continue
}
default {
puts "\rError: timeout or eof detected, exit 1"
exit 1
}
# this is conventional default, anything else hit here
"^?*\n" {
puts "\rError: unexpected output detected, exit 1. $expect_out(0,string)"
exit 1
}
}
exit 1
END
ret=$?
#flush the output
echo
if [ $ret -ne 0 ]; then
echo "Expect block return $ret"
func_lib_lsof_error_dump "$snd"
sof-process-kill.sh ||
dlogw "Kill process catch error"
Expand Down
22 changes: 20 additions & 2 deletions test-case/multiple-pause-resume.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,18 +115,22 @@ func_pause_resume_pipeline()
# after ms: Ms must be an integer giving a time in milliseconds.
# The command sleeps for ms milliseconds and then returns.
dlogi "$pcm to command: $cmd -D $dev -r $rate -c $channel -f $fmt -vv -i $file -q"

# NOTE: Purposely we don't handle 'Volume MAX' case. So that the MAX output can fall into
# 'anything else' case and error out.
# FIXME: share this expect script as a common function
expect <<END &
spawn $cmd -D $dev -r $rate -c $channel -f $fmt -vv -i $file -q
set i 1
expect {
"*#*+*\%" {
-re "#.*\%\r" {
set sleep_t [expr int([expr rand() * $rnd_range]) + $rnd_min ]
puts "\r(\$i/$repeat_count) pcm'$pcm' cmd'$cmd' id'$idx': Wait for \$sleep_t ms before pause"
send " "
after \$sleep_t
exp_continue
}
"*PAUSE*" {
"=== PAUSE ===" {
set sleep_t [expr int([expr rand() * $rnd_range]) + $rnd_min ]
puts "\r(\$i/$repeat_count) pcm'$pcm' cmd'$cmd' id'$idx': Wait for \$sleep_t ms before resume"
send " "
Expand All @@ -135,6 +139,20 @@ expect {
if { \$i > $repeat_count } { exit 0 }
exp_continue
}
-re ".*\:.*\n"|"Hardware PCM card.*\n" {
puts "\rexpect ignore dump-hw-params"
after 10
exp_continue
}
default {
puts "\rError: timeout or eof detected, exit 1"
exit 1
}
# this is conventional default, anything else hit here
"^?*\n" {
puts "\rError: unexpected output detected, exit 1. $expect_out(0,string)"
exit 1
}
}
exit 1
END
Expand Down