Browse Source

Added Math::Round to dependency list in Makefile.PL. Credit - Syohei YOSHIDA.
Refactored methods to return N/A first before any further proessing is done
Cleaned up unit tests
get_disk_model() only outputs just the model information now.
Added get_disk_errors()
Methods now validate the device passed to it, to ensure smartctl has already read it
Added unit tests to test passing invalid device to methods
Changed to croak() as I didn't feel a trace was necessary with proper error returning.

Paul Trost 11 years ago
parent
commit
34f45903bb
5 changed files with 64 additions and 24 deletions
  1. 10 0
      Changes
  2. 44 15
      lib/Disk/SMART.pm
  3. 0 2
      t/00-load.t
  4. 10 5
      t/02-get_functions.t
  5. 0 2
      t/manifest.t

+ 10 - 0
Changes

@@ -1,5 +1,15 @@
 Revision history for Disk-SMART
 
+0.03    2014-10-04
+        Added Math::Round to dependency list in Makefile.PL. Credit - Syohei YOSHIDA.
+        Refactored methods to return N/A first before any further proessing is done
+        Cleaned up unit tests
+        get_disk_model() only outputs just the model information now.
+        Added get_disk_errors()
+        Methods now validate the device passed to it, to ensure smartctl has already read it
+        Added unit tests to test passing invalid device to methods
+        Changed to croak() as I didn't feel a trace was necessary with proper error returning.
+
 0.02    2014-10-03
         Added confess to new() in case smartctl cannot find the specified device
 

+ 44 - 15
lib/Disk/SMART.pm

@@ -11,11 +11,11 @@ Disk::SMART - Provides an interface to smartctl
 
 =head1 VERSION
 
-Version 0.01
+Version 0.03
 
 =cut
 
-our $VERSION = '0.01';
+our $VERSION = '0.03';
 
 =head1 SYNOPSIS
 
