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

Support for pull-down resistor on pin #72

Merged
merged 1 commit into from Oct 26, 2016

Conversation

andrewn
Copy link
Collaborator

@andrewn andrewn commented Oct 17, 2016

Mirrors the activation of the pull-up resistor in that writing LOW to a pin will enable the built-in pull-down resistor. This does no checking to see if the pin actually has the required resistor.

The limitation of this API is that once set, there's no way to set the pin back to the no-resistor state.

Fixes #71.

Mirrors the activation of the pull-up resistor in that writing LOW
to a pin will enable the built-in pull-down resistor. This does
no checking to see if the pin actually has the required resistor.

The limitation of this API is that once set, there's no way to
set the pin back to the no-resistor state.
Copy link
Owner

@nebrius nebrius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! The code looks good, but I have an ecosystem question for you.

@@ -422,6 +422,8 @@ class Raspi extends EventEmitter {
const pinInstance = this[getPinInstance](this.normalize(pin));
if (pinInstance.mode === INPUT_MODE && value === HIGH) {
this[pinMode]({ pin, mode: INPUT_MODE, pullResistor: PULL_UP });
} else if (pinInstance.mode === INPUT_MODE && value === LOW) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this behavior landed in Johnny-Five yet?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just opened an issue and pull-request to add this behaviour to Johnny-Five and start a discussion. 👇

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! We can leave this PR open for now, but I want to wait to land it until the discussion for Johnny-Five is resolved.

Just an FYI, this module is specifically designed to be a shim between Johnny-Five and Raspi.js. So from a design perspective, it doesn't make sense to add anything here that Johnny-Five specifically isn't requesting.

If you want to use raspi-io w/o Johnny-Five, I would recommend using Raspi.js as it's a little easier to use IMO.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nebrius I'm ok with the minor change in Johnny-Five, since it's really just making a way to explicitly do something that is the default behavior of StandardFirmata.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just waiting on tests (or response of any kind) rwaldron/johnny-five#1243

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nebrius Ah, sorry if I've done things the wrong way round. Johnny-Five + Raspi-IO is my go-to for doing physical computing on the Pi so thanks for that!

This PR (and the soft PWM one) mean that I can use the built-in Johnny-Five components like Button and RGB LEDs with more of the Pi's GPIO pins without needing extra resistors.

@rwaldron
Copy link
Collaborator

rwaldron commented Oct 26, 2016

@nebrius the corresponding patch landed in J5. I will do a release shortly.

Johnny-Five + Raspi-IO is my go-to for doing physical computing on the Pi so thanks for that!

<3

@rwaldron
Copy link
Collaborator

@nebrius
Copy link
Owner

nebrius commented Oct 26, 2016

Johnny-Five + Raspi-IO is my go-to for doing physical computing on the Pi so thanks for that!

Yay! :)

https://github.com/rwaldron/johnny-five/releases/tag/v0.10.4

Sweet! This looks good to me then. I'm working on a few other fixes to roll into a new release, but nonetheless this should be published sometime in the next few days.

Thanks again for the help!

@nebrius nebrius merged commit 2383534 into nebrius:master Oct 26, 2016
@rwaldron
Copy link
Collaborator

@andrewn great job coordinating these patches!!

@nebrius as always <3

@nebrius
Copy link
Owner

nebrius commented Oct 26, 2016

btw, since you've landed a few commits, I went ahead and added you as a collaborator @andrewn, feel free to take a look around!

@andrewn andrewn deleted the support-pull-down branch October 27, 2016 09:55
@andrewn
Copy link
Collaborator Author

andrewn commented Oct 27, 2016

Thanks both, I'm just happy to be able to contribute something back to these projects.

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

Successfully merging this pull request may close these issues.

None yet

3 participants