Fixed softpwm bug ("disable" code didn't synchronize thread exit)

source/c_softpwm.c

@@ -62,6 +62,7 @@ struct pwm_params
 struct softpwm
 {
     char key[KEYLEN+1]; /* leave room for terminating NUL byte */
+    int gpio;
     struct pwm_params params;
     pthread_mutex_t* params_lock;
     pthread_t thread;
@@ -142,10 +143,10 @@ int softpwm_set_duty_cycle(const char *key, float duty) {;
     return 0;
 }
 
-void *softpwm_thread_toggle(void *key)
+void *softpwm_thread_toggle(void *arg)
 {
-  struct softpwm *pwm;
+  struct softpwm *pwm = (struct softpwm *)arg;
-  int gpio;
+  int gpio = pwm->gpio;
   struct timespec tim_on;
   struct timespec tim_off;
   unsigned int sec;
@@ -163,11 +164,8 @@ void *softpwm_thread_toggle(void *key)
   bool enabled_local = false;
   bool recalculate_timing = false;
 
-
-  get_gpio_number(key, &gpio);
-  pwm = lookup_exported_pwm((char*)key);
-
   while (!stop_flag_local) {
+    /* Take a snapshot of the parameter block */
     pthread_mutex_lock(pwm->params_lock);
     if ((freq_local != pwm->params.freq) || (duty_local != pwm->params.duty)) {
       recalculate_timing = true;
@@ -178,6 +176,7 @@ void *softpwm_thread_toggle(void *key)
     stop_flag_local = pwm->params.stop_flag;
     polarity_local = pwm->params.polarity;
     pthread_mutex_unlock(pwm->params_lock);
+
     /* If freq or duty has been changed, update the
      * sleep times
      */
@@ -220,8 +219,8 @@ void *softpwm_thread_toggle(void *key)
       }
 
       nanosleep(&tim_off, NULL);
-    }
+    }  /* if enabled_local */
-  }
+  }  /* while !stop_flag_local */
 
   if (!polarity_local)
     gpio_set_value(gpio, LOW);
@@ -240,24 +239,26 @@ int softpwm_start(const char *key, float duty, float freq, int polarity)
     int gpio;
     int ret;
 
-    get_gpio_number(key, &gpio);
+    if (get_gpio_number(key, &gpio) < 0)
-    gpio_export(gpio);
+        return -1;
-    gpio_set_direction(gpio, OUTPUT);
+    if (gpio_export(gpio) < 0)
+        return -1;
+    if (gpio_set_direction(gpio, OUTPUT) < 0)
+        return -1;
 
     // add to list
-    new_pwm = malloc(sizeof(struct softpwm));
+    new_pwm = malloc(sizeof(struct softpwm));  ASSRT(new_pwm != NULL);
-    if (new_pwm == 0) {
-        return -1; // out of memory
-    }
     new_params_lock = (pthread_mutex_t *)malloc(sizeof(pthread_mutex_t));
     if (new_pwm == 0) {
         return -1; // out of memory
     }
     pthread_mutex_init(new_params_lock, NULL);
+    pthread_mutex_lock(new_params_lock);
 
     strncpy(new_pwm->key, key, KEYLEN);  /* can leave string unterminated */
     new_pwm->key[KEYLEN] = '\0'; /* terminate string */
-    new_pwm->params.enabled = false;
+    new_pwm->gpio = gpio;
+    new_pwm->params.enabled = true;
     new_pwm->params.stop_flag = false;
     new_pwm->params_lock = new_params_lock;
     new_pwm->next = NULL;
@@ -274,20 +275,16 @@ int softpwm_start(const char *key, float duty, float freq, int polarity)
         pwm->next = new_pwm;
     }
 
-    softpwm_set_duty_cycle(new_pwm->key, duty);
+    ASSRT(softpwm_set_duty_cycle(new_pwm->key, duty) == 0);
-    softpwm_set_frequency(new_pwm->key, freq);
+    ASSRT(softpwm_set_frequency(new_pwm->key, freq) == 0);
-    softpwm_set_polarity(new_pwm->key, polarity);
+    ASSRT(softpwm_set_polarity(new_pwm->key, polarity) == 0);
 
     // create thread for pwm
-    ret = pthread_create(&new_thread, NULL, softpwm_thread_toggle, (void *)new_pwm->key);
+    ret = pthread_create(&new_thread, NULL, softpwm_thread_toggle, (void *)new_pwm);
-    if (ret) {
+    ASSRT(ret == 0);
-       PySys_WriteStderr("DEBUG; soft_pwm ERROR IN pthread_create\n");
-       exit(-1);
-    }
 
     new_pwm->thread = new_thread;
-    pthread_mutex_lock(new_params_lock);
+
-    new_pwm->params.enabled = true;
     pthread_mutex_unlock(new_params_lock);
 
     return 1;
