Skip to content

Fix CPU temperature reading on Rapsberry Pi - #4

Merged
EnKrypt merged 2 commits into
EnKrypt:masterfrom
philw07:master
Nov 25, 2019
Merged

EnKrypt merged 2 commits into
EnKrypt:masterfrom
philw07:master

Conversation

@philw07

@philw07 philw07 commented Nov 23, 2019 •

Copy link
Copy Markdown
Contributor

This PR changes the cpu temperature reading to use the max temperature provided by systeminformation instead of manually calculating it from the core temperature array. On the RPi, the cores array is actually empty, but the max temperature is provided.
Note that I could only verify the change on an RPi, since on my Windows machine it doesn't return any temperature reading at all.

@EnKrypt
EnKrypt self-requested a review November 24, 2019 04:25
@EnKrypt EnKrypt self-assigned this Nov 24, 2019
@EnKrypt

EnKrypt commented Nov 24, 2019

Copy link
Copy Markdown
Owner

Thanks again for the PR.

I think my comment from the parent issue sums up my thoughts on this.

I think I'll hold off on reviewing this until you can let us know what your plans are with respect to our discussion.

Comment thread backend/src/app.ts
network = await SI.networkStats();
try {
cpuTemp = await SI.cpuTemperature();
if (!cpuTemp.cores.length && cpuTemp.max > 0) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'm wondering if there is a better way to check if the cpuTemp.max is valid in this condition.

The issue is that a temperature of zero degrees, or even negative degrees is technically a perfectly valid temperature.

However, even the module we're using sets the default to -1°C, so we may not have a choice in this matter. If the module can't get the max value, it'll still return -1°C which is sadly a valid temperature.

Let's just hope for now that no one using Doppler on a Raspberry Pi is experimenting with LN2 cooling, or lives in the Artic Circle.

@EnKrypt
EnKrypt merged commit 0f46317 into EnKrypt:master Nov 25, 2019
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.

2 participants