Uh oh!
There was an error while loading. Please reload this page.
Updated check-sof-logger.sh and verify-kernel-boot-log.sh - #1245
Updated check-sof-logger.sh and verify-kernel-boot-log.sh#1245harajend wants to merge 2 commits into
Conversation
sofci
commented
Dec 17, 2024
Can one of the admins verify this patch?
|
marc-hb
commented
Dec 17, 2024
test this please |
There was a problem hiding this comment.
Are there any other possibilities besides these two? @kv2019i , @ranj063 , @bardliao can you answer?
The commit message must link to the corresponding bug, e.g. "Fixes issue #1243"
The commit title and message must describe the change much precisely. For instance:
check-sof-logger: remove wildcard from sof_alsa_card_found(). The title has very little space, so don't use words like "update": every commit is an update.
See past commit message examples in the git log.
Here are some good references, most of this applies to any open-source project so please spend some time and take a look
https://docs.zephyrproject.org/latest/contribute/guidelines.html#commit-guidelines
https://wiki.openstack.org/wiki/GitCommitMessages
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#write-a-meaningful-commit-message
https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
Uh oh!
There was an error while loading. Please reload this page.
marc-hb
commented
Dec 17, 2024
Answer: it is not enough. |
marc-hb
commented
Dec 18, 2024
Simpler and probably good enough: sof_alsa_card_found()
{
foriin /proc/asound/sof*/id;doiftest -e "$i";thenreturn 0;fidonereturn 1
} |
marc-hb
commented
Jan 2, 2025
You need to use Also, see advice above for a useful commit message. |
marc-hb
commented
Jan 2, 2025
Jenkins did not test this at all :-( |
Updated the test script with suggested changes Signed-off-by: harajend <hariprasad.rajendra@intel.com> Updated check-sof-logger.sh Updated SOF-specific codecs since the sof* regex also lists sofprobes. Signed-off-by: harajend <hariprasad.rajendra@intel.com>
marc-hb
commented
Jan 6, 2025
Still not running :-( |
Jenkins still not running; pushing new versions does not make any difference. Something is wrong on the Jenkins side. cc: @kv2019i
Still not done. |
By default card 0 is selected so i added dynamic selection for amixer card Signed-off-by: harajend <hariprasad.rajendra@intel.com>
marc-hb
commented
Jan 9, 2025
There is still no testing. Squashing looks good but the commit message of 212a420 is still a mess: Don't say "update", every commit is an update. Just describe the change (once). |
kv2019i
left a comment
There was a problem hiding this comment.
Mostly good, but some comments inline.
| # This returns a non-zero error status on failure | ||
| amixer cget name='Dmic0 Capture Switch' | ||
| switch=$(aplay -l | head -2 | tail -1 | awk '{print $3}') | ||
| amixer -c "$switch" cget name='Dmic0 Capture Switch' |
There was a problem hiding this comment.
Hmm, doesn't this still select a fixed card?
E.g.
~$ aplay -l
**** List of PLAYBACK Hardware Devices ****
card 0: sofhdadsp [sof-hda-dsp], device 0: HDA Analog (*) []
Subdevices: 1/1
Subdevice #0: subdevice #0
... you'd get the same with "amixer -c 0" gets us.
There was a problem hiding this comment.
Select a card using its name, such as "sofsoundwire", instead of selecting it using numbers
aplay -l | head -2 | tail -1 | awk '{print $3}'sofsoundwire
Command shall be like: amixer -c "sofsoundwire" cget name='Dmic0 Capture Switch'
There was a problem hiding this comment.
@harajend But "head -2 | tail -1" will always pick the first card, so this is same as passing "amixer -c 0", right?
There was a problem hiding this comment.
awk is a "big boy" and can select lines itself. It's shorter AND more readable:
switch=$(aplay -l | awk 'NR == 2 {print $3}')amixer -c 0 looks even better? I'm only simplifying code but I don't know if it is correct.
There was a problem hiding this comment.
@harajend But "head -2 | tail -1" will always pick the first card, so this is same as passing "amixer -c 0", right?
Yes, On functionality front both are same. Here I thought to pass "name/identifier" instead of "index number"
| @@ -71,7 +71,10 @@ sof_alsa_card_found() | |||
| # note: assumes SOF card names to start with "sof", e.g. | |||
There was a problem hiding this comment.
Change is good, but the commit message seems wrong. There are two signed offs. Can you amend the message and repush?
marc-hb
commented
Jan 11, 2025
Mmm.... Jenkins just started running in unrelated #1247... Let's try something.... |
marc-hb
commented
Jan 11, 2025
test this please |
marc-hb
commented
Jan 11, 2025
Jenkins is still not running :-( @harajend please try closing this one and re-submitting in a new PR? |
Updated SOF-specific codecs since the sof* regex also lists sofprobes.