Let pins float when writing (cf data sheet section 'write time slots')

[nxg]

In a write time slot, the datasheet indicates that 'the bus master must release the 1-Wire bus' after pulling it low. The current code, in write_bit, instead forces the bus high.

The attached patch seems to match the datasheet, and appears to work for me in practice.

I hope this is useful.

[nxg]

I've just noticed that this provides a fix to issue #59

[orgua]

i think paul is not interested in fixing all the bugs or keeping the codebase alive. this thing works sometimes and that seems to be enough. i proposed a version 3 (also with a fix for this problem) almost a year ago ( #39) and didn't get any response either. the are even more bus related implementation errors in the code....

[nomis]

write_bit() is documented as always leaving the bus powered; you need to call depower()

[nxg]

Can you pinpoint just where that's documented?

It's not in the comments in the code, nor in the documentation for this library at arduino.cc (either of which URIs would be regarded as authoritative by at least some people).

Also, it's worth while noting that ‘documented behaviour’ and ‘correct behaviour’ are not necessarily the same thing.

[nomis]

https://github.com/PaulStoffregen/OneWire/blob/master/OneWire.h#L100

[nomis]

The real problem here is that write_bit() has no option to avoid powering the bus. You'd need a power option on write_bit() and search() to be able to stop this and fix #59... but I see that has already been done in #39.

[orgua]

write_bit() should not power the bus at all. the original implementation states that the data line has a pull-up resistor and the communication partners only pull down or leave it floating. there are however exceptions - some devices can "order" a powered line for an amount of time.

[nxg]

It's been a few months since I last looked at this (during which my fixed version of the code has been in production), so I wouldn't want to be too dogmatic below.

While I agree that the original code does match its documentation (thanks for the pointer to that), the original code does seem to fairly clearly conflict with the datasheet. Specifically, I'm looking at the DS18B20 datasheet, which says (p15):

To generate a Write 1 time slot, after pulling the 1-Wire bus low, the bus master must release the 1-Wire bus within 15μs. When the bus is released, the 5kΩ pullup resistor will pull the bus high. To generate a Write 0 time slot, after pulling the 1-Wire bus low, the bus master must continue to hold the bus low for the duration of the time slot (at least 60μs).

Similarly, a note which ‘introduces the user to the 1-Wire® communication protocol’ at microchip.com says (p2):

To write the data, the master first initiates a time slot by driving the 1-Wire line low, and then, either holds the line low (wide pulse) to transmit a logic ‘0’ or releases the line (short pulse) to allow the bus to return to the logic ‘1’ state.

Thus, the writer fairly clearly (to me) has to release the line, not drive it high, and the original code is at variance to this.

I suspect that code which does drive the line high will work by accident in many cases, but I at least found that the code was failing in practice, until I fixed it. If the original code is indeed working only by accident, then changing the behaviour of write_bit() won't be a practical regression.

[PaulStoffregen]

I at least found that the code was failing in practice

Can you be specific about this case where things fail? Think "reproducible test case".

Just to be clear, this is a very old and very mature code base. It has been used successfully by many thousands of people across a wide range of hardware. Even if the implementation isn't perfect, even if it does not perfectly match Maxim's recommendations, I will not consider any bug reports which lack a reproducible test case. If I can not buy the needed hardware and reproduce the problem here on my workbench, I'm not going to consider this a valid issue.

If this sounds rigid and inflexible, well, it is. That is the way of maintaining very mature code.

[nxg]

I've had a chance to discuss this with the colleague I was working with on this project.

I can't give you a test-case, I'm afraid. We were talking to DS18B20 temperature sensors (so, a very standard bit of kit), but on a busy board and in an electrically noisy environment. It would be hard to boil this down to a test-rig, and this isn't amenable to a software-only test-case.

I will stress, though, that the data sheet doesn't talk about Maxim's ‘recommendations’, but about their instructions. The data sheet is unambiguous in its language, and is clear which side must release the bus and which must pull the line high. It could well be that, in many cases, the distinction between the master (incorrectly) pulling the bus high, and simply releasing it, does not matter, so that the current code works by accident. That also seems to indicate that if the code is changed to match the spec, it's unlikely to break things.

I'm reluctant to speculate about what precisely is going wrong here, since I don't have hardware to play with, but I suspect that variations in the pull-up resistors, or in the line and its capacitance, could potentially change timings significantly. At any rate, it seems important that, outside of read or write timeslots, the line is being held high only by pull-up resistors.

Also – and this is to some extent the bottom line – the code didn't work in its original state, and did work when it was changed to match the datasheet. I can't say much more than that.