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

LNL alsabat test to support both USB Codec/AudioPlug loopback setup #1231

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ssavati
Copy link

@ssavati ssavati commented Sep 10, 2024

No description provided.

@ssavati ssavati requested a review from a team as a code owner September 10, 2024 11:02
fi

# Check device for AudioPlug Loopback enabled or not for LNL SDW
if [ "$AUDIOPLUG_LOOPBACK" == "true" ] && [ "$MODEL" == "LNLM_SDW_AIOC" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

you probably want to avoid using the MODEL, we can use this test on previous generations as well.

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.

This is a big hack with a lot of ugly hardcoding :-(

I think there is a much simpler, more flexible and more reliable way. Something like this (untested)

usb_audio_found()
{
   aplay -l | grep -q 'USB Audio'
}



if [[ "$pcm_p" =~ CODEC ]] && ! usb_audio_found; then
   dlogi "No USB audio found, discarding pcm_p=$pcm_p"
   unset pcm_p
fi

# <same thing for pcm_c>

# default values for pcm_p
if [ -n "$pcm_p" ]; then
   case $MODEL in
   *HDA)  pcm_p=...;;
    ...)
    
    *) die "No default pcm_p for MODEL=$MODEL"
    esac
   dlogi "Using default value pcm_p=$pcm_p for MODEL=$MODEL"
fi

# <default values for pcm_c>


@marc-hb
Copy link
Collaborator

marc-hb commented Sep 10, 2024

@plbossart
Copy link
Member

BTW do you know why we have all these SKIPs? https://sof-ci.01.org/sofpr/PR9455/build7827/devicetest/index.html?model=LNLM_RVP_HDA&testcase=check-alsabat-nocodec-32bits-599

I am not sure why we have an alsa-bat test that's specific to the nocodec mode, but it makes sense that it would only be applied to nocodec platforms and skipped for both HDaudio and SoundWire platforms.

@plbossart
Copy link
Member

usb_audio_found()
{
   aplay -l | grep -q 'USB Audio'
}

This is also fragile in that it will fail if there's a USB device connected by accident.
I think it's better to rely on environment variables set by the CI setup to understand when we want to use the analog dongle or the USB loopback.

@marc-hb
Copy link
Collaborator

marc-hb commented Sep 11, 2024

This is also fragile in that it will fail if there's a USB device connected by accident.

These loopback setups involve a fair amount of cables and are a bit tedious to install, it's not really something anyone has been connecting "by accident".

I think it's better to rely on environment variables set by the CI setup to understand when we want to use the analog dongle or the USB loopback.

The main issue is: we don't have a reliable way to maintain per-device settings (in fact we don't even have a reliable way to maintain consistent settings either). Unlike hardware configuration "accidents", software configuration accidents have been happening regularly.

Considering it's impossible for a device to be able to test both configurations, merely echoing the hardware configuration in a configuration file manually seems just error-prone with no value.

All this being said, usb_audio_found() can very easily be replaced by TEST_USB_LOOPBACK=true in my sample code. My suggestion was not just about that.

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.

3 participants