@@ -42,20 +42,16 @@ C<DEVICE> - Device identifier of SSD / Hard Drive
 
 sub new {
     my ( $class, @devices ) = @_;
-    confess "Valid device identifier not supplied to constructor.\n" if ( !@devices );
-
     my $smartctl = '/usr/sbin/smartctl';
-    if ( !-f $smartctl ) {
-        confess "smartctl binary was not found on your system, are you running as root?\n";
-    }
-
     my $self = bless {}, $class;
 
+    croak "Valid device identifier not supplied to constructor for $class.\n" if ( !@devices );
+    croak "smartctl binary was not found on your system, are you running as root?\n" if !-f $smartctl;
+
     foreach my $device (@devices) {
         my $out = qx($smartctl -a $device);
         if ( $out =~ /No such device/i ) {
-            confess "Device $device could not be found\n";
-            next;
+            croak "Smartctl couldn't poll device $device\n";
         }
         $self->{'devices'}->{$device}->{'SMART_OUTPUT'} = $out;
     }
@@ -79,6 +75,7 @@ C<DEVICE> - Device identifier of SSD / Hard Drive
 
 sub get_disk_temp {
     my ( $self, $device ) = @_;
+    $self->_validate_param($device);
     my $smart_output = $self->{'devices'}->{$device}->{'SMART_OUTPUT'};
     my ($temp_c) = $smart_output =~ /(Temperature_Celsius.*\n)/;
 
@@ -90,7 +87,6 @@ sub get_disk_temp {
     $temp_c =~ s/ //g;
     $temp_c =~ s/.*-//;
     $temp_c =~ s/\(.*\)//;
-    }
 
     my $temp_f = round( ( $temp_c * 9 ) / 5 + 32 );
     return ( $temp_c, $temp_f );
@@ -109,10 +105,11 @@ C<DEVICE> - Device identifier of SSD / Hard Drive
 
 sub get_disk_health {
     my ( $self, $device ) = @_;
+    $self->_validate_param($device);
     my $smart_output = $self->{'devices'}->{$device}->{'SMART_OUTPUT'};
     my ($health) = $smart_output =~ /(SMART overall-health self-assessment.*\n)/;
 
-    if ( !defined $health or $health !~ /PASSED|FAILED/x ) {
+    if ( (!defined $health) or $health !~ /PASSED|FAILED/x ) {
         return 'N/A';
     }
 
@@ -123,7 +120,7 @@ sub get_disk_health {
 
 =head2 B<get_disk_model (DEVICE)>
 
-Returns the model of the device. Output is "<device>: <model>" or "N/A". eg. "/dev/sda: ST3250410AS"
+Returns the model of the device. eg. "ST3250410AS".
 
 C<DEVICE> - Device identifier of SSD / Hard Drive
 
@@ -133,18 +130,50 @@ C<DEVICE> - Device identifier of SSD / Hard Drive
 
 sub get_disk_model {
     my ( $self, $device ) = @_;
+    $self->_validate_param($device);
     my $smart_output = $self->{'devices'}->{$device}->{'SMART_OUTPUT'};
     my ($model) = $smart_output =~ /(Device\ Model.*\n)/;
 
     if ( !defined $model ) {
-        return "$device: N/A";
+        return 'N/A';
     }
     
     $model =~ s/.*:\ //;
     $model =~ s/^\s+|\s+$//g;    #trim beginning and ending whitepace
-    return "$device: $model";
+    return $model;
 }
 
+=head2 B<get_disk_errors (DEVICE)>
+
+Returns any listed errors
+
+C<DEVICE> - DEvice identifier of SSD/ Hard Drive
+
+    my $disk_errors = $smart->get_disk_errors('/dev/sda');
+
+=cut
+
+sub get_disk_errors {
+    my ( $self, $device ) = @_;
+    $self->_validate_param($device);
+
+    my $smart_output = $self->{'devices'}->{$device}->{'SMART_OUTPUT'};
+    my ($errors) = $smart_output =~ /SMART Error Log Version: [1-9](.*)SMART Self-test log/s;
+
+    if ( !defined $errors ) {
+        return 'N/A';
+    }
+
+    $errors =~ s/^\s+|\s+$//g;    #trim beginning and ending whitepace
+    return $errors;
+}
+
+sub _validate_param {
+    my ( $self, $device ) = @_;
+    croak "$device not found in object, You probably didn't enter it right" if ( !exists $self->{'devices'}->{$device} );
+}
+
+
 1;
 
 __END__

+ 0 - 2
t/00-load.t

@@ -1,5 +1,3 @@
-#!perl -T
-
 use Test::More tests => 1;
 
 BEGIN {

+ 10 - 5
t/02-get_functions.t

@@ -1,11 +1,16 @@
 use warnings;
 use strict;
-use Test::More 'tests' => 3;
+use Test::More 'tests' => 8;
 use Disk::SMART;
 
-my $disk = '/dev/sda';
+my $disk  = '/dev/sda';
 my $smart = Disk::SMART->new($disk);
 
-ok( $smart->get_disk_temp($disk), 'get_disk_temp() returns drive temperature or N/A');
-like( $smart->get_disk_health('/dev/sda'), qr/PASSED|FAILED|N\/A/, 'get_disk_health() returns health status or N/A');
-like( $smart->get_disk_model('/dev/sda'), qr/(\w+: \w+)|N\/A/, 'get_disk_model() returned disk model or N/A');
+ok( $smart->get_disk_temp($disk), 'get_disk_temp() returns drive temperature or N/A' );
+is( eval{$smart->get_disk_temp('/dev/sdz')}, undef, 'get_disk_temp() returns failure when passed invalid device' );
+like( $smart->get_disk_health($disk), qr/PASSED|FAILED|N\/A/, 'get_disk_health() returns health status or N/A' );
+is( eval{$smart->get_disk_health('/dev/sdz')}, undef, 'get_disk_health() returns failure when passed invalid device' );
+like( $smart->get_disk_model($disk),  qr/\w+/,    'get_disk_model() returns disk model or N/A' );
+is( eval{$smart->get_disk_errors('/dev/sdz')}, undef, 'get_disk_model() returns failure when passed invalid device' );
+like( $smart->get_disk_errors($disk), qr/\w+/, 'get_disk_errors() returns proper string' );
+is( eval{$smart->get_disk_errors('/dev/sdz')}, undef, 'get_disk_errors() returns failure when passed invalid device' );

+ 0 - 2
t/manifest.t

@@ -1,5 +1,3 @@
-#!perl -T
-
 use strict;
 use warnings;
 use Test::More;