ClementTsang / bottom

Yet another cross-platform graphical process/system monitor.
https://clementtsang.github.io/bottom
MIT License
10.23k stars 240 forks source link

Temperature Over Time #1366

Open Freyert opened 10 months ago

Freyert commented 10 months ago

Checklist

Describe the feature request

My motivation is that I have issues on my computer where it overheats, and crashes. I'm actually not sure it's overheating though.

I was looking at the code and readme which caused me to notice that there are essentially Widgets with Graphs and Widgets without Graphs.

I also noticed you doing a larger rewrite of the widget system to improve extensibility https://github.com/ClementTsang/bottom/issues/374

I think the core issue why I can't graph temperatures is related to this lack of extensibility.

From my personal exploration:

My expectations would be that I could convert either one of these metrics into an instantaneous table or a graph as suits me. Though I can also understand particular widgets like the process table being different...

ClementTsang commented 10 months ago

Yeah, it's something I wanted to do (I made a comment a while ago in https://github.com/ClementTsang/bottom/issues/205), but haven't had time with other stuff in life.

Freyert commented 7 months ago

@ClementTsang I see! That's helpful to get more context from those other issues.

I think I'll fork and make a mess, but try some things out. If I come up with anything I'll share it back here for your consideration.

Freyert commented 7 months ago

Ok so my main focus area is the fact that all of these widgets for graphs have to implement a different draw function depending on the data they receive.

flowchart TD
    subgraph src/canvas.rs
    A[struct Painter] --> B[impl Painter.draw_data]
    end

    subgraph src/canvas/widgets/cpu_graph.rs
    B --> C[impl Painter.draw_cpu]
    end

    subgraph src/canvas/widgets/mem_graph.rs
    B --> D[impl Painter.draw_memory_graph]
    end

    subgraph src/canvas/widgets/network_graph.rs
    B --> E[impl Painter.draw_network]
    end
Freyert commented 7 months ago

My plan is to invert control here a bit. Instead of a CPU Widget or a Network Widget... We can have a Graph Widget or a Table Widget.

So then where do we get our data from? From things that are Graphable or Tableable. Which now I think is trapped in the widget state.

This is already kind of how it works. crate::canvas::components::{TimeGraph, GraphData} take care of this.

Which really helps me hone in on: "how do I remove all these draw_x functions and just wind up with a draw.

My current hunch is that it's just a poor separation of concerns: Painter should not be figuring out the legends and labels for the data. That should come from the State.


This is tricky. So there's App which has AppWidgetStates. AppWidgetStates just does some real minimal basic stuff. The actual data resides inside the App.converted_data.

From an ergonomics perspective I think I would enjoy having App not be in charge of so much itself, but instead delegate it out to the widgets.

So my near target is to have the widgets own their converted_data somehow, so they can use it to return labels.

I think it would be much easier to get down to a single draw method without much switching if the WidgetState were responsible for the data being displayed in that widget and sending all necessary config to Painter for drawing.

TLDR; there's too much state in my paint!

Freyert commented 7 months ago

I've been exploring more and it seems like separation of concerns is a bit difficult. There are good reasons for everything to be scoped the way it is, but there are ways to separate concerns with a little more added complexity.


One of my big goals is to have the state objects (TempState, NetState) own the data that their widgets need to render.

If we're talking about a Model View Controller framework (which bottom uses?) then the state objects should own all data that the application uses.


However, we also have the concept of the Collector. The Collector runs in a separate thread to collect system metrics on a defined interval.

I first thought it would be good to separate collection to each Widget so that each Widget would have a State and a Collector. However this isn't ideal because Collector collects sysinfo for example and then delivers it to several other widgets.

It would be inefficient at this moment to have each widget have a thread for collecting their info because they would all make redundant calls with sysinfo.


This pattern could be handled by Channels. However we would now need a thread to pull data into the state objects.

collect -> channel_n -> Vec<i32>.

Downside: need thread to pull data out of the channel. Upside: it probably already exists.


Goal: give state ownership of a data channel that the collector can send to and give the collector a reference to transmit to. Just for Temperature, but hopefully if the pattern works well refactor other states to follow.

In the end this will give the states higher authority over their data and we can potentially move the farmer responsibility into the state.

States being responsible for their data allows us to then focus on making widgets be Views instead of a whole MVC architecture themselves.

Models: Battery, Temperature, Network, CPU, Disk Views: Graph, Table

BatteryModel + GraphView = BatteryGraphWidget TemperatureModel + GraphView = TemperatureGraphWidget TemperatureModel + TableView = TemperatureTableWidget

etc.

I would probably also do away with the term Widget all together in favor of Views. Highlighting an architectural pattern in the naming choices will make it much easier to navigate and construct well. Or saying that a widget is a combination of a Model and a View. That could work.