Implementing proper errors handling.

Both battery and battery-ffi are now propagating possible errors to callers.

Supporting multiple devices for FreeBSD and DragonFlyBSD systems (closes #17)
Ignoring Linux devices with scope other than System (closes #18).

.gitignore

@@ -3,4 +3,5 @@
 **/*.rs.bk
 Cargo.lock
 
+*.orig
 local_build.sh

Cargo.toml

@@ -7,3 +7,4 @@ members = [
 
 [patch.crates-io]
 battery = { path = "./battery" }
+battery-ffi = { path = "./battery-ffi" }

battery-cli/src/ui/app.rs

@@ -3,7 +3,7 @@
 use super::util::tabs;
 use super::util::graph;
 
-use battery::Battery;
+use battery::{Battery, Result};
 use battery::units::power::watt;
 use battery::units::electric_potential::volt;
 use battery::units::thermodynamic_temperature::degree_celsius;
@@ -24,8 +24,9 @@ pub struct App<'a> {
 }
 
 impl<'a> App<'a> {
-    pub fn new(manager: battery::Manager) -> App<'a> {
-        let stats: Vec<BatteryStats> = manager.iter()
+    pub fn new(manager: battery::Manager) -> Result<App<'a>> {
+        let stats: Vec<BatteryStats> = manager.batteries()?
+            .flatten()
             .map(|b| {
                 BatteryStats {
                     battery: b,
@@ -45,11 +46,11 @@ impl<'a> App<'a> {
         })
         .collect();
 
-        App {
+        Ok(App {
             manager,
             batteries: stats,
             tabs: tabs::TabsState::new(names),
-        }
+        })
     }
 
     pub fn update(&mut self) {

battery-cli/src/ui/mod.rs

@@ -59,9 +59,9 @@ pub fn start() -> Result<(), Box<Error>> {
     terminal.hide_cursor()?;
 
     let events = Events::new();
-    let manager = battery::Manager::new();
+    let manager = battery::Manager::new()?;
 
-    let mut app = app::App::new(manager);
+    let mut app = app::App::new(manager)?;
 
     loop {
         terminal.draw(|mut f| {

battery-ffi/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "battery-ffi"
-version = "0.2.0"
+version = "0.7.0"
 authors = ["svartalf <[email protected]>"]
 edition = "2018"
 description = "FFI bindings for battery crate"

battery-ffi/README.md

@@ -3,7 +3,7 @@
 [![Latest Version](https://img.shields.io/crates/v/battery-ffi.svg)](https://crates.io/crates/battery-ffi)
 [![Latest Version](https://docs.rs/battery-ffi/badge.svg)](https://docs.rs/battery-ffi)
 [![Build Status](https://travis-ci.org/svartalf/rust-battery.svg?branch=master)](https://travis-ci.org/svartalf/rust-battery)
-[![dependency status](https://deps.rs/crate/battery-ffi/0.2.0/status.svg)](https://deps.rs/crate/battery-ffi/0.2.0)
+[![dependency status](https://deps.rs/crate/battery-ffi/0.7.0/status.svg)](https://deps.rs/crate/battery-ffi/0.7.0)
 ![Apache 2.0 OR MIT licensed](https://img.shields.io/badge/license-Apache2.0%2FMIT-blue.svg)
 
 This is a FFI bindings for [battery](https://github.com/svartalf/rust-battery/tree/master/battery)

battery-ffi/build.rs

@@ -3,13 +3,10 @@ fn build_header() {
     use std::env;
     use std::path::PathBuf;
 
-    let crate_dir = env::var("CARGO_MANIFEST_DIR")
-        .expect("CARGO_MANIFEST_DIR env var is not defined");
-    let out_dir = PathBuf::from(env::var("OUT_DIR")
-        .expect("OUT_DIR env var is not defined"));
+    let crate_dir = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR env var is not defined");
+    let out_dir = PathBuf::from(env::var("OUT_DIR").expect("OUT_DIR env var is not defined"));
 
-    let config = cbindgen::Config::from_file("cbindgen.toml")
-        .expect("Unable to find cbindgen.toml configuration file");
+    let config = cbindgen::Config::from_file("cbindgen.toml").expect("Unable to find cbindgen.toml configuration file");
 
     cbindgen::generate_with_config(&crate_dir, config)
         .unwrap()

battery-ffi/examples/ffi.c

@@ -6,6 +6,7 @@
 //
 // 3. Run `./a.out`
 
+#include <string.h>
 #include <stdio.h>
 #include <float.h>
 #include <limits.h>
@@ -109,7 +110,7 @@ void pretty_print(Battery *battery, uint32_t *idx) {
         printf("  time-to-empty:\t\t%ld sec.\n", time_to_empty);
     }
 
-    printf("  state of charge:\t\t%.2f %%\n", battery_get_state_of_charge(battery));
+    printf("  state of charge:\t%.2f %%\n", battery_get_state_of_charge(battery));
     float temp = battery_get_temperature(battery);
     printf("  temperature:\t\t");
     if (temp < FLT_MAX) {
@@ -118,7 +119,7 @@ void pretty_print(Battery *battery, uint32_t *idx) {
         printf("N/A\n");
     }
 
-    printf("  state of health:\t\t%.2f %%\n", battery_get_state_of_health(battery));
+    printf("  state of health:\t%.2f %%\n", battery_get_state_of_health(battery));
     uint32_t cycle_count = battery_get_cycle_count(battery);
     printf("  cycle-count:\t\t");
     if (cycle_count < UINT_MAX) {
@@ -128,13 +129,35 @@ void pretty_print(Battery *battery, uint32_t *idx) {
     }
 }
 
+void print_error() {
+    int length = battery_last_error_length();
+    char *message = malloc(length);
+    // Handle possible error return here
+    battery_last_error_message(message, strlen(message));
+    printf("%s", message);
+    free(message);
+}
+
 void main() {
     Manager *manager = battery_manager_new();
+    if (manager == NULL) {
+        print_error();
+        return;
+    }
+
     Batteries *iterator = battery_manager_iter(manager);
+    if (iterator == NULL) {
+        print_error();
+        return;
+    }
+
     uint32_t idx = 0;
     while (true) {
         Battery *battery = battery_iterator_next(iterator);
         if (battery == NULL) {
+            if (battery_have_last_error() == 1) {
+                print_error();
+            }
             break;
         }
 

battery-ffi/examples/ffi.py

@@ -14,6 +14,7 @@
 
 import sys
 import ctypes
+import logging
 
 prefix = {'win32': ''}.get(sys.platform, 'lib')
 extension = {'darwin': 'dylib', 'win32': 'dll'}.get(sys.platform, 'so')
@@ -56,19 +57,35 @@ class Battery(ctypes.Structure):
     pass
 
 
+def check_result(result, _func, _args):
+    # Checking if passed value is not `NULL` pointer.
+    if lib.battery_have_last_error() == 0:
+        return result
+
+    # If it is, constructing error message and raising it
+    length = lib.battery_last_error_length()
+    message = ctypes.create_string_buffer(length)
+    lib.battery_last_error_message(ctypes.byref(message), len(message))
+
+    raise ValueError(message.value)
+
+
 #
 # Bindings for exported functions
 #
 
 lib.battery_manager_new.argtypes = None
 lib.battery_manager_new.restype = ctypes.POINTER(Manager)
+lib.battery_manager_new.errcheck = check_result
 lib.battery_manager_iter.argtypes = (ctypes.POINTER(Manager), )
 lib.battery_manager_iter.restype = ctypes.POINTER(Batteries)
+lib.battery_manager_iter.errcheck = check_result
 lib.battery_manager_free.argtypes = (ctypes.POINTER(Manager), )
 lib.battery_manager_free.restype = None
 
 lib.battery_iterator_next.argtypes = (ctypes.POINTER(Batteries), )
 lib.battery_iterator_next.restype = ctypes.POINTER(Battery)
+lib.battery_iterator_next.errcheck = check_result
 
 lib.battery_free.argtypes = (ctypes.POINTER(Battery), )
 lib.battery_free.restype = None
@@ -107,6 +124,14 @@ class Battery(ctypes.Structure):
 lib.battery_get_cycle_count.argtypes = (ctypes.POINTER(Battery), )
 lib.battery_get_cycle_count.restype = ctypes.c_uint32
 
+lib.battery_have_last_error.argtypes = None
+lib.battery_have_last_error.restype = ctypes.c_int
+lib.battery_last_error_length.argtypes = None
+lib.battery_last_error_length.restype = ctypes.c_int
+lib.battery_last_error_message.argtypes = (ctypes.c_char_p, ctypes.c_int)
+lib.battery_last_error_message.restype = ctypes.c_int
+
+
 if __name__ == '__main__':
     manager = lib.battery_manager_new()
     iterator = lib.battery_manager_iter(manager)

battery-ffi/src/errors.rs

@@ -0,0 +1,86 @@
+use std::cell::RefCell;
+use std::error::Error;
+use std::ptr;
+use std::slice;
+
+thread_local! {
+    static LAST_ERROR: RefCell<Option<Box<Error>>> = RefCell::new(None);
+}
+
+pub fn set_last_error<E: Error + 'static>(err: E) {
+    LAST_ERROR.with(|prev| {
+        *prev.borrow_mut() = Some(Box::new(err));
+    });
+}
+
+pub fn take_last_error() -> Option<Box<Error>> {
+    LAST_ERROR.with(|prev| prev.borrow_mut().take())
+}
+
+pub fn clear_last_error() {
+    let _ = take_last_error();
+}
+
+/// Checks if there was an error before.
+///
+/// # Returns
+///
+/// `0` if there was no error, `1` if error had occured.
+#[no_mangle]
+pub extern "C" fn battery_have_last_error() -> libc::c_int {
+    LAST_ERROR.with(|prev| match *prev.borrow() {
+        Some(_) => 1,
+        None => 0,
+    })
+}
+
+/// Gets error message length if any error had occurred.
+///
+/// # Returns
+///
+/// If there was no error before, returns `0`,
+/// otherwise returns message length including trailing `\0`.
+#[no_mangle]
+pub extern "C" fn battery_last_error_length() -> libc::c_int {
+    // TODO: Support Windows UTF-16 strings
+    LAST_ERROR.with(|prev| match *prev.borrow() {
+        Some(ref err) => err.to_string().len() as libc::c_int + 1,
+        None => 0,
+    })
+}
+
+/// Fills passed buffer with an error message.
+///
+/// Buffer length can be get with [battery_last_error_length](fn.battery_last_error_length.html) function.
+///
+/// # Returns
+///
+/// Returns `-1` is passed buffer is `NULL` or too small for error message.
+/// Returns `0` if there was no error previously.
+///
+/// In all other cases returns error message length.
+#[no_mangle]
+pub unsafe extern "C" fn battery_last_error_message(buffer: *mut libc::c_char, length: libc::c_int) -> libc::c_int {
+    if buffer.is_null() {
+        return -1;
+    }
+
+    let last_error = match take_last_error() {
+        Some(err) => err,
+        None => return 0,
+    };
+
+    let error_message = last_error.to_string();
+
+    let buffer = slice::from_raw_parts_mut(buffer as *mut u8, length as usize);
+
+    if error_message.len() >= buffer.len() {
+        return -1;
+    }
+
+    ptr::copy_nonoverlapping(error_message.as_ptr(), buffer.as_mut_ptr(), error_message.len());
+
+    buffer[error_message.len()] = b'\0';
+
+    error_message.len() as libc::c_int
+}

battery-ffi/src/iterator.rs

@@ -7,25 +7,35 @@ use crate::{Batteries, Battery};
 /// Caller is required to call [battery_free](fn.battery_free.html) in order
 /// to properly free memory for the returned battery instance.
 ///
-/// Caller is required to call [battery_iterator_free](fn.battery_iterator_free.html)
-/// if order to properly free memory for the returned batteries iterator instance.
-///
 /// # Panics
 ///
-/// This function will panic if any passed pointer is `NULL`
+/// This function will panic if passed pointer is `NULL`.
 ///
 /// # Returns
 ///
-/// If there is no batteries left to iterate, this function returns `NULL`,
-/// otherwise it returns pointer to next battery.
+/// Returns pointer to next battery.
+///
+/// If there is no batteries left to iterate or some error happened, this function will return `NULL`.
+///
+/// Caller is required to differentiate between these two cases and should check
+/// if there was any error with [battery_have_last_error](fn.battery_have_last_error.html).
+///
+/// If there is no batteries left, `battery_have_last_error` will return `0`.
 #[no_mangle]
 pub unsafe extern "C" fn battery_iterator_next(ptr: *mut Batteries) -> *mut Battery {
     assert!(!ptr.is_null());
     let iterator = &mut *ptr;
 
     match iterator.next() {
-        None => ptr::null_mut(),
-        Some(battery) => Box::into_raw(Box::new(battery)),
+        None => {
+            crate::errors::clear_last_error();
+            ptr::null_mut()
+        }
+        Some(Ok(battery)) => Box::into_raw(Box::new(battery)),
+        Some(Err(e)) => {
+            crate::errors::set_last_error(e);
+            ptr::null_mut()
+        }
     }
 }
 

battery-ffi/src/lib.rs

@@ -2,22 +2,28 @@
 //!
 //! # Bindings generation
 //!
-//! Among library creation this crate generates `battery_ffi.h` file,
+//! Among library creation this crate generates `battery_ffi.h` file, enabled by default by `cbindgen` feature,
 //! which might be useful for automatic bindings generation or just with plain `C`/`C++` development.
 //!
 //! After build it will be located somewhere at `target/*/build/battery-ffi-*/out/`,
 //! depending on build profile (`debug`/`release`) and build hash.
 //!
+//! Disabling `cbindgen` feature might speed up compilation a little bit,
+//! especially if you don't need the header file.
+//!
 //! # Examples
 //!
 //! ```c
 //! #include "battery_ffi.h"
 //!
 //! void main() {
 //!    Manager *manager = battery_manager_new();
+//!    // .. handle `manager == NULL` here ..
 //!    Batteries *iterator = battery_manager_iter(manager);
+//!    // .. handle `iterator == NULL` here ..
 //!    while (true) {
 //!        Battery *battery = battery_iterator_next(iterator);
+//!        // .. handle possible error here ..
 //!        if (battery == NULL) {
 //!            break;
 //!        }
@@ -31,12 +37,15 @@
 //!    battery_manager_free(manager);
 //! }
 //! ```
+//!
+//! Also, check the `examples/` directory in the repository for examples with C and Python.
 
 // cbindgen==0.8.0 fails to export typedefs for opaque pointers
 // from the battery crate, if this line is missing
 extern crate battery as battery_lib;
 
 mod battery;
+mod errors;
 mod iterator;
 mod manager;
 mod state;
@@ -61,6 +70,7 @@ pub type Batteries = battery_lib::Batteries;
 pub type Battery = battery_lib::Battery;
 
 pub use self::battery::*;
+pub use self::errors::{battery_last_error_length, battery_last_error_message};
 pub use self::iterator::*;
 pub use self::manager::*;
 pub use self::state::*;