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.

Sign up for GitHub

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

Enable pull-up resistor in OneWire library #1226

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Enable pull-up resistor in OneWire library #1226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Sming/Libraries/OneWire/OneWire.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ OneWire::OneWire(uint8_t pin)

void OneWire::begin()
{
pinMode(pin, INPUT);
pinMode(pin, INPUT_PULLUP);
noPullup(pin);
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it will disable the pullup. To me this looks put here on purpose maybe the Arduino way is not to enable pullups?. Also be aware that if you enable pullups and also have pullups they will go in parallel making the equivalent smaller in value that together with line capacitance might not be appropiate for given sck frequency.
I would not touch lib code if it is working an tested as it probably is consistent with Arduino way. After onewire::begin you can manually set the pins to pullup in your client code.

bitmask = PIN_TO_BITMASK(pin);
baseReg = PIN_TO_BASEREG(pin);
Expand All @@ -136,7 +136,7 @@ void OneWire::begin()
void OneWire::begin(uint8_t pinOneWire)
{
pin = pinOneWire;
pinMode(pin, INPUT);
pinMode(pin, INPUT_PULLUP);
noPullup(pin);
bitmask = PIN_TO_BITMASK(pin);
baseReg = PIN_TO_BASEREG(pin);
Expand Down
4 changes: 2 additions & 2 deletions Sming/Libraries/OneWire/OneWire.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@
#define IO_REG_TYPE uint16_t
#define IO_REG_ASM
#define DIRECT_READ(base, mask) (digitalRead(mask) ? 1 : 0)
#define DIRECT_MODE_INPUT(base, mask) pinMode(mask, INPUT)
#define DIRECT_MODE_INPUT(base, mask) pinMode(mask, INPUT_PULLUP)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the really interesting change

#define DIRECT_MODE_OUTPUT(base, mask) pinMode(mask, OUTPUT)
#define DIRECT_WRITE_LOW(base, mask) digitalWrite(mask, LOW);
#define DIRECT_WRITE_HIGH(base, mask) digitalWrite(mask, HIGH);
Expand All @@ -123,7 +123,7 @@
#define IO_REG_TYPE uint16_t
#define IO_REG_ASM
#define DIRECT_READ(base, mask) ((digitalRead(mask) > 0) ? 1 : 0)
#define DIRECT_MODE_INPUT(base, mask) (pinMode(mask, INPUT))
#define DIRECT_MODE_INPUT(base, mask) (pinMode(mask, INPUT_PULLUP))
#define DIRECT_MODE_OUTPUT(base, mask) (pinMode(mask, OUTPUT))
#define DIRECT_WRITE_LOW(base, mask) (digitalWrite(mask, LOW))
#define DIRECT_WRITE_HIGH(base, mask) (digitalWrite(mask, HIGH))
Expand Down