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

Add powersave on overheat rules #221

Merged
merged 8 commits into from
Feb 13, 2022
Merged

Add powersave on overheat rules #221

merged 8 commits into from
Feb 13, 2022

Conversation

Camerooooon
Copy link
Collaborator

Right now I'm unsure if this logic is correct

@Camerooooon
Copy link
Collaborator Author

Camerooooon commented Feb 12, 2022

Wait for #222 to merge
222 is merged

@Camerooooon
Copy link
Collaborator Author

Ok, the logic of this needs to be tested. We need to make sure that plugging in the laptop won't override the temperature rule. I don't have my charge so I can't test this right now.

@JakeRoggenbuck
Copy link
Owner

I can test in a bit

@JakeRoggenbuck
Copy link
Owner

Only one thing I would change, commit and should_graph were moved to settings.rs, so they can be removed from daemon_init and struct Daemon, but other than that, looks good!

@Camerooooon
Copy link
Collaborator Author

I'm also making temp threshold configurable.

@JakeRoggenbuck
Copy link
Owner

Smart getting get_highest temp, because sometimes one core might be like wayyyy higher and could get damaged without the system average being too hot. good job!

@JakeRoggenbuck
Copy link
Owner

I'm also making temp threshold configurable.

Yees!!

@JakeRoggenbuck
Copy link
Owner

JakeRoggenbuck commented Feb 12, 2022

oop, you got it!

@JakeRoggenbuck
Copy link
Owner

Awesome, looks great!!

@JakeRoggenbuck
Copy link
Owner

I can test tomorrow morning

@JakeRoggenbuck
Copy link
Owner

fixes #122

@Camerooooon Camerooooon linked an issue Feb 12, 2022 that may be closed by this pull request
@JakeRoggenbuck
Copy link
Owner

The logic looks solid. Great implementation. The only real way to test is to put a heater up to the laptop while opening a million tabs. I will try setting the threshold low, then put my laptop in the freezer, then move it into a space heater while building kernel from source. Wish me luck.

@JakeRoggenbuck
Copy link
Owner

image

@JakeRoggenbuck
Copy link
Owner

it works!
image

@JakeRoggenbuck
Copy link
Owner

@Camerooooon good to merge?

@Camerooooon
Copy link
Collaborator Author

Plugging it in after temp is overheating is going to cause problems. We don't want the governor to switch back to performance when you plug the laptop in.

@JakeRoggenbuck
Copy link
Owner

Plugging it in after temp is overheating is going to cause problems. We don't want the governor to switch back to performance when you plug the laptop in.
Oh, true, hmm. Maybe we should give daemon a state. For example, normal, low_power, overheating, closed, etc.

@Camerooooon
Copy link
Collaborator Author

Plugging it in after temp is overheating is going to cause problems. We don't want the governor to switch back to performance when you plug the laptop in.
Oh, true, hmm. Maybe we should give daemon a state. For example, normal, low_power, overheating, closed, etc.

Good idea! That's a clean way to solve this issue. How about we merge this as is and make that a later PR?

@JakeRoggenbuck
Copy link
Owner

Sure, that would be good. I can make the state.rs and stuff, and then we could work together on adding state everywhere

@JakeRoggenbuck JakeRoggenbuck merged commit 04d4d5a into main Feb 13, 2022
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.

Power save on overheat
2 participants