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 disk io stats to EWW_DISK #1267

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

Conversation

bkueng
Copy link

@bkueng bkueng commented Jan 11, 2025

Very cool project.

Description

This updates system_stats and exposes read and written to EWW_DISK, allowing to monitor disk io stats.

Checklist

Please make sure you can check all the boxes that apply to this PR.

  • All widgets I've added are correctly documented.
  • I added my changes to CHANGELOG.md, if appropriate.
  • The documentation in the docs/content/main directory has been adjusted to reflect my changes.
  • I used cargo fmt to automatically format all code before committing

Copy link
Contributor

@w-lfchen w-lfchen left a comment

Choose a reason for hiding this comment

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

this only works after running cargo update --recursive sysinfo, which alters the lockfile. otherwise, there are dependency resolution errors.

components
.iter()
.map(|c| {
(
c.label().to_uppercase().replace(' ', "_"),
// It is common for temperatures to report a non-numeric value.
// Tolerate it by serializing it as the string "null"
c.temperature().to_string().replace("NaN", "\"null\""),
c.temperature().unwrap_or(NAN).to_string().replace("NaN", "\"null\""),
Copy link
Contributor

Choose a reason for hiding this comment

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

this should leverage the power of the type system.
consider this, from #1217:

Suggested change
c.temperature().unwrap_or(NAN).to_string().replace("NaN", "\"null\""),
c.temperature().map_or_else(|| String::from("\"null\""), |temp| temp.to_string()),

"read": usage.read_bytes as f64 / elapsed.as_secs_f64(),
"written": usage.written_bytes as f64 / elapsed.as_secs_f64(),
Copy link
Contributor

Choose a reason for hiding this comment

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

it might be unclear which unit this is. consider documenting it

Comment on lines 30 to 31
let (ref mut last_refresh, ref mut disks) = &mut *DISKS.lock().unwrap();
disks.refresh_specifics(true, DiskRefreshKind::everything());
Copy link
Contributor

Choose a reason for hiding this comment

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

cleanup, nothing big

Suggested change
let (ref mut last_refresh, ref mut disks) = &mut *DISKS.lock().unwrap();
disks.refresh_specifics(true, DiskRefreshKind::everything());
let (ref mut last_refresh, ref mut disks) = *DISKS.lock().unwrap();
disks.refresh(true);

@bkueng
Copy link
Author

bkueng commented Jan 14, 2025

Thanks for the review, I addressed your comments.

Copy link
Contributor

@w-lfchen w-lfchen left a comment

Choose a reason for hiding this comment

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

looks good now, thank you! <3
(only did some rough testing but should be fine)

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