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

Add OpenDrainIO pin state (with InputPin capability) #401

Merged
merged 6 commits into from
Sep 27, 2022

Conversation

Finomnis
Copy link
Contributor

As already reported in #339, there is currently no way to create an I/O pin in nrf-hal.

There are several reasons why having this would be useful. My personal usecase is the DHT22/AM2302 sensor. Controlling it requires a single-wire pulled-up open drain IO.

Sadly, this means that all libraries that can interface with it require InputPin + OutputPin.

Solution

There is no inherent reason why nrf chips shouldn't be able to implement this. The input buffer is always available and allows reading back the pin values during every pin configuration. Although for energy saving reasons, the input buffer is currently disabled during the OpenDrain state.

My proposal is to add an OpenDrainIO state that does not disable the input buffer and implements InputPin.

@Finomnis Finomnis changed the title Open Drain with Input OpenDrain pin state with InputPin capability Sep 20, 2022
@Finomnis

This comment was marked as outdated.

@Finomnis

This comment was marked as outdated.

@Finomnis

This comment was marked as outdated.

@Finomnis Finomnis changed the title OpenDrain pin state with InputPin capability Add OpenDrainIO pin state (with InputPin capability) Sep 23, 2022
@Finomnis
Copy link
Contributor Author

Will write tests for this as well, as soon as I have an nrf52840-DK.

@jonas-schievink
Copy link
Contributor

I wonder if this should just be the behavior of OpenDrain pins in general? I agree with #340 (comment) that it's a footgun for PushPull pins, but for OpenDrain, not so much, since those are legitimate input pins.

@Finomnis
Copy link
Contributor Author

Finomnis commented Sep 26, 2022

I wonder if this should just be the behavior of OpenDrain pins in general?

To my knowledge this was an intentional decision, because having an input buffer connected to a floating pin can cause substantially increased power drain. So people wanted to have an explicit opt-in. But I'm open to dispute that decision.
Note that this would be potentially a (behavioral) breaking change.

@jonas-schievink
Copy link
Contributor

Okay, that sounds good!

bors r+

@bors
Copy link
Contributor

bors bot commented Sep 27, 2022

Build succeeded:

@bors bors bot merged commit 1f8fccf into nrf-rs:master Sep 27, 2022
@Finomnis Finomnis deleted the open_drain_io_pin branch September 28, 2022 10:25
@Finomnis
Copy link
Contributor Author

@jonas-schievink Added tests in #402

bors bot added a commit that referenced this pull request Sep 28, 2022
402: Add missing OpenDrain[IO] tests r=jonas-schievink a=Finomnis

- Add tests for `OpenDrainIO`
    - Added in #401
- Add missing test for `OpenDrain`

Co-authored-by: Finomnis <[email protected]>
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.

2 participants