PaulStoffregen / OneWire

Library for Dallas/Maxim 1-Wire Chips
http://www.pjrc.com/teensy/td_libs_OneWire.html
579 stars 382 forks source link

OneWire::write() what is the argument "power" for? #15

Closed wuyuanyi135 closed 8 years ago

wuyuanyi135 commented 8 years ago

Hello. I was using nodemcu, where the onewire library is quite similar to the implementation in your repository. We discussed the issue in nodemcu-firmware. The problem fell into that if we really need the power argument. Therefore, I want to ask, is there any purpose to implement this argument, given that the bus should always hold high after operation.

Brief

I recently ran into the problem when I use the function write() with power argument set to 0, since I had external power on the slave. But what happened, is that, this line will cause the bus pulled down after write operation. This means the slave will take this falling edge as the initiator of next bit.

See power=0 image power=1 image

Where the red cycle addressed the unexpected low level, which had make my logic analyzer took it for a bit.

I have seen many example codes using power=1 when writing, regardless the actual hardware connection is in parasite or external powering mode. Currently, the power=0 actually is not functional. Could you join our discussion for whether removing the power argument or not in that issue?

pstolarz commented 8 years ago

Hi, The "power" argument in OneWire::write() and OneWire::write_bytes() is valid and influential. It controls supplying a sensor with power directly (not via the pull-up resistor).

The problem you described looks like a bug in the OneWire library. I guess it's related to DIRECT_WRITE_LOW(); call (line 238) in OneWire::write(). I anticipate you don't use ATmega/AVR as w1 master in your setup, do you?

Could you check if you have the same issue on OneWire library with my patch and let me know about result: https://github.com/pstolarz/OneWire

Piotrek

wuyuanyi135 commented 8 years ago

@pstolarz Thanks! My MCU was esp8266, running the nodemcu firmwar. For AVR, In gpio input mode, if you say output LOW, does the gpio actually being pulled down? I guess this does not happen on AVR platform so this line was there.

I fixed that problem by simply remove the DIRECT_WRITE_LOW(); in write and write_bytes.

I have looked at your library. You have made it more explicit that if power=1, use push-pull to power the bus. I think this should have fixed the bug.

pstolarz commented 8 years ago

I think the author of OneWire::write() didn't really understand what was doing by writing DIRECT_WRITE_LOW() after setting the gpio to input mode. On AVR such approach simply turn off internal pull-up resistor, therefore doesn't change the voltage on the open-drain wire. It's somehow mysterious for me that on your plat. you may observe shortcut to the ground by switching the gpio as input (high impedance). Could you show your schematics?

PaulStoffregen commented 8 years ago

2 things:

1: Usually I ignore github issues which are really requests for tech support. Forums are the appropriate place to ask for project specific help. As a general rule, I usually don't investigate bug reports without complete sample code (no guesswork required to compile & run) and clearly started steps to reproduce the problem.

2: Currently I'm very busy with another project. All requests for features and even bug fixes that aren't extremely urgent on all libraries and projects I maintain are on hold until I have more time.

wuyuanyi135 commented 8 years ago

@pstolarz I used the forth mode. AFAIK, they haven't implemented open-drain configuration on nodemcu. They use PP to control the level. When there is possibility of shorting (when reading, bus is high but slave will pull it down), they switch it to float (input). image

I believe this is the key point that on AVR WRITE_LOW in input mode does not actually connect the bus to ground. I will try this on an Arduino board latter. @pstolarz Appreciate for helping me address the problem!

@PaulStoffregen I understand. Hopefully we have found the reason, which is not a bug probably. It's the person who ported the library ignored the difference of the platforms

pstolarz commented 8 years ago

@wuyuanyi135 I probably solved your issue. It looks like a bug in your nodemcu library not OneWire, so I think this thread should be closed by the admin to not provide others to confusion. Also the discussion should be switched to nodemcu forum.

--- All the remaining notes are nodemcu specific.

When you say, you observed short to the ground when switching gpio to the input it looked suspicious for me (digital input is characterized by high impedance and even with internal pull-down for open-emitter setup you should not observe ground). So, it looked like low level issue with gpio handling. Indeed, look at app/include/driver/onewire.h:

#define DIRECT_READ(pin) (0x1 & GPIO_INPUT_GET(GPIO_ID_PIN(pin_num[pin]))) #define DIRECT_MODE_INPUT(pin) GPIO_DIS_OUTPUT(pin_num[pin]) #define DIRECT_MODE_OUTPUT(pin) #define DIRECT_WRITE_LOW(pin) (GPIO_OUTPUT_SET(GPIO_ID_PIN(pin_num[pin]), 0)) #define DIRECT_WRITE_HIGH(pin) (GPIO_OUTPUT_SET(GPIO_ID_PIN(pin_num[pin]), 1))

DIRECT_MODE_OUTPUT() does nothing! DIRECT_MODE_INPUT() disabled output by GPIO_DIS_OUTPUT() defined in sdk/esp_iot_sdk_v1.4.0/include/gpio.h:

#define GPIO_OUTPUT_SET(gpio_no, bit_value) gpio_output_set((bit_value)<<gpio_no, ((~(bit_value))&0x01)<<gpio_no, 1<<gpio_no,0) #define GPIO_DIS_OUTPUT(gpio_no) gpio_output_set(0,0,0, 1<<gpio_no) #define GPIO_INPUT_GET(gpio_no) ((gpio_input_get()>>gpio_no)&BIT0)

gpio_output_set() is closed source but it looks like it simply sets gpio state (set/cleared) and it's mode (input/output). So when you issue:

DIRECT_MODE_INPUT(baseReg, bitmask); DIRECT_WRITE_LOW(baseReg, bitmask);

It switches gpio to the input state but later switches it back as an output shorted to the ground. This is a bug. I recommend to rethink and rewrite this stuff in your nodemcu lib. E.g. look at OneWire lib where the functions have different implementations:

#define DIRECT_READ(base, mask) ((GPI & (mask)) ? 1 : 0) //GPIO_IN_ADDRESS #define DIRECT_MODE_INPUT(base, mask) (GPE &= ~(mask)) //GPIO_ENABLE_W1TC_ADDRESS #define DIRECT_MODE_OUTPUT(base, mask) (GPE |= (mask)) //GPIO_ENABLE_W1TS_ADDRESS #define DIRECT_WRITE_LOW(base, mask) (GPOC = (mask)) //GPIO_OUT_W1TC_ADDRESS #define DIRECT_WRITE_HIGH(base, mask) (GPOS = (mask)) //GPIO_OUT_W1TS_ADDRESS

Try playing whit this. It looks better at first glance than in nodemcu. I don't have ESP platform to investigate this issue further.

Good luck Piotrek