zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.04k stars 6.18k forks source link

W1 DS2484 seems to ignore W1 disconection. #74898

Open Martdur opened 1 week ago

Martdur commented 1 week ago

Describe the bug Using STM32wl55jc with a DS2484 on a I2C bus controlling several DS18B20. I'm managed to make all working great with all W1 feature I need by using the DS2484 sensor driver. But when I disconnect some sensors from the W1 bus (not all), I don't get any error from a match rom -> write -> read. Despite the fact that the sensor that I match isn't on the bus anymore. This introduce wrong sensor value and no clear way on how to handle sensor disconnection.

To Reproduce Steps to reproduce the behavior: You can use the scanner sample as a starter point.

  1. Save the different rom you discovered.
  2. Try to use w1_write_read method (it should work well).
  3. Introduce it in a loop(with some delay).
  4. Print the error and the data.
  5. While the program is running disconnect one sensor.

Expected behavior You should observe no error from the W1 API w1_write_read function.

Impact This impact the error handling since we can not know if the sensor is connected or not. Furthermore the data read from the disconnected device is false. (ex -0.062500°C for the DS18B20)

Environment (please complete the following information):

Additional context My project is OO so sorry If I can not provide a simple example to run atm.

str4t0m commented 1 week ago

Thanks for reporting this issue. The DS18B20 sensor driver is clearly missing a validity check of the scratchpad, and I have created a fix to address this. Will open a PR once I have done some additional testing in the evening.

Martdur commented 1 week ago

What I've done as a workaround is to test if all value in the scratchpad are equals to 0xff. Hope that can help.

str4t0m commented 5 days ago

@Martdur with a bit a delay I have opened #75246 to address this issue. Maybe you could have a look at it and check if it resolves your issues.

Martdur commented 5 days ago

@Martdur with a bit a delay I have opened #75246 to address this issue. Maybe you could have a look at it and check if it resolves your issues.

Thanks for the PR 75246! Sorry for the misleading post, I'm using the DS2484 driver but not the DS18B20 driver, I've done my how implementation of the DS18B20 driver in cpp. I was hoping that the w1_write_read was able to detect this kind of error.

str4t0m commented 5 days ago

I see, yeah I am considering changing the API to return an error in such a case, but as we are already in the release phase for 3.7 only fixes will be merged and no API changes. So the priority is to improve/fix the handling for the sensor driver for now. I'll need to check more datasheets to be sure that we will never need to read blocks of only 0xff as a precondition to such a change, which I didn't have enough time to do yet.