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.

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

lib.sh: disable logging with SOF_LOGGING=none env or missing .ldc #813

Merged
merged 1 commit into from
Dec 2, 2021
Merged

lib.sh: disable logging with SOF_LOGGING=none env or missing .ldc #813

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions case-lib/lib.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ fake_kern_error()

}

# Prints the .ldc file found on stdout, errors on stderr.
find_ldc_file()
{
local ldcFile
Expand All @@ -87,6 +88,7 @@ find_ldc_file()
# and then on the standard location.
if [ -n "$SOFLDC" ]; then
ldcFile="$SOFLDC"
>&2 dlogi "SOFLDC=${SOFLDC} overriding default locations"
else
local platf; platf=$(sof-dump-status.py -p) || {
>&2 dloge "Failed to query platform with sof-dump-status.py"
Expand All @@ -98,7 +100,7 @@ find_ldc_file()
fi

[[ -e "$ldcFile" ]] || {
>&2 dloge "LDC file $ldcFile not found, check the SOFLDC environment variable or copy your sof-*.ldc to /etc/sof"
>&2 dlogi "LDC file $ldcFile not found"
return 1
}
printf '%s' "$ldcFile"
Expand Down Expand Up @@ -397,7 +399,27 @@ is_zephyr()

logger_disabled()
{
[[ ${OPT_VAL['s']} -eq 0 ]]
local ldcFile
# Some firmware/OS configurations do not support logging.
ldcFile=$(find_ldc_file) || {
dlogi '.ldc dictionary file not found, SOF logs collection disabled'
return 0 # 0 is 'true'
Copy link
Contributor

@greg-intel greg-intel Dec 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure this comment is needed, as the script self documents on the line before. Or the comment can be on the line before the function definition, to clarify the outputs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right, it's overkill. I will remove but only if someone requests some other changes because it does do not much harm either and uses very little real estate.

}

# Disable logging when available...
if [ ${OPT_VAL['s']} -eq 0 ]; then
return 0
fi

# ... across all tests at once.
# In the future we should support SOF_LOGGING=etrace (only), see
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need more clarification. In future we will support only etrace? why? slogger seems useful to me

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See #726 referenced on the next line.

# sof-test#726
if [ "x$SOF_LOGGING" == 'xnone' ]; then
dlogi 'SOF logs collection globally disabled by SOF_LOGGING=none'
return 0
fi

return 1
}

print_module_params()
Expand Down