Improve CPU throttle lines in TinyPilot logs #1543
Conversation
Automated comment from CodeApprove ➜👀 @cghague it's your turn, please take a look |
Automated comment from CodeApprove ➜⏳ @jdeanwallace please review this Pull Request |
There was a problem hiding this comment.
Automated comment from CodeApprove ➜
⏳ Approval Pending (2 unresolved comments)
Approval will be granted automatically when all comments are resolved
LGTM, thanks!
In: Discussion
With regards to consistency, #1489 suggests:
CPU throttled since boot: No
However, I think the current style of the rest of the logs is for the value after the colon to be lowercase. For example:
TinyPilot state
Read-only filesystem: off
SSH access: enabled
Mouse jiggler: disabled
Can we use a lowercase "yes"/"no"?
In: debian-pkg/opt/tinypilot-privileged/scripts/collect-debug-logs:
> Line 139
if vcgencmd get_throttled | grep -q "0x0" ; then
I'm not sure what all the possible outputs of vcgencmd get_throttled can be, but just to be safe can we make sure that grep matches the entire line exactly?
Something like:
if vcgencmd get_throttled | grep --line-regexp -q '0x0' ; then👀 @cghague it's your turn please take a look
There was a problem hiding this comment.
Automated comment from CodeApprove ➜
⏳ Approval Pending (3 unresolved comments)
Approval will be granted automatically when all comments are resolved
In: debian-pkg/opt/tinypilot-privileged/scripts/collect-debug-logs:
> Line 137
printf "%s\n\n" "$(vcgencmd get_throttled)" >> "${LOG_FILE}"
Also, can we preserve the extra newline?
Before:
After:
👀 @cghague it's your turn please take a look
There was a problem hiding this comment.
Automated comment from CodeApprove ➜
In: Discussion
Resolved
In: debian-pkg/opt/tinypilot-privileged/scripts/collect-debug-logs:
Great point! Resolved.
In: debian-pkg/opt/tinypilot-privileged/scripts/collect-debug-logs:
> Line 137
printf "%s\n\n" "$(vcgencmd get_throttled)" >> "${LOG_FILE}"
Resolved
There was a problem hiding this comment.
Automated comment from CodeApprove ➜
Approved: I have approved this change on CodeApprove and all of my comments have been resolved.
Resolves #1489
Parses the output of
vcgencmd get_throttledinto a simple “yes” or “no” result.Decoding the output of
vcgencmd get_throttledWe could decode the output of
vcgencmd get_throttledto get more details, but for our purposes we simply want to detect that nothing relating to throttling has occurred since the device was booted, a situation which is indicated by an output value of0x0.