Repository navigation
test-case: refine the test script and fix the shellcheck warnings - #316
Conversation
marc-hb
left a comment
marc-hb
left a comment
There was a problem hiding this comment.
Please use check-fw-echo-reference.sh in your commit subject to be more specific
| sleep 1 | ||
| dlogc "alsabat -C $cp_dev -c $channel -f $fmt -r $rate -F $frequency" | ||
| alsabat -C "$cp_dev" -c "$channel" -f "$fmt" -r "$rate" -F "$frequency" | ||
| if ! alsabat -C "$cp_dev" -c "$channel" -f "$fmt" -r "$rate" -F "$frequency"; then |
There was a problem hiding this comment.
In general I think this style is better:
alsabat -C ... || {
find ...
die "test X failed because..."
}there are a couple reasons:
- The fewer negations the better.
- You cannot copy/paste
!commands to the interactive command line (try it) doIt || diereads more like error handling fordoItand less likedoItis a test
| __upload_wav_file | ||
| exit 1 | ||
| if ! timeout 2 bash -c wait $alsabatPID; then | ||
| kill -9 $alsabatPID |
There was a problem hiding this comment.
If alsabat should have stopped and it didn't then it is a test failure and we should die "test failed because..." here.
| if [[ "$?" != "0" ]]; then | ||
| __upload_wav_file | ||
| exit 1 | ||
| if ! timeout 2 bash -c wait $alsabatPID; then |
There was a problem hiding this comment.
This is already a /bin/bash script, why is bash -c needed here?
There was a problem hiding this comment.
I tried timeout 2 wait $pid directly, but failed with "timeout: failed to run command ‘wait’: No such file or directory"
but looks like with bash -c, it can only fix the error, but it cannot return error code when $pid is timed out.
so I changed the way to check it.
| alsabat -P "$pb_dev" --standalone -c "$channel" -f "$fmt" -r "$rate" -n "$frames" -F "$frequency" & | ||
| alsabat -P "$pb_dev" --standalone -c "$channel" -f "$fmt" -r "$rate" -n "$frames" -F "$frequency" & alsabatPID=$! | ||
| # playback may have low latency, add 0.5 second delay to aviod recording zero at beginning. | ||
| sleep 1 |
There was a problem hiding this comment.
The comment says 0.5 but the code says 1.
Did you mean "high" latency?
There was a problem hiding this comment.
it should be 1 second delay.
| @@ -84,17 +75,20 @@ do | |||
| fi | |||
There was a problem hiding this comment.
Shouldn't this print a warning?
ec837fb to
fc149e1
Compare
fc149e1 to
b73c764
Compare
| dlogc "alsabat -P $pb_dev --standalone -c $channel -f $fmt -r $rate -n $frames -F $frequency" | ||
| alsabat -P "$pb_dev" --standalone -c "$channel" -f "$fmt" -r "$rate" -n "$frames" -F "$frequency" & | ||
| # playback may have low latency, add 0.5 second delay to aviod recording zero at beginning. | ||
| alsabat -P "$pb_dev" --standalone -c "$channel" -f "$fmt" -r "$rate" -n "$frames" -F "$frequency" &\ |
There was a problem hiding this comment.
You don't need the backslash. I remember that the syntax for & is the same as for ;, I think it helps.
| die "alsabat test failed on pcm: $cp_dev" | ||
| } | ||
|
|
||
| timeout 5 tail --pid=$alsabatPID -f /dev/null || die "Failed to stop alsabat playback" |
There was a problem hiding this comment.
Sorry I gave you some wrong advice earlier: timeout wait $pid does not work because wait is a built-in that works only for direct child processes. tail --pid is an interesting idea but it doesn't not collect the exit status/error code of the alsabat command which is the goal here so tail --pid doesn't not solve the problem. This works:
timeout -k 10 5 alsabat -P "$pb_dev" -... -F "$frequency" & timedAlsabatPID=$!
...
...
wait $timedAlsabatPID || die "alsabat failed..."Unlike my previous advice I tested this logic and it does fail when alsabat fail.
There was a problem hiding this comment.
Thanks @marc-hb for your suggestion, I have updated the code and did some fine-tuning for the time.
b73c764 to
2a45fc0
Compare
refine the check-fw-echo-reference.sh script and fix some shellcheck warnings. Signed-off-by: Zhang Keqiao <keqiao.zhang@linux.intel.com>
review addressed, to merge need to dismiss
4 participants
Refine the check-fw-echo-reference.sh script and fix some shellcheck warnings.
Signed-off-by: Zhang Keqiao keqiao.zhang@linux.intel.com