Skip to content

multiple-pipeline-capture: simplify func_error_exit() #523

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

Jump to bottom
Merged
marc-hb merged 1 commit into from
Nov 24, 2020
Merged

multiple-pipeline-capture: simplify func_error_exit() #523

marc-hb merged 1 commit into from
Nov 24, 2020

Conversation

@marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Nov 18, 2020

As suggested by Chao in #521

Also remove no-op exit $? on the last line.

Signed-off-by: Marc Herbert marc.herbert@intel.com

@marc-hb marc-hb mentioned this pull request Nov 18, 2020
Merged
@marc-hb marc-hb marked this pull request as ready for review November 18, 2020 21:09
@marc-hb marc-hb requested a review from a team as a code owner November 18, 2020 21:09
@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 18, 2020

https://sof-ci.01.org/softestpr/PR523/build407/devicetest/ had:

  • jf-icl-rvp-hda-2/boot_failed
  • One aplay: pcm_write:2061: write error: Input/output error on one platform in a different test.

None can possibly come from this PR.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 18, 2020

No new shellcheck warning in https://travis-ci.org/github/thesofproject/sof-test/jobs/744366328

aiChaoSONG
aiChaoSONG previously approved these changes Nov 19, 2020
View reviewed changes
Copy link

@aiChaoSONG aiChaoSONG left a comment

Choose a reason for hiding this comment

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

LGTM

@marc-hb marc-hb marked this pull request as draft November 19, 2020 06:36
@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 19, 2020

Let's not merge this yet because it diverges multiple-pipeline-capture from multiple-pipeline-playback again while #525 tries to make them identical again.

@aiChaoSONG
Copy link

aiChaoSONG commented Nov 20, 2020

@marc-hb I think we have to merge this and #525 to make the two multiple-pipeline-* 99% identical, or the code of func_error_exit() diverges in two scripts.

@aiChaoSONG aiChaoSONG marked this pull request as ready for review November 20, 2020 01:58
@aiChaoSONG
Copy link

aiChaoSONG commented Nov 23, 2020

Merge this to make multiple-pipeline-playback.sh and multiple-pipeline-capture.sh 99.9% identical. Then we can merge the scripts into one.

@aiChaoSONG
Copy link

aiChaoSONG commented Nov 23, 2020

SOFCI TEST

@marc-hb marc-hb marked this pull request as draft November 23, 2020 17:05
@marc-hb
multiple-pipeline-capture: simplify func_error_exit()
de7c2eb 
As suggested by Chao in thesofproject#521

Also remove no-op exit $? on the last line.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 23, 2020

No new warning in https://travis-ci.org/github/thesofproject/sof-test/jobs/744366328

https://sof-ci.01.org/softestpr/PR523/build422/devicetest is all green.

@marc-hb marc-hb marked this pull request as ready for review November 23, 2020 22:46
@marc-hb marc-hb requested a review from aiChaoSONG November 23, 2020 22:46
@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 23, 2020

Merge this to make multiple-pipeline-playback.sh and multiple-pipeline-capture.sh 99.9% identical.

In "synchronization" commit 310fed5 I made the same change in multiple-pipeline-playback.sh before this #523 was merged. This made multiple-pipeline-playback.sh ahead of multiple-pipeline-capture.sh, sorry for the confusion.

@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 24, 2020

The only failure in https://sof-ci.01.org/softestpr/PR523/build424/devicetest/?model=BSW_CYN_MAX98090&testcase=multiple-pipeline-capture looks like unrelated thesofproject/sof#3462

marc-hb added a commit to marc-hb/sof-test that referenced this pull request Nov 24, 2020
@marc-hb
multiple-pipeline-playback: remove dmesg -C call
bfc7128 
This was apparently missed in thesofproject#529

Together with thesofproject#523 this makes it identical to multiple-pipeline-capture.sh
again.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb marc-hb mentioned this pull request Nov 24, 2020
Merged
aiChaoSONG
aiChaoSONG approved these changes Nov 24, 2020
View reviewed changes
Copy link

@aiChaoSONG aiChaoSONG left a comment

Choose a reason for hiding this comment

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

LGTM

@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 24, 2020

Small, approved change which has been in review for 6 days, merging.

@marc-hb marc-hb merged commit d01ff1a into thesofproject:master Nov 24, 2020
@marc-hb marc-hb deleted the multi-exit branch November 24, 2020 01:54
@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 24, 2020

Now we need #531, new divergence (accidentally?) introduced by @xiulipan in #529

xiulipan pushed a commit that referenced this pull request Nov 24, 2020
@marc-hb @xiulipan
multiple-pipeline-playback: remove dmesg -C call
f105fc1 
This was apparently missed in #529

Together with #523 this makes it identical to multiple-pipeline-capture.sh
again.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb
Copy link
Collaborator Author

marc-hb commented Nov 24, 2020

Now we need #531, new divergence (accidentally?) introduced by @xiulipan in #529

Thanks @xiulipan for the quick #531 review. Deduplication submitted in #532, not ready for merge today considering the other changes already merged but 100% ready for review, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@aiChaoSONG aiChaoSONG aiChaoSONG approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@marc-hb @aiChaoSONG