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

Suggestion - add relay class? #66

Closed
raspitv opened this issue Oct 13, 2015 · 13 comments
Closed

Suggestion - add relay class? #66

raspitv opened this issue Oct 13, 2015 · 13 comments

Comments

@raspitv
Copy link
Contributor

raspitv commented Oct 13, 2015

I've been hacking up a thing I'll be blogging about in a day or two. It uses a small arduino relay board that triggers when a port is 0 and is off when a port is 1. I've been using the LED class to drive it, but obviously have to invert the logic. It would be nice to have an inverted logic class so that you could address a relay as e.g. lamp.on() for it to be on instead of lamp.off() when you wanted it switched on.

It would certainly make things easier for kids. The only other way round it is to wire up the lamp to the NC terminal but then it's always on unless actively switched off, which isn't ideal either. It's a suggested addition that I think would be very worthwhile.

@RZRZR
Copy link

RZRZR commented Oct 13, 2015

I agree in principle, but if the aim of GPIOzero is to encourage the very beginners, then playing with high voltage should come with some warnings.

An energenie class would be good.

@bennuttall
Copy link
Member

How common is it for relays to work this way? (Sorry, I'm unfamiliar)

Are there relays that work the opposite way?

If so, perhaps a Relay class with an optional active_high=True argument would work?

@RZRZR
Copy link

RZRZR commented Oct 13, 2015

It is strongly encouraged that relays are wired up NO as Alex has described.

You can have them the other way, but that means if the Pi is disconnected then it will be NC (normally closed - i.e. contacts closed, power can run through, device is on). And if the device is a bunch of loose wires you've connected to 240V then that isn't good.

@P33M
Copy link

P33M commented Oct 13, 2015

The use of direct wiring to mains voltages is dangerous unless you know what you're doing and so you won't find any of our examples using it.

At the most, I'd go with an Energenie class (or some other popular RF/IR controlled plug switch).

@raspitv
Copy link
Contributor Author

raspitv commented Oct 13, 2015

It's a 12V lamp I'm using. I don't recommend using relays for High Voltage either :) Sorry for being vague. I don't want to let the cat out of the bag before I publish :)

@waveform80
Copy link
Member

Sounds like just as InputDevice has an optional pull_up argument to the constructor, OutputDevice ought to have a similar optional argument (active_high? invert_logic? your_suggestion_here?) which is forced to False for LED (and every other extant class) but which something like a Relay class could default to True but permit flipping (in a similar manner to how Button flips the default pull-up to True but permits users to set it back to False if they want to wire things differently).

If everyone's happy with active_high I can bung together a PR for this in a bit (just finished the PIR changes).

@bennuttall
Copy link
Member

Yeah that makes sense. Would we need to ensure it's initially switched off?

@raspitv
Copy link
Contributor Author

raspitv commented Oct 13, 2015

Off would be GPIO.HIGH in this case if it was one of the Arduino relay boards (pretty much all the ones I've seen are like that). And yes I think it should default to off initially if possible.

@bennuttall bennuttall mentioned this issue Oct 13, 2015
@waveform80
Copy link
Member

Just finished the active_high changes (and realized I had my logic backwards in the previous message: active_high is True for LEDs, False for relays). I'll PR it in a mo so everyone can have a look at the changes.

I'm a bit wary about the initially switched off bit. When we initialize a GPIO device at the moment we just set it up as an output, but this doesn't change its state - basically we leave the device in whatever state we find it (if the GPIO was already high, we leave it high, which is why you can connect an LED to a GPIO left previously high and it'll light up with no script running, then construct an LED object and it'll remain lit until you explicitly force it low).

This is generally considered good practice as people don't tend to expect constructors to "take action" beyond preparing a resource to manipulate its state. They generally expect subsequent methods (named after verbs) to "take action" (like switching stuff on and off, read and writing to sockets / files, and so forth). Thus having our constructor implicitly change state on the GPIO doesn't feel quite right to me.

It also doesn't feel quite right deliberately forcing the GPIO high upon construction. If that's the "safe" position, then surely they weren't "safe" until the script started (GPIOs generally default to low with the exception of 2 and 3 which have enforced pull-ups), but I'm sure there's something I'm not understanding there...

waveform80 added a commit to waveform-computing/gpio-zero that referenced this issue Oct 14, 2015
This PR adds the `active_high` parameter (defaulted to `True`) to the
`OutputDevice` class. This permits flipping the logic of an output
device in a similar manner to `pull_up` on `InputDevice`.

It also adds a `Relay` class derived from `DigitalOutputDevice` which
uses this parameter to flip the logic (as this is typically required
with relays).
@waveform80
Copy link
Member

Right, there's the PR - go play and let me know if it's okay (haven't got any relays on my ever more cluttered desk! :)

@bennuttall
Copy link
Member

Ok just been discussing this in the office. There's a feeling that relays are potentially dangerous and the presence of a class gives too much of a push towards using them without proper understanding.

I think if we have an OutputDevice class that allows active_high configuration, that should be enough for those (like @raspitv) who know what they're doing.

I'm going to merge the PR and delete the Relay class.

bennuttall added a commit that referenced this issue Oct 14, 2015
Fix #66 - Add Relay and make active_high configurable
bennuttall added a commit that referenced this issue Oct 14, 2015
@waveform80
Copy link
Member

That's probably a decent choice for now. I might suggest that perhaps in future the class ought to be added back in, but left out of chapters like "Getting Started", and "Recipes" (in my experience, if you leave stuff out of there, very few people notice it - only weirdos who go around trawling API references ;)

@bennuttall
Copy link
Member

True

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

5 participants