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.

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

Test-case: Enhance "amixer" card selection #1249

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

harajend
Copy link
Contributor

Added Enhancement for "amixer" card selection: support card identification by name

@harajend harajend requested a review from a team as a code owner January 13, 2025 05:56
@sofci
Copy link
Collaborator

sofci commented Jan 13, 2025

Can one of the admins verify this patch?

reply test this please to run this test once

Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

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

The commit message is empty.

Avoid vague words like "update" or "Enhance". Try to be as specific as possible, for instance "select the first card by name because it's not always numbered 0" (assuming I'm guessing correctly)

Note the review already started in #1245.

test-case/verify-kernel-boot-log.sh Outdated Show resolved Hide resolved
test-case/verify-kernel-boot-log.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Code good, but git commit message needs to follow repository style (and have a signed off).

test-case/verify-kernel-boot-log.sh Outdated Show resolved Hide resolved
@harajend harajend force-pushed the fix/verify-kernel-boot-log branch 2 times, most recently from fba0b90 to 9969f61 Compare January 15, 2025 11:49
Copy link
Contributor

@kv2019i kv2019i left a comment

Choose a reason for hiding this comment

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

Thanks @harajend !

Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

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

The commit message is still confusing. I think it should say:

Search for DMIC on the first card found instead of card 0

Avoid vague words like "enhance". This is a bug FIX that makes the test pass, so it's not just an "enhancement".

test-case/verify-kernel-boot-log.sh Outdated Show resolved Hide resolved
test-case/verify-kernel-boot-log.sh Outdated Show resolved Hide resolved
Select the first card by name because it is not always numbered 0

Signed-off-by: Hariprasad, Rajendra <[email protected]>
@harajend harajend force-pushed the fix/verify-kernel-boot-log branch from 9969f61 to a4dc166 Compare January 16, 2025 18:55
@harajend harajend requested a review from marc-hb January 16, 2025 18:57
@marc-hb
Copy link
Collaborator

marc-hb commented Jan 16, 2025

Again, no Jenkins testing :-(

@@ -114,8 +114,10 @@ dmic_switch_present()

(set -x
# name= is hardcoded in /usr/share/alsa/ucm2/*
# - https://github.com/thesofproject/sof-test/issues/1244
first_card_name=$(aplay -l | awk 'NR == 2 {print $3}')
Copy link
Contributor

Choose a reason for hiding this comment

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

you could do the same with arecord and probably verify that the name starts with sof

@kv2019i kv2019i merged commit d4b799a into thesofproject:main Jan 17, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants