gobot
gobot copied to clipboard
NewRelayDriver should allow for creating inverted relay
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?
Seems like a good idea. A PR that added this would be gladly accepted.
@deadprogram just a ping to remind that I have done a PR for this. (just in case this issue was overlooked)
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.
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?
Yes, please, and thanks!
Great :-)
@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!
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:
Seems to be fixed. Otherwise please try "WithPinActiveLow()", available on "dev" branch.