pwm.c: Improve pwm error handling

Signed-off-by: Martin G Lane-Smith <mlanesmith@gmail.com>
Signed-off-by: Brendan Le Foll <brendan.le.foll@intel.com>

src/pwm/pwm.c

@@ -27,6 +27,8 @@
 #include <sys/stat.h>
 #include <unistd.h>
 #include <limits.h>
+#include <errno.h>
+#include <string.h>
 
 #include "pwm.h"
 #include "mraa_internal.h"
@@ -50,6 +52,11 @@ mraa_pwm_setup_duty_fp(mraa_pwm_context dev)
 static mraa_result_t
 mraa_pwm_write_period(mraa_pwm_context dev, int period)
 {
+    if (!dev) {
+        syslog(LOG_ERR, "pwm: write_period: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     if (IS_FUNC_DEFINED(dev, pwm_period_replace)) {
         mraa_result_t result = dev->advance_func->pwm_period_replace(dev, period);
         if (result == MRAA_SUCCESS) {
@@ -62,13 +69,14 @@ mraa_pwm_write_period(mraa_pwm_context dev, int period)
 
     int period_f = open(bu, O_RDWR);
     if (period_f == -1) {
-        syslog(LOG_ERR, "pwm: Failed to open period for writing");
+        syslog(LOG_ERR, "pwm%i write_period: Failed to open period for writing: %s", dev->pin, strerror(errno));
         return MRAA_ERROR_INVALID_RESOURCE;
     }
     char out[MAX_SIZE];
     int length = snprintf(out, MAX_SIZE, "%d", period);
     if (write(period_f, out, length * sizeof(char)) == -1) {
         close(period_f);
+        syslog(LOG_ERR, "pwm%i write_period: Failed to write to period: %s", dev->pin, strerror(errno));
         return MRAA_ERROR_INVALID_RESOURCE;
     }
 
@@ -80,24 +88,38 @@ mraa_pwm_write_period(mraa_pwm_context dev, int period)
 static mraa_result_t
 mraa_pwm_write_duty(mraa_pwm_context dev, int duty)
 {
+    if (!dev) {
+        syslog(LOG_ERR, "pwm: write_duty: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     if (IS_FUNC_DEFINED(dev, pwm_write_replace)) {
         return dev->advance_func->pwm_write_replace(dev, duty);
     }
     if (dev->duty_fp == -1) {
         if (mraa_pwm_setup_duty_fp(dev) == 1) {
-            return MRAA_ERROR_INVALID_HANDLE;
+            syslog(LOG_ERR, "pwm%i write_duty: Failed to open duty_cycle for writing: %s", dev->pin, strerror(errno));
+            return MRAA_ERROR_INVALID_RESOURCE;
         }
     }
     char bu[64];
     int length = sprintf(bu, "%d", duty);
     if (write(dev->duty_fp, bu, length * sizeof(char)) == -1)
+    {
+        syslog(LOG_ERR, "pwm%i write_duty: Failed to write to duty_cycle: %s", dev->pin, strerror(errno));
         return MRAA_ERROR_INVALID_RESOURCE;
+    }
     return MRAA_SUCCESS;
 }
 
 static int
 mraa_pwm_read_period(mraa_pwm_context dev)
 {
+    if (!dev) {
+        syslog(LOG_ERR, "pwm: read_period: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     if (IS_FUNC_DEFINED(dev, pwm_read_replace)) {
         return dev->period;
     }
@@ -108,27 +130,25 @@ mraa_pwm_read_period(mraa_pwm_context dev)
 
     int period_f = open(bu, O_RDWR);
     if (period_f == -1) {
-        syslog(LOG_ERR, "pwm: Failed to open period for reading");
+        syslog(LOG_ERR, "pwm%i read_period: Failed to open period for reading: %s", dev->pin, strerror(errno));
         return 0;
     }
-    off_t size = lseek(period_f, 0, SEEK_END);
-    lseek(period_f, 0, SEEK_SET);
 
-    ssize_t rb = read(period_f, output, size + 1);
+    ssize_t rb = read(period_f, output, MAX_SIZE);
     close(period_f);
 
     if (rb < 0) {
-        syslog(LOG_ERR, "pwm: Error in reading period");
+        syslog(LOG_ERR, "pwm%i read_period: Failed to read period: %s", dev->pin, strerror(errno));
         return -1;
     }
 
     char* endptr;
     long int ret = strtol(output, &endptr, 10);
     if ('\0' != *endptr && '\n' != *endptr) {
-        syslog(LOG_ERR, "pwm: Error in string conversion");
+        syslog(LOG_ERR, "pwm%i read_period: Error in string conversion", dev->pin);
         return -1;
     } else if (ret > INT_MAX || ret < INT_MIN) {
-        syslog(LOG_ERR, "pwm: Number is invalid");
+        syslog(LOG_ERR, "pwm%i read_period: Number is invalid", dev->pin);
         return -1;
     }
     dev->period = (int) ret;
@@ -138,33 +158,39 @@ mraa_pwm_read_period(mraa_pwm_context dev)
 static int
 mraa_pwm_read_duty(mraa_pwm_context dev)
 {
+    if (!dev) {
+       syslog(LOG_ERR, "pwm: read_duty: context is NULL");
+       return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     if (IS_FUNC_DEFINED(dev, pwm_read_replace)) {
         return dev->advance_func->pwm_read_replace(dev);
     }
 
     if (dev->duty_fp == -1) {
         if (mraa_pwm_setup_duty_fp(dev) == 1) {
-            return MRAA_ERROR_INVALID_HANDLE;
+            syslog(LOG_ERR, "pwm%i read_duty: Failed to open duty_cycle for reading: %s",
+                    dev->pin, strerror(errno));
+            return -1;
         }
     } else {
         lseek(dev->duty_fp, 0, SEEK_SET);
     }
-    off_t size = lseek(dev->duty_fp, 0, SEEK_END);
+
-    lseek(dev->duty_fp, 0, SEEK_SET);
     char output[MAX_SIZE];
-    ssize_t rb = read(dev->duty_fp, output, size + 1);
+    ssize_t rb = read(dev->duty_fp, output, MAX_SIZE);
     if (rb < 0) {
-        syslog(LOG_ERR, "pwm: Error in reading duty");
+        syslog(LOG_ERR, "pwm%i read_duty: Failed to read duty_cycle: %s", dev->pin, strerror(errno));
         return -1;
     }
 
     char* endptr;
     long int ret = strtol(output, &endptr, 10);
     if ('\0' != *endptr && '\n' != *endptr) {
-        syslog(LOG_ERR, "pwm: Error in string conversion");
+        syslog(LOG_ERR, "pwm%i read_duty: Error in string conversion", dev->pin);
         return -1;
     } else if (ret > INT_MAX || ret < INT_MIN) {
-        syslog(LOG_ERR, "pwm: Number is invalid");
+        syslog(LOG_ERR, "pwm%i read_duty: Number is invalid", dev->pin);
         return -1;
     }
     return (int) ret;
@@ -191,24 +217,24 @@ mraa_pwm_init(int pin)
 {
     mraa_board_t* board = plat;
     if (board == NULL) {
-        syslog(LOG_ERR, "pwm: Platform Not Initialised");
+        syslog(LOG_ERR, "pwm_init: Platform Not Initialised");
         return NULL;
     }
     if (mraa_is_sub_platform_id(pin)) {
-        syslog(LOG_NOTICE, "pwm: Using sub platform");
+        syslog(LOG_NOTICE, "pwm_init: Using sub platform");
         board = board->sub_platform;
         if (board == NULL) {
-            syslog(LOG_ERR, "pwm: Sub platform Not Initialised");
+            syslog(LOG_ERR, "pwm_init: Sub platform Not Initialised");
             return NULL;
         }
         pin = mraa_get_sub_platform_index(pin);
     }
     if (pin < 0 || pin > board->phy_pin_count) {
-        syslog(LOG_ERR, "pwm: pin %i beyond platform definition", pin);
+        syslog(LOG_ERR, "pwm_init: pin %i beyond platform definition", pin);
         return NULL;
     }
     if (board->pins[pin].capabilites.pwm != 1) {
-        syslog(LOG_ERR, "pwm: pin not capable of pwm");
+        syslog(LOG_ERR, "pwm_init: pin %i not capable of pwm", pin);
         return NULL;
     }
 
@@ -228,26 +254,26 @@ mraa_pwm_init(int pin)
         mraa_gpio_context mux_i;
         mux_i = mraa_gpio_init_raw(board->pins[pin].gpio.pinmap);
         if (mux_i == NULL) {
-            syslog(LOG_ERR, "pwm: error in gpio->pwm transition");
+            syslog(LOG_ERR, "pwm_init: error in gpio->pwm%i transition. gpio_init", pin);
             return NULL;
         }
         if (mraa_gpio_dir(mux_i, MRAA_GPIO_OUT) != MRAA_SUCCESS) {
-            syslog(LOG_ERR, "pwm: error in gpio->pwm transition");
+            syslog(LOG_ERR, "pwm_init: error in gpio->pwm%i transition. gpio_dir", pin);
             return NULL;
         }
         if (mraa_gpio_write(mux_i, 1) != MRAA_SUCCESS) {
-            syslog(LOG_ERR, "pwm: error in gpio->pwm transition");
+            syslog(LOG_ERR, "pwm_init: error in gpio->pwm%i transition. gpio_write", pin);
             return NULL;
         }
         if (mraa_gpio_close(mux_i) != MRAA_SUCCESS) {
-            syslog(LOG_ERR, "pwm: error in gpio->pwm transition");
+            syslog(LOG_ERR, "pwm_init: error in gpio->pwm%i transition. gpio_close", pin);
             return NULL;
         }
     }
 
     if (board->pins[pin].pwm.mux_total > 0) {
         if (mraa_setup_mux_mapped(board->pins[pin].pwm) != MRAA_SUCCESS) {
-            syslog(LOG_ERR, "pwm: Failed to set-up multiplexer");
+            syslog(LOG_ERR, "pwm_init: Failed to set-up pwm%i multiplexer", pin);
             return NULL;
         }
     }
@@ -278,14 +304,14 @@ mraa_pwm_init_raw(int chipin, int pin)
     snprintf(directory, MAX_SIZE, SYSFS_PWM "/pwmchip%d/pwm%d", dev->chipid, dev->pin);
     struct stat dir;
     if (stat(directory, &dir) == 0 && S_ISDIR(dir.st_mode)) {
-        syslog(LOG_NOTICE, "pwm: Pin already exported, continuing");
+        syslog(LOG_NOTICE, "pwm_init: pwm%i already exported, continuing", pin);
         dev->owner = 0; // Not Owner
     } else {
         char buffer[MAX_SIZE];
         snprintf(buffer, MAX_SIZE, "/sys/class/pwm/pwmchip%d/export", dev->chipid);
         int export_f = open(buffer, O_WRONLY);
         if (export_f == -1) {
-            syslog(LOG_ERR, "pwm: Failed to open export for writing");
+            syslog(LOG_ERR, "pwm_init: pwm%i. Failed to open export for writing: %s", pin, strerror(errno));
             free(dev);
             return NULL;
         }
@@ -293,7 +319,7 @@ mraa_pwm_init_raw(int chipin, int pin)
         char out[MAX_SIZE];
         int size = snprintf(out, MAX_SIZE, "%d", dev->pin);
         if (write(export_f, out, size * sizeof(char)) == -1) {
-            syslog(LOG_WARNING, "pwm: Failed to write to export! Potentially already enabled");
+            syslog(LOG_WARNING, "pwm_init: pwm%i. Failed to write to export! (%s) Potentially already in use.", pin, strerror(errno));
             close(export_f);
             free(dev);
             return NULL;
@@ -309,13 +335,18 @@ mraa_pwm_init_raw(int chipin, int pin)
 mraa_result_t
 mraa_pwm_write(mraa_pwm_context dev, float percentage)
 {
+    if (!dev) {
+        syslog(LOG_ERR, "pwm: write: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     if (dev->period == -1) {
         if (mraa_pwm_read_period(dev) <= 0)
             return MRAA_ERROR_NO_DATA_AVAILABLE;
     }
 
     if (percentage > 1.0f) {
-        syslog(LOG_WARNING, "pwm: number greater than 1 entered, defaulting to 100%%");
+        syslog(LOG_WARNING, "pwm_write: %i%% entered, defaulting to 100%%",(int) percentage * 100);
         return mraa_pwm_write_duty(dev, dev->period);
     }
     return mraa_pwm_write_duty(dev, percentage * dev->period);
@@ -324,6 +355,11 @@ mraa_pwm_write(mraa_pwm_context dev, float percentage)
 float
 mraa_pwm_read(mraa_pwm_context dev)
 {
+    if (!dev) {
+        syslog(LOG_ERR, "pwm: read: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     int period = mraa_pwm_read_period(dev);
     if (period > 0) {
         return (mraa_pwm_read_duty(dev) / (float) period);
@@ -347,6 +383,12 @@ mraa_result_t
 mraa_pwm_period_us(mraa_pwm_context dev, int us)
 {
     int min, max;
+
+    if (!dev) {
+        syslog(LOG_ERR, "pwm: period: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     if (mraa_is_sub_platform_id(dev->chipid)) {
         min = plat->sub_platform->pwm_min_period;
         max = plat->sub_platform->pwm_max_period;
@@ -355,7 +397,7 @@ mraa_pwm_period_us(mraa_pwm_context dev, int us)
         max = plat->pwm_max_period;
     }
     if (us < min || us > max) {
-        syslog(LOG_ERR, "pwm: period value outside platform range");
+        syslog(LOG_ERR, "pwm_period: pwm%i: %i uS outside platform range", dev->pin, us);
         return MRAA_ERROR_INVALID_PARAMETER;
     }
     return mraa_pwm_write_period(dev, us * 1000);
@@ -382,25 +424,30 @@ mraa_pwm_pulsewidth_us(mraa_pwm_context dev, int us)
 mraa_result_t
 mraa_pwm_enable(mraa_pwm_context dev, int enable)
 {
+    if (!dev) {
+        syslog(LOG_ERR, "pwm: enable: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     if (IS_FUNC_DEFINED(dev, pwm_enable_replace)) {
         return dev->advance_func->pwm_enable_replace(dev, enable);
     }
-    int status;
+
     char bu[MAX_SIZE];
     snprintf(bu, MAX_SIZE, "/sys/class/pwm/pwmchip%d/pwm%d/enable", dev->chipid, dev->pin);
 
     int enable_f = open(bu, O_RDWR);
 
     if (enable_f == -1) {
-        syslog(LOG_ERR, "pwm: Failed to open enable for writing");
+        syslog(LOG_ERR, "pwm_enable: pwm%i: Failed to open enable for writing: %s", dev->pin, strerror(errno));
         return MRAA_ERROR_INVALID_RESOURCE;
     }
     char out[2];
     int size = snprintf(out, sizeof(out), "%d", enable);
     if (write(enable_f, out, size * sizeof(char)) == -1) {
-        syslog(LOG_ERR, "pwm: Failed to write to enable");
+        syslog(LOG_ERR, "pwm_enable: pwm%i: Failed to write to enable: %s", dev->pin, strerror(errno));
         close(enable_f);
-        return MRAA_ERROR_INVALID_RESOURCE;
+        return MRAA_ERROR_UNSPECIFIED;
     }
     close(enable_f);
     return MRAA_SUCCESS;
@@ -414,16 +461,16 @@ mraa_pwm_unexport_force(mraa_pwm_context dev)
 
     int unexport_f = open(filepath, O_WRONLY);
     if (unexport_f == -1) {
-        syslog(LOG_ERR, "pwm: Failed to open unexport for writing");
+        syslog(LOG_ERR, "pwm_unexport: pwm%i: Failed to open unexport for writing: %s", dev->pin, strerror(errno));
         return MRAA_ERROR_INVALID_RESOURCE;
     }
 
     char out[MAX_SIZE];
     int size = snprintf(out, MAX_SIZE, "%d", dev->pin);
     if (write(unexport_f, out, size * sizeof(char)) == -1) {
-        syslog(LOG_ERR, "pwm: Failed to write to unexport");
+        syslog(LOG_ERR, "pwm_unexport: pwm%i: Failed to write to unexport: %s", dev->pin, strerror(errno));
         close(unexport_f);
-        return MRAA_ERROR_INVALID_RESOURCE;
+        return MRAA_ERROR_UNSPECIFIED;
     }
 
     close(unexport_f);
@@ -433,16 +480,26 @@ mraa_pwm_unexport_force(mraa_pwm_context dev)
 mraa_result_t
 mraa_pwm_unexport(mraa_pwm_context dev)
 {
+    if (!dev) {
+        syslog(LOG_ERR, "pwm: unexport: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     mraa_pwm_enable(dev, 0);
     if (dev->owner) {
         return mraa_pwm_unexport_force(dev);
     }
-    return MRAA_ERROR_INVALID_RESOURCE;
+    return MRAA_ERROR_INVALID_PARAMETER;
 }
 
 mraa_result_t
 mraa_pwm_close(mraa_pwm_context dev)
 {
+    if (!dev) {
+        syslog(LOG_ERR, "pwm: close: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     mraa_pwm_unexport(dev);
     free(dev);
     return MRAA_SUCCESS;
@@ -451,8 +508,10 @@ mraa_pwm_close(mraa_pwm_context dev)
 mraa_result_t
 mraa_pwm_owner(mraa_pwm_context dev, mraa_boolean_t owner_new)
 {
-    if (dev == NULL)
+    if (!dev) {
-        return MRAA_ERROR_INVALID_RESOURCE;
+        syslog(LOG_ERR, "pwm: owner: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
     dev->owner = owner_new;
     return MRAA_SUCCESS;
 }
@@ -461,6 +520,12 @@ mraa_result_t
 mraa_pwm_config_ms(mraa_pwm_context dev, int ms, float ms_float)
 {
     int old_dutycycle, old_period, status;
+
+    if (!dev) {
+        syslog(LOG_ERR, "pwm: config_ms: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     old_dutycycle = mraa_pwm_read_duty(dev);
     old_period = mraa_pwm_read_period(dev);
     status = mraa_pwm_period_us(dev, ms * 1000);
@@ -485,6 +550,12 @@ mraa_result_t
 mraa_pwm_config_percent(mraa_pwm_context dev, int ms, float percentage)
 {
     int old_dutycycle, old_period, status;
+
+    if (!dev) {
+        syslog(LOG_ERR, "pwm: config_percent: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     old_dutycycle = mraa_pwm_read_duty(dev);
     old_period = mraa_pwm_read_period(dev);
     status = mraa_pwm_period_us(dev, ms * 1000);
@@ -511,6 +582,12 @@ mraa_pwm_get_max_period(mraa_pwm_context dev)
     if (plat == NULL) {
         return -1;
     }
+
+    if (!dev) {
+        syslog(LOG_ERR, "pwm: get_max_period: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     if (mraa_is_sub_platform_id(dev->chipid)) {
         return plat->sub_platform->pwm_max_period;
     }
@@ -523,6 +600,12 @@ mraa_pwm_get_min_period(mraa_pwm_context dev)
     if (plat == NULL) {
         return -1;
     }
+
+    if (!dev) {
+        syslog(LOG_ERR, "pwm: get_min_period: context is NULL");
+        return MRAA_ERROR_INVALID_HANDLE;
+    }
+
     if (mraa_is_sub_platform_id(dev->chipid)) {
         return plat->sub_platform->pwm_min_period;
     }