From 2404b0a683e765cdec085a345bc34851d0feb834 Mon Sep 17 00:00:00 2001 From: wda2945 Date: Tue, 12 Apr 2016 13:41:01 +0100 Subject: [PATCH] pwm.c: Improve pwm error handling Signed-off-by: Martin G Lane-Smith Signed-off-by: Brendan Le Foll --- src/pwm/pwm.c | 165 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 124 insertions(+), 41 deletions(-) diff --git a/src/pwm/pwm.c b/src/pwm/pwm.c index 9b3fb0f..6690f65 100644 --- a/src/pwm/pwm.c +++ b/src/pwm/pwm.c @@ -27,6 +27,8 @@ #include #include #include +#include +#include #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) - return MRAA_ERROR_INVALID_RESOURCE; + if (!dev) { + 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; }