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

SensorRelay fixes #536

Open
wants to merge 2 commits into
base: development
Choose a base branch
from
Open

Conversation

nekromant
Copy link

This MR attempts to fix the problem of relays toggling on/off during initialisation, especially active-low relays. This can be a big problem if a board has some 8 relays toggling some serious loads.

Signed-off-by: Andrew Andrianov <andrew@ncrmnt.org>
Signed-off-by: Andrew Andrianov <andrew@ncrmnt.org>
@nekromant
Copy link
Author

@user2684 There seems to be some CI problem that's definetely NOT related to this MR

 ls -l --recursive ./examples/RFM69/build/
ls: cannot access './examples/RFM69/build/': No such file or directory

@user2684
Copy link
Contributor

@nekromant Hi, sorry for reviewing this soooo late! A couple of random comments:

  • Yes, the checks failed are not due to your code, this is something I will need to fix in a different PR
  • I see you call setStatus() before initializing the pin with pinMode(). Are you sure this is not creating issues?
  • The "inverted" variable in SensorRelay. would you please add it as the last parameter of the class? Otherwise would break compatibility with other sensors inheriting from it
  • Can you please add newly added functions to README.md?

Thanks a lot!

@user2684 user2684 added this to In Progress in v1.9 via automation Mar 29, 2021
@nekromant
Copy link
Author

I see you call setStatus() before initializing the pin with pinMode(). Are you sure this is not creating issues?

Precisely! The spurious relay click happens between the moment we set the pin mode to output, and before we write the actual value to the pin. This way we have a value we want to use in the actual register before we change modes, thus avoiding the glitch.
As a side effect, on avr this enables/disables the pull-up, but that should match the desired pin state later on.

As for the rest, I'll fix it ASAP.

@user2684
Copy link
Contributor

Interesting, thanks, never had the chance to notice it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
v1.9
  
In Progress
Development

Successfully merging this pull request may close these issues.

None yet

2 participants