gobot icon indicating copy to clipboard operation
gobot copied to clipboard

NewRelayDriver should allow for creating inverted relay

Open andrewebdev opened this issue 5 years ago • 8 comments

Currently .On() sets the relay state to high, and .Off() sets the relay state to low.

We have realays boards in one of our projects that are actually inverted. This causes us to do things in the code like relayDriver.On() when we want it switched off, and of course it leads to some confusion down the line.

It may be a good idea for NewRelayDriver to allow for this use case?

andrewebdev avatar Dec 31 '18 16:12 andrewebdev

Seems like a good idea. A PR that added this would be gladly accepted.

deadprogram avatar May 31 '19 06:05 deadprogram

@deadprogram just a ping to remind that I have done a PR for this. (just in case this issue was overlooked)

andrewebdev avatar Jun 15 '19 13:06 andrewebdev

func (l *RelayDriver) On() (err error) {
	if err = l.connection.DigitalWrite(l.Pin(), 1); err != nil {
		return
	}

	if l.Inverted {
		l.high = false
	} else {
		l.high = true
	}

	return
}

I notice you are still writing 1 to DigitalWriter (Outside the Inverted if logic) in Driver.On - this is hardly helping. Most of the relays close NO-C on LOW ! Wouldn't it help to check Inverted before you DigitalWrite 1.

kneerunjun avatar Oct 16 '19 01:10 kneerunjun

As @kneerunjun noted, On() always writes 1 and Off() always write 0. I expected inverted to invert the value that was written as well. Unfortunately this means that I'm still having to use Off() to turn on the component attached to the relay.

Would you accept a PR that takes the Inverted field into account when writing?

stuartleeks avatar Mar 29 '20 21:03 stuartleeks

Yes, please, and thanks!

deadprogram avatar Mar 29 '20 21:03 deadprogram

Great :-)

stuartleeks avatar Mar 29 '20 21:03 stuartleeks

@stuartleeks Thank for this. I saw this email yesterday and was going to start updating this today; Then I noticed you already submitted a pull request :)

Not sure how I missed that it still writes 1 instead of 0. Our code that uses this functionality is currently still doing the old manual if-check, so we never actually caught this bug in our code.

Good job catching it!

andrewebdev avatar Mar 31 '20 11:03 andrewebdev

Thanks @andrewebdev. I was working on a home project with my raspberry pi and thought I'd misunderstood gobot or mis-wired my relay. After checking my circuits and code I started digging in to the gobot code and found what @kneerunjun described which led me to this issue.

Hopefully we can get the PR merged and the change released soon, and then I can use the Inverted setting and remove the big NOTE comment I have in my code explaining the use of Off() for On() :grin:

stuartleeks avatar Mar 31 '20 12:03 stuartleeks

Seems to be fixed. Otherwise please try "WithPinActiveLow()", available on "dev" branch.

gen2thomas avatar Jan 28 '23 12:01 gen2thomas