@@ -296,7 +293,6 @@ int softpwm_start(const char *key, float duty, float freq, int polarity)
 int softpwm_disable(const char *key)
 {
     struct softpwm *pwm, *temp, *prev_pwm = NULL;
-    int gpio = 0;
 
     // remove from list
     pwm = exported_pwms;
@@ -307,9 +303,9 @@ int softpwm_disable(const char *key)
             pthread_mutex_lock(pwm->params_lock);
             pwm->params.stop_flag = true;
             pthread_mutex_unlock(pwm->params_lock);
-            get_gpio_number(key, &gpio);
+            pthread_join(pwm->thread, NULL);  /* wait for thread to exit */
-            gpio_set_value(gpio, LOW);
+
-            gpio_unexport(gpio);
+            gpio_unexport(pwm->gpio);
 
             if (prev_pwm == NULL)
             {

source/event_gpio.c

@@ -314,9 +314,13 @@ int gpio_get_direction(int gpio, unsigned int *value)
         return -1;
     }
 
-    char direction[4] = { 0 };  /* make sure read is null-terminated */
+    char direction[16] = { 0 };  /* make sure read is null-terminated */
     ssize_t s = read(fd, &direction, sizeof(direction) - 1);  e_no = errno;
     close(fd);
+    while (s > 0 && direction[s-1] == '\n') {  /* strip trailing newlines */
+        direction[s-1] = '\0';
+        s --;
+    }
     if (s < 0) {
         char err[256];
         snprintf(err, sizeof(err), "gpio_set_direction: could not read '%s' (%s)", filename, strerror(e_no));

source/py_gpio.c

@@ -213,7 +213,12 @@ static PyObject *py_input_gpio(PyObject *self, PyObject *args)
         return NULL;
     }
 
-    gpio_get_value(gpio, &value);
+    if (gpio_get_value(gpio, &value) < 0) {
+      char err[1024];
+      snprintf(err, sizeof(err), "Could not get value ('%s')", get_error_msg());
+      PyErr_SetString(PyExc_RuntimeError, err);
+      return NULL;
+    }
 
     py_value = Py_BuildValue("i", value);
 
@@ -567,7 +572,12 @@ static PyObject *py_gpio_function(PyObject *self, PyObject *args)
         return NULL;
     }
 
-    gpio_get_direction(gpio, &value);
+    if (gpio_get_direction(gpio, &value) < 0) {
+      char err[1024];
+      snprintf(err, sizeof(err), "Could not get direction ('%s')", get_error_msg());
+      PyErr_SetString(PyExc_RuntimeError, err);
+      return NULL;
+    }
     func = Py_BuildValue("i", value);
     return func;
 }

test/.ropeproject/globalnames

@@ -1,2 +1,2 @@
 <EFBFBD>}q
(Utest_softpwm_setup]q(Uteardown_moduleqUTestSoftpwmSetupqeUtest_gpio_output]q(hUTestGPIOOutputqeUtest_pwm_setup]q(hUTestPwmSetupqeUtest_gpio_setup]q	(hU	TestSetupq
-eUtest_gpio_input]q(hU
+eUtest_gpio_input]q(hU

test/gptest.py

@@ -239,4 +239,3 @@ GPIO.cleanup()
 
 mystr = "DONE: %d ERRORS" % num_errs
 print(mystr)
-

test/test_gpio_input.py

@@ -15,7 +15,7 @@ class TestGPIOInput:
         #value read from the file will have a \n new line
         value = open('/sys/class/gpio/gpio138/value').read()
         assert int(value) == input_value
-        time.sleep(30)
+        # time.sleep(30) - what is this for?
         GPIO.cleanup()
 
     def test_direction_readback(self):

test/test_gpio_output.py

@@ -1,11 +1,12 @@
 import pytest
-import os
 
 import CHIP_IO.GPIO as GPIO
 
+
 def teardown_module(module):
     GPIO.cleanup()
 
+
 class TestGPIOOutput:
     def test_output_high(self):
         GPIO.setup("CSID6", GPIO.OUT)
@@ -25,6 +26,7 @@ class TestGPIOOutput:
         GPIO.setup("CSID6", GPIO.OUT)
         direction = GPIO.gpio_function("CSID6")
         assert direction == GPIO.OUT
+        GPIO.cleanup()
 
     def test_output_greater_than_one(self):
         GPIO.setup("CSID6", GPIO.OUT)

test/test_softpwm_setup.py

@@ -2,18 +2,21 @@ import pytest
 import os
 
 import CHIP_IO.SOFTPWM as PWM
+import CHIP_IO.GPIO as GPIO
+
 
 def teardown_module(module):
     PWM.cleanup()
 
+
 class TestSoftpwmSetup:
     def test_start_pwm(self):
         PWM.start("XIO-P7", 50, 10)
         base = GPIO.get_gpio_base() + 7
         gfile = '/sys/class/gpio/gpio%d' % base
-        assert os.path.exists(base)
+        assert os.path.exists(gfile)
-        direction = open(base + '/direction').read()
+        direction = open(gfile + '/direction').read()
-        assert direction == 'out\n'
+        assert(direction == 'out\n')
         PWM.cleanup()
 
     def test_pwm_start_invalid_pwm_key(self):
@@ -25,12 +28,12 @@ class TestSoftpwmSetup:
             PWM.start("XIO-P7", -1)
 
     def test_pwm_start_valid_duty_cycle_min(self):
-        #testing an exception isn't thrown
+        # testing an exception isn't thrown
         PWM.start("XIO-P7", 0)
         PWM.cleanup()
 
     def test_pwm_start_valid_duty_cycle_max(self):
-        #testing an exception isn't thrown
+        # testing an exception isn't thrown
         PWM.start("XIO-P7", 100)
         PWM.cleanup()