9
\$\begingroup\$

I come from a C background and am trying to learn Rust. I wrote my first Rust program, to check the battery level on my laptop and warn me if it's low/critical.

It works fine, but I wrote it in a way I'm familiar with and was wondering if I'm missing out on some features of the Rust language that could make this more portable, more optimal, or safer.

extern crate notify_rust;    
use notify_rust::Notification;
use notify_rust::NotificationUrgency;

use std::fs::File;
use std::io::prelude::*;

static CHARGE_NOW: &str =  "/sys/class/power_supply/BAT0/charge_now";
static CHARGE_FULL: &str = "/sys/class/power_supply/BAT0/charge_full";
static STATUS: &str = "/sys/class/power_supply/BAT0/status";
static CHARGING_STR: &str = "Charging";

const CHARGE_WARN: f32 = 15.0;
const CHARGE_CRIT: f32 = 5.0;
const NOTIFICATION_TIMEOUT: i32 = 4000;

fn main() {

    /* If we're charging, just exit */
    if get_str_from_file(STATUS).trim() == CHARGING_STR {
        std::process::exit(0);
    }

    let charge_level: f32 = (get_float_from_file(CHARGE_NOW) /
                             get_float_from_file(CHARGE_FULL)) * 100.0;

    if charge_level < CHARGE_WARN {
        Notification::new()
            .summary("Battery level low!")
            .body(&format!("Battery level: {0:.1$}%", charge_level, 2))
            .icon("dialog-information")
            .urgency(
                if charge_level < CHARGE_CRIT { 
                    NotificationUrgency::Critical
                } else { 
                    NotificationUrgency::Normal
                }
            )
            .timeout(NOTIFICATION_TIMEOUT)
            .show().unwrap();
    }
}

fn get_str_from_file(file_path: &str) -> String {
    let mut file = File::open(file_path)
        .expect("file not found");
    let mut ret = String::new();
    file.read_to_string(&mut ret)
        .expect("failed to read file");

    ret
}

fn get_float_from_file(file_path: &str) -> f32 {
    get_str_from_file(file_path).trim().parse::<f32>().unwrap()
}

Used crate: notify_rust.

\$\endgroup\$
4
\$\begingroup\$

Unwrapping options is probably fine for this simple little app that only you’re using, but it dumps out a panic if the result is None. I like your use of expect better. That can give a nicer error message to the user. Either way, it’d be good to learn the more idiomatic way of dealing with options.

match x {
    None => foo,
    Some => bar
}

The function at the end of your file should probably be returning an Option that can be handled in your main logic.

\$\endgroup\$
6
\$\begingroup\$

You could replace your get_str_from_file function with the standard read_to_string function:

For instance like so:

use std::fs;

if fs::read_to_string(STATUS).unwrap().trim() == CHARGING_STR) {
    return; //return should be equivalent to std::process::exit(0)
}

This is your implementation:

fn get_str_from_file(file_path: &str) -> String {
    let mut file = File::open(file_path)
        .expect("file not found");
    let mut ret = String::new();
    file.read_to_string(&mut ret)
        .expect("failed to read file");

    ret
}

This is the implementation of std::fs::read_to_string

pub fn read_to_string<P: AsRef<Path>>(path: P) -> io::Result<String> {
    let mut file = File::open(path)?;
    let mut string = String::with_capacity(initial_buffer_size(&file));
    file.read_to_string(&mut string)?;
    Ok(string)
}

Almost identical, except the std lib version is more generic and leaves error handling to the caller. It also uses more efficient memory allocation.

New Type Idiom

The following is not necessarily better, but it is certainly a valid alternative approach. Instead of relying on using an f32 you could use a wrapper struct around it:

#[derive(Debug, Default, Clone, Copy, PartialEq, PartialOrd)] //not necessary but nice to have
struct ChargeLevel(f32);

This is called New Type Idiom. You could further go ahead and put most of the logic surrounding the charge level into special functions in the ChargeLevel's impl block. Some suggestions:

impl ChargeLevel {
    const WARNING_LEVEL: f32 = 15.0;
    const CRITICAL_LEVEL: f32 = 5.0;

    fn new(now: f32, full: f32) -> Self {
        ChargeLevel((now / full) * 100.0)
    }

    fn is_warning(&self) -> bool {
        self.0 < Self::WARNING_LEVEL
    }

    fn is_critical(&self) -> bool {
        self.0 < Self::CRITICAL_LEVEL
    }
}

You could also combine this struct with the external library stuff:

impl From<ChargeLevel> for NotificationUrgency {
    fn from(level: ChargeLevel) -> Self {
        if level.is_critical() {
            NotificationUrgency::Critical
        } else {
            NotificationUrgency::Normal
        }
    }
}

This would allow you to replace

if charge_level < CHARGE_WARN {
    Notification::new()
        .summary("Battery level low!")
        .body(&format!("Battery level: {0:.1$}%", charge_level, 2))
        .icon("dialog-information")
        .urgency(
            if charge_level < CHARGE_CRIT { 
                NotificationUrgency::Critical
            } else { 
                NotificationUrgency::Normal
            }
        )
        .timeout(NOTIFICATION_TIMEOUT)
        .show().unwrap();
}

with

if charge_level.is_warning() {
    Notification::new()
        .summary("Battery level low!")
        .body(&format!("Battery level: {0:.1$}%", charge_level.0, 2))
        .icon("dialog-information")
        .urgency(NotificationUrgency::from(charge_level))
        .timeout(NOTIFICATION_TIMEOUT)
        .show()
        .unwrap();
}

A little much for such a small application perhaps, but might give you an example how to write nice abstractions in Rust that don't cost any runtime overhead.

Implementing std::fmt::Display would also make sense for the string formatting, but you get the point.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.