Skip to content

Regex fix for issue #5 #6

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

Merged
merged 2 commits into from
Jan 28, 2023
Merged

Regex fix for issue #5 #6

merged 2 commits into from
Jan 28, 2023

Conversation

blipk
Copy link
Contributor

@blipk blipk commented Jan 28, 2023

There was a couple of issues with the updated regex from the last PR.

I've updated it to fix issue #5 and it should catch some other edge cases as well

Confirmed fix for provided output on #5:
image

A better fix may be to use getTempInfo_json instead, then there doesn't have to be any regex involved, you just have to iterate through the devices and then its sensors to see which device has a sensor labelled "CPU", and filter zeroed values or such like.

I'm not sure if the slight performance hit from having to process json will make any difference or not. If you would like me to submit a PR for that I could.

@blipk blipk mentioned this pull request Jan 28, 2023
Copy link
Owner

@zocker-160 zocker-160 left a comment

Choose a reason for hiding this comment

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

LGTM thank you

@zocker-160
Copy link
Owner

A better fix may be to use getTempInfo_json instead, then there doesn't have to be any regex involved, you just have to iterate through the devices and then its sensors to see which device has a sensor labelled "CPU", and filter zeroed values or such like.

Yes would probably be better, I don't think performance is an issue here, but I personally would leave it as is for now and revisit the json parsing when I do my fan curve implementation, since then I will need a better CPU temp reading anyway.

Currently the CPU temp display is more or less gimmicky anyway as it does not actually do anything regarding the fan.

@zocker-160 zocker-160 merged commit 618f989 into zocker-160:master Jan 28, 2023
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