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

Catch aplay error messages in apause.exp and exit #1226

Merged
merged 3 commits into from
Aug 1, 2024
Merged

Catch aplay error messages in apause.exp and exit #1226

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2e0ebd4
apause.exp: search/replace "recording" with "active"
marc-hb Aug 1, 2024
e5f9248
apause.exp: catch and print error messages and exit immediately
marc-hb Aug 1, 2024
d25ac5b
apause.exp: promote unexpected "PAUSE" feedback from warning to failure
marc-hb Aug 1, 2024
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
42 changes: 25 additions & 17 deletions case-lib/apause.exp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ proc rel_time_ms {} {

proc press_space {} {
global last_space_time
log 2 "Pressing SPACE"
log 1 "Pressing SPACE"
send " "
set last_space_time [rel_time_ms]
log 3 "last_space_time set to $last_space_time"
Expand Down Expand Up @@ -111,8 +111,8 @@ spawn {*}$argv
set start_time_ms [clock milliseconds]; # re-adjust
set last_space_time 0 ; # could not resist that name

# states: recording, pause_requested, paused, recording_requested
set state recording_requested
# states: active, pause_requested, paused, active_requested
set state active_requested

set in_max_burst false
set volume_always_zero true
Expand All @@ -137,8 +137,20 @@ set volume_always_zero true

expect {

# `man re_syntax` claims that Tcl regular expressions are compliant
# with the basic and extended POSIX ones while adding a 3rd,
# extended flavor. It's not clear which flavor `expect -re` uses
# but it's not the basic one.
# Use {} not "" to avoid quoting issues and backslash proliferation.

# When multiple patterns match, first pattern wins.

-nocase -re {error.*\r|PAUSE.*no[[:blank:]]*hw[[:blank:]]*support.*\r} {
Copy link
Member

Choose a reason for hiding this comment

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

laughing at my desk at 8am, no idea what this does haha.

Copy link
Collaborator Author

@marc-hb marc-hb Aug 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It took me only a couple days of practice to become productive in Tcl and expect (without any "stackoverflow garbage in, AI garbage out!"); it's not so bad.

{ } is like the single quotes in shell except they can be nested.

The basic syntax of the expect instruction is just:

expect {
 { pattern1 } { action1 }
 { pattern2 } { action2 }
 ...
}

-nocase -re means the pattern is a case-insensitive regular expression (as opposed to globbing).

That regular expression has itself nothing specific to Tcl.

Thanks for taking a look!

set buffer_with_lf "[cr_to_lf $expect_out(buffer)]"
log 0 "ERROR: $buffer_with_lf"
exit 1
}

Copy link
Member

Choose a reason for hiding this comment

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

just to be clear on the commit message: PAUSE will be disabled unless we have a kernel parameter that keeps it enabled. All CI platforms SHALL keep the pause enabled.

Copy link
Collaborator Author

@marc-hb marc-hb Aug 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

That's what I tried to say in the commit message but more briefly and without getting into Intel specifics. Too briefly? I mean, should I change the commit message or is your comment here enough?

Copy link
Member

Choose a reason for hiding this comment

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

thats' fine, just wanted to check we were aligned.

# Volume xx% or MAX line
#
# When not backpressured by a sleeping (=bad!) Expect process,
Expand Down Expand Up @@ -182,13 +194,13 @@ expect {

switch $state {

recording {
log 2 "Recording volume #... | xx%:\n[cr_to_lf $expect_out(buffer)]"
active {
log 2 "Volume #... | __%:\n[cr_to_lf $expect_out(buffer)]"
exp_continue
}

pause_requested {
log 2 "Volume #... | xx% left after requesting pause:\n[cr_to_lf $expect_out(buffer)]"
log 2 "Volume #... | __% left after requesting pause:\n[cr_to_lf $expect_out(buffer)]"
exp_continue
}

Expand All @@ -198,12 +210,12 @@ expect {
exit 1
}

recording_requested {
# First volume line printed since unpaused; recording successfully started!
set state recording
active_requested {
# First volume line printed since unpaused; stream successfully started!
set state active

set _record_for [rand_min_range $rnd_min $rnd_range]
log 1 "($pauses_counter/$repeat_count) Found volume ### | xx%, recording for $_record_for ms"
log 1 "($pauses_counter/$repeat_count) Found volume ### | __%, active for $_record_for ms"

set _delay [substract_time_since_last_space $_record_for]
after $_delay "press_space; set state pause_requested"
Expand All @@ -225,12 +237,8 @@ expect {

{=== PAUSE ===} {
if {$state != "pause_requested"} {
# TODO: upgrade this to an ERROR if we want to fix pause bugs like
# https://github.com/thesofproject/linux/issues/5109
# As of July 2024, pause is rather being disabled:
# https://github.com/thesofproject/linux/pull/5041
log 0 "WARNING: received == PAUSE == while in state $state! Ignoring."
exp_continue
log 0 "ERROR: received == PAUSE == while in state $state!"
exit 1
}

set state paused
Expand All @@ -241,7 +249,7 @@ expect {
log 1 "($pauses_counter/$repeat_count) Found === PAUSE === , pausing for $_pausing_for ms"

set _delay [substract_time_since_last_space $_pausing_for]
after $_delay "press_space; set state recording_requested"
after $_delay "press_space; set state active_requested"
log 3 "last_space_time=$last_space_time; timer in $_delay"


Expand Down
Loading