Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Not sure unexport is working cleanly #14

Open
mikeholczer opened this issue May 23, 2013 · 13 comments
Open

Not sure unexport is working cleanly #14

mikeholczer opened this issue May 23, 2013 · 13 comments
Assignees
Milestone

Comments

@mikeholczer
Copy link

I'm new to working with the Raspberry Pi, so this could totally be user error.

I've tried your sample code and it appears to work without error. The problem I see is that if after running your code, I try similar sample code from adafruit's tutorial written in python, that code claims that the pin in still in use.

I'm still to new to all this to be able to determine if the issue is with this component, the python one, or just my lack of knowledge. I would be welcome to any ideas on how I can figure out what is going on. Thanks.

@EnotionZ
Copy link
Owner

You're referring to the voltage cycle example? Did both pins have trouble unexporting or was there only an issue with one? Also, are you using RPi.GPIO? I can run some test.

@mikeholczer
Copy link
Author

I actually did most of my testing with a simplified version of your voltage cycling code. I changed to use one pin, so I could change the pin quicker to test if it was pin specific. My usage of this module is below.

var gpio = require("gpio");
var pin = 18;
var led, intervalTimer;

// Flashing lights if LED connected to pin
led = gpio.export(pin, {
   ready: function() {
      inervalTimer = setInterval(function() {
         led.set();
         setTimeout(function() { led.reset(); }, 500);
      }, 1000);
   }
});


// reset the headers and unexport after 10 seconds
setTimeout(function() {
   clearInterval(intervalTimer);          // stops the voltage cycling
   led.removeAllListeners('change');   // unbinds change event

   led.reset();
   led.unexport(function() {
      // unexport takes a callback which gets fired as soon as unexporting is done
      process.exit(); // exits your node program
   });
}, 10000)

I was new to python, so it you are too here are adafruits instructions on setting up the Pi to run python including installing the Rpi.GPIO module: http://learn.adafruit.com/raspberry-pi-e-mail-notifier-using-leds/prepare-python

And this is the python code that complains about the pin being in use:

#!/usr/bin/env python

import RPi.GPIO as GPIO, feedparser, time

DEBUG = 1

GPIO.setmode(GPIO.BCM)
GREEN_LED = 18

GPIO.setup(GREEN_LED, GPIO.OUT)

GPIO.output(GREEN_LED, True)

time.sleep(2)

GPIO.cleanup()

@rzr
Copy link
Collaborator

rzr commented Aug 10, 2018

Please confirm this issue is still valid, BTW I plan to deprecate export/unexport API to use generic open/close is there any objection doing that?

@EnotionZ
Copy link
Owner

EnotionZ commented Oct 1, 2018

Regardless of the API change, the internal function to close/unexport is the same. I'll write some tests for this - we can validate the success of the close/unexport call by checking the gpio directory path. Moving to v1.0 milestone.

@EnotionZ EnotionZ added this to the v1.0 milestone Oct 1, 2018
@EnotionZ EnotionZ self-assigned this Oct 1, 2018
@rzr
Copy link
Collaborator

rzr commented Oct 1, 2018

IIRC unexport expects a pin number, while close knows its number (as created by open "constructor")

my 0.02EUR

@EnotionZ
Copy link
Owner

EnotionZ commented Oct 1, 2018

In this library, unexport (and now close) exists both in the module exports (which expects a pin number) and as an instance method (without pin number).

What I meant was at the end of the day, it writes the pin number to /sys/class/gpio/unexport, and afterwards we should no longer see the path /sys/class/gpio/gpio${pinNumber}/. If the unexport or close functionality works as expected, we can write a test to check the existence of that path.

@rzr
Copy link
Collaborator

rzr commented Oct 1, 2018

if close works same as unexport I dont see the point of adding it, in my mind I wished something like:

var port = gpio.open( ...);
port.write(true, ...);
port.close();

@EnotionZ
Copy link
Owner

EnotionZ commented Oct 1, 2018

Sorry, poor communication on my part. That's actually how it works in the v1 branch to adhere to the Samsung IoTjs API (except for write method call which is not implemented yet).

in v1 branch

var port = gpio.open({pin: PIN_NUM}, function(err) {
    // both these calls do the same thing
    // with the exception that port.close will stop the filewatcher
    port.close();
    gpio.close(PIN_NUM);
});

in master branch

var port = gpio.export(PIN_NUM, function() {
    // both these calls do the same thing
    // with the exception that port.unexport will stop the filewatcher
    port.unexport();
    gpio.unexport(PIN_NUM);
});

I think this particular issue (#14) has to do with the fact that when unexport is called, an external program thinks it's still open.

@EnotionZ
Copy link
Owner

EnotionZ commented Oct 1, 2018

Hmm... looking at the post I just made, since gpio.open acts like a Factory pattern for a discrete number of pins, there's no reason why we shouldn't store the GPIOPin instances internally. The benefit is that gpio.close will get all the benefit of contextually closing from the instance (port.close) like killing the filewatcher or whatever else we may add in the future.

@rzr
Copy link
Collaborator

rzr commented Oct 1, 2018

BTW: gpio.close(PIN_NUM) can close a pin that wasnt open ... so I prefer to think OOP here and remove param, while unexport(num) can be used as static method.

@EnotionZ
Copy link
Owner

EnotionZ commented Oct 1, 2018

I think I get it... so as far as opening and closing, these are the official calls we should support?

gpio.open(number, function);
gpio.unexport(number, function);
gpiopin.close(function); // assuming gpiopin is the GPIOPin instance created from open

@rzr
Copy link
Collaborator

rzr commented Oct 1, 2018

This would be aligned to:
https://github.com/Samsung/iotjs/blob/master/docs/api/IoT.js-API-GPIO.md

gpio.open( {pin: number ...} , function(err, gpiopin) {
 //...
  gpiopin.close(function(err) {...})
});

and deprecate current:

gpio.unexport(number).

@EnotionZ
Copy link
Owner

EnotionZ commented Oct 1, 2018

And not expose the unexporting of pins directly from the gpio module? I can't think of a good scenario in my head where a user would want to unexport a pin that wasn't opened using this module, so it's probably fine.

What do you think about adding the ability to close/unexport all pins directly from gpio? Something like gpio.closeAll(function) which would iterate through all opened pins and call gpioPin.close(); on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants