Permalink
Browse files

Minimum changes needed to enable internal pull-ups

2 added lines enable the internal pull-ups during the read read window.
The pull-up will stay enabled after the bit read completes.

2 removed lines disabled the pull-up in the case of a non-parasite
powered bus.  Now pull-ups stay enabled after the write transaction
completes.
  • Loading branch information...
bigjosh committed Jun 23, 2014
1 parent e389ac4 commit ebba80cf61920aef399efa252826b1b59feb6589
Showing with 10 additions and 3 deletions.
  1. +10 −3 OneWire/OneWire.cpp
View
@@ -1,7 +1,14 @@
/*
Copyright (c) 2007, Jim Studt (original old version - many contributors since)
-The latest version of this library may be found at:
+Slightly modified by josh on 6/22/2014 to add support for internal pull-up resistors.
+
+More info at:
+
+ http://wp.josh.com/2014/06/21/no-external-pull-up-needed-for-ds18b20-temp-sensor
+
+Modifications based on the version of this library found at:
+
http://www.pjrc.com/teensy/td_libs_OneWire.html
OneWire has been maintained by Paul Stoffregen (paul@pjrc.com) since
@@ -157,6 +164,7 @@ uint8_t OneWire::reset(void)
delayMicroseconds(480);
noInterrupts();
DIRECT_MODE_INPUT(reg, mask); // allow it to float
+ DIRECT_WRITE_HIGH( reg , mask ); // enable pull-up resistor
delayMicroseconds(70);
r = !DIRECT_READ(reg, mask);
interrupts();
@@ -207,6 +215,7 @@ uint8_t OneWire::read_bit(void)
DIRECT_WRITE_LOW(reg, mask);
delayMicroseconds(3);
DIRECT_MODE_INPUT(reg, mask); // let pin float, pull up will raise
+ DIRECT_WRITE_HIGH( reg , mask ); // enable pull-up resistor
delayMicroseconds(10);
r = DIRECT_READ(reg, mask);
interrupts();
@@ -230,7 +239,6 @@ void OneWire::write(uint8_t v, uint8_t power /* = 0 */) {
if ( !power) {
noInterrupts();
DIRECT_MODE_INPUT(baseReg, bitmask);
- DIRECT_WRITE_LOW(baseReg, bitmask);
interrupts();
}
}
@@ -241,7 +249,6 @@ void OneWire::write_bytes(const uint8_t *buf, uint16_t count, bool power /* = 0
if (!power) {
noInterrupts();
DIRECT_MODE_INPUT(baseReg, bitmask);
- DIRECT_WRITE_LOW(baseReg, bitmask);
interrupts();
}
}

6 comments on commit ebba80c

@wldevries

This comment has been minimized.

Show comment
Hide comment
@wldevries

wldevries Dec 3, 2016

Awesome suggestion, for amateurs like me those resistors are hard to come by if you don't have a stash of them. Did you consider making a PR for the official library?

Awesome suggestion, for amateurs like me those resistors are hard to come by if you don't have a stash of them. Did you consider making a PR for the official library?

@ioch

This comment has been minimized.

Show comment
Hide comment
@ioch

ioch Mar 22, 2017

Cool. Is there any reason why not to push this upstream?

ioch replied Mar 22, 2017

Cool. Is there any reason why not to push this upstream?

@bigjosh

This comment has been minimized.

Show comment
Hide comment
@bigjosh

bigjosh Mar 22, 2017

Owner

I consider this an alternate functionality, rather than an improvement on the original general functionality so I think this should probably not be folded into the main project. For example, there might be people designing with very specific i2c parameters who could get unexpectedly messed up if all of a sudden a new 50k pull-up was added somewhere on their network.

That said, this inetrnal-pullup functionality could probably be folded into the original repo as an optional configuration parameter. I'd be willing to look into that if the original maintainer was up for it.

Owner

bigjosh replied Mar 22, 2017

I consider this an alternate functionality, rather than an improvement on the original general functionality so I think this should probably not be folded into the main project. For example, there might be people designing with very specific i2c parameters who could get unexpectedly messed up if all of a sudden a new 50k pull-up was added somewhere on their network.

That said, this inetrnal-pullup functionality could probably be folded into the original repo as an optional configuration parameter. I'd be willing to look into that if the original maintainer was up for it.

@micw

This comment has been minimized.

Show comment
Hide comment
@micw

micw Apr 2, 2017

Great idea ;)
A few days ago I tried it by manually setting the mode before using the lib which failed (probably because it's overridden by the library).
You could put your modifications between #ifdefined - statements so that they can be enabled by #define ONE_WIRE_INTERNAL_PULLUP 1. This would make it compatible with the existing lib and can be enabled on demand.

Edit: The change seems not to work on my ESP8266 (removed the external pullup and made your modifications to OneWire.cpp, git version).

micw replied Apr 2, 2017

Great idea ;)
A few days ago I tried it by manually setting the mode before using the lib which failed (probably because it's overridden by the library).
You could put your modifications between #ifdefined - statements so that they can be enabled by #define ONE_WIRE_INTERNAL_PULLUP 1. This would make it compatible with the existing lib and can be enabled on demand.

Edit: The change seems not to work on my ESP8266 (removed the external pullup and made your modifications to OneWire.cpp, git version).

@ioch

This comment has been minimized.

Show comment
Hide comment
@ioch

ioch Apr 2, 2017

Yes, I use this too, but this is not elegant ever. You can not pass this #defined from arduino IDE to lib. That mean library have to be edited every time you change your mind and it is totally not how libraries suppose to be used. Probably right way would be to have two constructors

public:
    OneWire( uint8_t pin);
    OneWire( uint8_t pin, bool softwarepullup);

And two functions handling those

OneWire::OneWire(uint8_t pin)
{
...
}
OneWire::OneWire(uint8_t pin, bool softwarepullup))
{
...
}

That way lib would be backwards compatible and if you don't use this option compiled code would be identical as in non modified library.
But boy how lazy I am to do it properly.

ioch replied Apr 2, 2017

Yes, I use this too, but this is not elegant ever. You can not pass this #defined from arduino IDE to lib. That mean library have to be edited every time you change your mind and it is totally not how libraries suppose to be used. Probably right way would be to have two constructors

public:
    OneWire( uint8_t pin);
    OneWire( uint8_t pin, bool softwarepullup);

And two functions handling those

OneWire::OneWire(uint8_t pin)
{
...
}
OneWire::OneWire(uint8_t pin, bool softwarepullup))
{
...
}

That way lib would be backwards compatible and if you don't use this option compiled code would be identical as in non modified library.
But boy how lazy I am to do it properly.

@draktheas

This comment has been minimized.

Show comment
Hide comment
@draktheas

draktheas Apr 13, 2018

@ioch, you could just add a:
bool softPullup
to the private section then change the current constructor to be:
OneWire(uint8_t pin, bool softwarePullup = false) { softPullup = softwarePullup; begin(pin); }
Then just check softPullup in all of the relevant code. This would allow for one constructor that creates both modes of operation and is still backward compatible.

@ioch, you could just add a:
bool softPullup
to the private section then change the current constructor to be:
OneWire(uint8_t pin, bool softwarePullup = false) { softPullup = softwarePullup; begin(pin); }
Then just check softPullup in all of the relevant code. This would allow for one constructor that creates both modes of operation and is still backward compatible.

Please sign in to comment.