Rework read_stats output formatting

AdnanHodzic / auto-cpufreq

Automatic CPU speed & power optimizer for Linux

https://foolcontrol.org/?p=4603

GNU Lesser General Public License v3.0

5.32k stars 259 forks source link

Rework read_stats output formatting #703

Open dementive opened 1 month ago

dementive commented 1 month ago

Makes the output of the --stats argument easier to read.

Formatted the output of each core properly into a table, the spacing was slightly off before.
Colored the headers of each table column in green for easier visual parsing
Colored the temperature reading in red for any core that is above 85 degrees so it's easy to quickly tell if something is going wrong. Might want to adjust the number it is pretty arbitrary.

This is kind of jank but only because it was already jank to begin with. The reading and writing of stats as a whole should probably be refactored to use a more convenient interface but it works fine as it is now so I'm not sure if it's actually necessary.

Old Output: old_output

New Output: new_output

shadeyg56 commented 1 month ago

Personally, I like this change and do think it looks much nicer, so I'm approving this, but leaving it up to @AdnanHodzic to merge this, in case he wants some changes.

The reading and writing of stats as a whole should probably be refactored to use a more convenient interface but it works fine as it is now so I'm not sure if it's actually necessary

I agree with this as well. I think taking over stdout for the stats is a bit unnecessary. Having the stats written to a file and read from there makes sense to me but doing it through stdout is weird and can be confusing at times. I think a good solution would be creating a class similar to Logging that can handle stats writing/reading without stopping up stdout, which I'm down to start working on a PR for.

dementive commented 1 month ago

I just tried to add the auto-cpufreq" is about to refresh ... part to the bottom but I can't get it working for some reason. It is currently already printing the about to refresh text but it is printing it at the top of the next refresh instead of the bottom of the last refresh and because of the way the stdout stuff works I don't see a way to fix it but im probably just missing something. Adding it to the bottom of the for loop didn't work and checking for it inside the for loop also didn't work...

shadeyg56 commented 1 month ago

Yeah it might just be best to merge this into its own branch since I'm in the midst of the stats rewrite right now. Then I can include your changes with mine and add you as a co-author

dementive commented 1 month ago

I agree, no need to merge this a stats rework will be much better, this code is pretty weird anyway because of how the stats currently work. You can just grab the parts you need out of this and include them in your rework its probably not necessary to merge it.

AdnanHodzic commented 1 month ago

Okay, then let's leave this PR open until PR with @shadeyg56 changes are merged, or even feel free to close it @dementive and create a new PR once ready? Up to you ...