Skip to content

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.

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).

@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".

Select the first card by name because it is not always numbered 0

Signed-off-by: Hariprasad, Rajendra <hariprasad.rajendra@intel.com>
@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 :-(

(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