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

Temperature sensor widget #135

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

unterwegi
Copy link

Cool project, hopefully I can add to that with this new widget type.

Essentially, this new temp widget shows temperature data reported by the internal hardware temperature sensors. To do this, it uses gopsutil/host.

The temp widget is essentially a copy of the top widget, so it displays the current sensor value as a bar graph, difference being that the max value is configurable (default is 100°C). The only annoying thing is figuring out the identifiers for the sensors, but I tried to explain it at least for Linux systems in the README section for the widget.

When I tested the widget, I found one little inefficiency (bug?) with the update interval handling in combination with the top widget (and my new temp widget). The RequiresUpdate function checks if enough time has passed since the widget was last updated based on the widget's lastUpdate time. This one is only updated in the widget render function.

The top (and temp) widgets both return early out of the Update function if the value to display hasn't changed, so the render function is never called in that case (which means lastUpdate is never updated). In the next updateWidgets cycle, the widget's Update function is called immediately again, as the lastUpdate time is still far enough in the past.

For the top widget that is not a big deal as the function calls for CPU and memory usage are pretty cheap. On the other hand, the collection of the temperature sensor data involves some file operations so you notice some CPU usage. It will not grind your machine to a halt, but hey, why waste those cycles if it can be avoided 😄

To fix that, the temp widget just updates the lastUpdate time when the early return condition is met. I thought about adding this line to the top widget as well, but I just wanted to check first if I should do this in the same PR or if you prefer a separate issue/PR for that.

I also noticed that the gopsutil module was quite outdated. Dependabot probably didn't update it because the module path changed to gopsutil/v3, so I updated that as well. The top widget still works the same with that new version.

Some things I considered but left open for the moment:

  1. Temperature data is currently only shown in °C (which is the scale the sensor values are reported in). As the weather widget allows either celsius or fahrenheit as a unit, it might make sense to provide an optional fahrenheit conversion here as well for consistency.
  2. When you add multiple temp widgets, each widget builds its own slice with all temperature sensor values via gopsutil/host (there is only a fetch all sensors function) and searches for its sensor in that during each update, which is a little bit wasteful. I don't know if off-loading the slice creation into a go-routine that provides the data to all widgets makes sense here, but if wanted, I can do so.
  3. Both the top and temp widgets have basically the same bar graph drawing code, which could be refactored out into one drawGraph function.

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.

1 participant